Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"make" should run Sage once #11926

Closed
jdemeyer opened this issue Oct 15, 2011 · 98 comments
Closed

"make" should run Sage once #11926

jdemeyer opened this issue Oct 15, 2011 · 98 comments

Comments

@jdemeyer
Copy link
Contributor

In a multi-user environment, the user compiling Sage must run it at least once to run sage-location and generate .pyc files.

The proposed fix is: in the default make rule, run Sage when local/bin/sage-started.txt does not exist and create that file in sage-location. Also run Sage after upgrading.

This patch also changes the error message when a spkg fails a build or test. Example error message:

Error: Configuring PARI with readline and GMP kernel failed.

real    0m0.100s
user    0m0.012s
sys     0m0.012s
************************************************************************
Error installing package pari-2.4.3.alpha.p7
************************************************************************
Please email sage-devel (http://groups.google.com/group/sage-devel)
explaining the problem and including the relevant part of the log file
  /usr/local/src/sage-4.7.2.rc0/spkg/logs/pari-2.4.3.alpha.p7.log
Describe your computer, operating system, etc.
If you want to try to fix the problem yourself, *don't* just cd to
/usr/local/src/sage-4.7.2.rc0/spkg/build/pari-2.4.3.alpha.p7 and type 'make' or whatever is appropriate.
Instead, the following commands setup all environment variables
correctly and load a subshell for you to debug the error:
  (cd '/usr/local/src/sage-4.7.2.rc0/spkg/build/pari-2.4.3.alpha.p7' && '/usr/local/src/sage-4.7.2.rc0/sage' -sh)
When you are done debugging, you can type "exit" to leave the subshell.
************************************************************************
Error: Failed to install package 'pari'.

Apply:

  1. attachment: 11926.patch to SAGE_ROOT
  2. attachment: 11926_sage_starts.patch, attachment: trac_11926-error-msg.patch, attachment: 11926-error-msg-review.patch to SCRIPTS (local/bin)
  3. attachment: 11926_sage.patch and attachment: 11926_doc.patch to the Sage library.

CC: @pcpa

Component: build

Keywords: Makefile build sage-starts

Author: Jeroen Demeyer

Reviewer: John Palmieri, Leif Leonhardy

Merged: sage-4.8.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/11926

@jdemeyer

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

I think I suggested this sort of thing once, and it was criticized. I don't remember the critiques. Anyway, I like the idea. In case some system administrator is building Sage and doesn't want a .sage directory created for them, should we set DOTSAGE to something temporary first?

By the way, running Sage as part of the build process should also take care of some of the issues at #5155 (not all of them, unfortunately).

@jdemeyer
Copy link
Contributor Author

comment:4

Replying to @jhpalmieri:

I think I suggested this sort of thing once, and it was criticized. I don't remember the critiques. Anyway, I like the idea.

In case some system administrator is building Sage and doesn't want a .sage directory created for them, should we set DOTSAGE to something temporary first?

Good idea. Make a temporary DOTSAGE, run Sage, remove the temporary DOTSAGE

By the way, running Sage as part of the build process should also take care of some of the issues at #5155 (not all of them, unfortunately).

Thanks for the pointer. Also #11887 is related.

@jdemeyer
Copy link
Contributor Author

Work Issues: temporary DOTSAGE

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 15, 2011

comment:5

I don't like the idea of running sage after make build since it certainly doesn't belong to that target, but if at all, it shouldn't be run from spkg/install but the Makefile, preferably in the default rule, not build. Or run sage-location from there if make did anything, but we should in principle run it from elsewhere.

Someone who is going to install Sage system-wide should run make ptestlong or similiar anyway (which also runs sage), just like one runs make check before running make install.

Also note that sage (or sage-location) should be run (at least) after it has been moved to its final installation location, which is likely to happen after having built Sage.

@jdemeyer
Copy link
Contributor Author

comment:6

Replying to @nexttime:

I don't like the idea of running sage after make build since it certainly doesn't belong to that target

I think it does. Currently, after make build (or simply make), Sage does not work for a different user. I think make build should result in a proper Sage installation, which means that it should work for every user.

it shouldn't be run from spkg/install but the Makefile

Why? I like spkg/install better because it allows us to run Sage only if something was actually installed. You don't want to run Sage everytime somebody does make foo for various values of "foo". Also, IIRC, we run spkg/install on upgrading but not a top-level make all or make build.

Or run sage-location from there if make did anything, but we should in principle run it from elsewhere.

Running sage-location is certainly not sufficient to generate all files needed to run Sage.

Someone who is going to install Sage system-wide should run make ptestlong

Let the sysadmin decide whether they want to run make ptestlong or not, I want to give him the option of NOT doing it.

Also note that sage (or sage-location) should be run (at least) after it has been moved to its final installation location, which is likely to happen after having built Sage.

I would think most people run Sage from the location where they installed it, or equivalently, install Sage in the location they want to run it from. In any case, this does not invalidate the reasons for this ticket.

@jdemeyer
Copy link
Contributor Author

comment:7

Updated version of the patch using a temporary DOT_SAGE directory. I have not really tested it well.

@jdemeyer
Copy link
Contributor Author

Changed work issues from temporary DOTSAGE to none

@jdemeyer
Copy link
Contributor Author

comment:8

Tested with a complete build from scratch that this does indeed what it should do. A simple make build allows any user to run Sage.

@jhpalmieri
Copy link
Member

comment:9

I don't see anything in the new patch about DOT_SAGE.

@jdemeyer
Copy link
Contributor Author

comment:10

Replying to @jhpalmieri:

I don't see anything in the new patch about DOT_SAGE.

So true! I seems I forget a hg qrefresh.

@jdemeyer
Copy link
Contributor Author

comment:11

Replying to @jhpalmieri:

In case some system administrator is building Sage and doesn't want a .sage directory created for them, should we set DOTSAGE to something temporary first?

Related question which comes up: should we also use a temporary DOT_SAGE directory for make doc? In #10163, I found out that make doc does run Sage (and does create a DOT_SAGE directory).

@jdemeyer

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:13

I'm somewhat tempted to add a new command-line option to Sage:

diff --git a/sage-sage b/sage-sage
--- a/sage-sage
+++ b/sage-sage
@@ -247,6 +247,27 @@ if [ $# -gt 0 ]; then
   fi
 fi
 
+# The following function creates a temporary file and stores its name
+# in $FILE.  (It would be nice to replace this with 'mktemp', but that
+# command has different syntax on OS X compared to linux or Solaris.)
+sagetempfile() {
+    FILE=`mktemp -d 2>/dev/null`
+    if [ $? -ne 0 ]; then
+        # presumably because the "-d" option to mktemp expects an argument,
+        # e.g., on OS X.
+        FILE=`mktemp -d -t dotsage`
+    fi
+}
+
+if [ "$1" = '--norc' -o "$1" = '--nodotsage' ]; then
+    OLD_DOT_SAGE=$DOT_SAGE
+    sagetempfile
+    DOT_SAGE=$FILE && export DOT_SAGE
+    shift
+    sage "$@"
+    rm -rf "$DOT_SAGE"
+    DOT_SAGE=$OLD_DOT_SAGE && export DOT_SAGE
+fi
 
 LOGFILE="$SAGE_ROOT/sage.log"
 LOGOPT=""

We should probably check exit codes for various things here, and of course this would need to be documented. If we had this, then in Makefile, we could replace "sage --docbuild ..." with "sage --norc --docbuild ...". We could possibly do the same with "make test", etc.

@jdemeyer
Copy link
Contributor Author

comment:14

Replying to @jhpalmieri:

I'm somewhat tempted to add a new command-line option to Sage:

Let's discuss this in a different ticket: #11932.

I think I am going to revert the DOT_SAGE handling here and leave that for #11932, because currently it's very hard to avoid having a $HOME/.sage directory created when installing Sage.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 2, 2011

comment:74

Added an extra patch with the documentation changes proposed by leif, needs_review.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 3, 2011

Milestone sage-4.7.3 deleted

@jdemeyer jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011
@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 3, 2011

Changed merged from sage-4.7.3.alpha0 to sage-4.8.alpha0

@jdemeyer jdemeyer added this to the sage-4.8 milestone Nov 3, 2011
@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 3, 2011

comment:78

Attachment: 11926_doc.patch.gz

Can somebody please review the last patch here?

@jhpalmieri
Copy link
Member

comment:79

Looks okay to me.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 3, 2011

comment:80

Replying to @jhpalmieri:

Looks okay to me.

Yes.

But still doc should depend on start, such that one immediately gets a single, meaningful error message, not intermixed by repeated random tracebacks from Sphinx (after each of which the script btw. still unconditionally prints "Build succeeded. The built documents can be found in...").

And it would be far better to have the output of sage-starts also appended to install.log, for the reasons given earlier.

W.r.t. #11021:

Despite its current title, it involves a "major" rewrite of sage-spkg, i.e., makes changes all over the code, half of which were -- though already positively reviewed -- removed from another ticket for IMHO no reason (since at that point, they did apply and didn't break any other tickets).

So that ticket certainly drags (which mainly is my fault; I've returned to it many times, but then again had to work on too many other tickets), but it doesn't make things easier if other tickets do redundant changes to sage-spkg, thereby just causing tedious rebasing.

One can by the way search for other tickets before working on e.g. scripts or spkgs. And there's btw. also a meta-ticket for changes to the Makefile, #11622.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 4, 2011

comment:81

Replying to @nexttime:

But still doc should depend on start, such that one immediately gets a single, meaningful error message

I don't find it logical that doc would depend on start. I still think the best solution would be to have make build run sage-starts. So here, we have to agree to disagree.

So that ticket certainly drags (which mainly is my fault; I've returned to it many times, but then again had to work on too many other tickets), but it doesn't make things easier if other tickets do redundant changes to sage-spkg, thereby just causing tedious rebasing.

If we are not allowed to make patches just because some other ticket makes changes to the same code, then we might as well stop Sage development. Especially if the ticket gets "dragged" as you say.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Nov 4, 2011

comment:82

Replying to @nexttime:

And it would be far better to have the output of sage-starts also appended to install.log, for the reasons given earlier.

I don't have strong feelings about this issue. If you want to make this change on a different ticket, go ahead, I will have a look.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 4, 2011

comment:83

Replying to @jdemeyer:

Replying to @nexttime:

But still doc should depend on start, such that one immediately gets a single, meaningful error message

I don't find it logical that doc would depend on start. I still think the best solution would be to have make build run sage-starts. So here, we have to agree to disagree.

The builder imports sage.all, hence doc requires that Sage can start.

Perhaps sage --docbuild should check once that it is functional (with try: import sage.all ...).

If we are not allowed to make patches just because some other ticket makes changes to the same code, then we might as well stop Sage development. Especially if the ticket gets "dragged" as you say.

I was talking about unnecessary, unrelated changes. Any developer should at least be aware of other tickets touching the same code.

It's also bad practice to bundle unrelated changes to different files into a single patch, just because they're in the same repo.

@jhpalmieri
Copy link
Member

comment:84

See #11991 for what I hope is a quick a follow-up: sage-starts should record the time and version number.

@kiwifb
Copy link
Member

kiwifb commented Nov 24, 2011

comment:85

I hate to be late to the party. I was bogged down into work. I appreciate the point of view that most people won't build the stuff system wide. Me and Paulo from Mandriva do. I and probably him will object to that sage-started.txt file being stored in $SAGE_LOCAL/lib/. To us it belongs to DOTSAGE. I know it complicates the testing that Jeroen wants to do. But this goes a long way to making a system wide installation troublesome.

I may have missed it but where is the documentation for someone who wants to make a system wide install? Just this ticket?

@jhpalmieri
Copy link
Member

comment:86

I think that sage_started.txt should be viewed as a file like sage-current-location.txt: a file which is produced during the build/test process, which ordinary users don't need write-access to. This ticket is related to some of the issues at #5155, and should indeed fix some of them. But maybe I don't understand your complaint; can you explain it more?

@jdemeyer
Copy link
Contributor Author

comment:87

Replying to @kiwifb:

I hate to be late to the party. I was bogged down into work. I appreciate the point of view that most people won't build the stuff system wide. Me and Paulo from Mandriva do. I and probably him will object to that sage-started.txt file being stored in $SAGE_LOCAL/lib/. To us it belongs to DOTSAGE. I know it complicates the testing that Jeroen wants to do. But this goes a long way to making a system wide installation troublesome.

Why do you think this makes a system-wide install harder? In fact, this ticket was exactly supposed to make it easier to do a system-wide install.

I may have missed it but where is the documentation for someone who wants to make a system wide install? Just this ticket?

See http://sagemath.org/doc/installation/source.html#installation-in-a-multiuser-environment. Of course this ticket (and probably others to be merged in sage-4.8) will change this documentation, but the essence should remain the same.

@kiwifb
Copy link
Member

kiwifb commented Nov 25, 2011

comment:88

OK. I just completed my first ebuild of a sage-4.8 alpha this afternoon. The first I noticed about this is when I ran the doctests as a normal user. I do that straight away as this is the only validation test I have. A truckload of doctests just failed complaining that they couldn't find "sage-started.txt". Actually so many failed that I am almost wondering if there is something wrong with the ones that didn't.

I spent 3 weeks fixing bad fortran code and bad makefiles so I may have had a bit of a temper there but it just made my life difficult.

I am not sure how it helps with #5155 since it adds stuff that actually wants access to SAGE_ROOT.

I'll admit that I skimmed a bit on reading this thread so I won't attribute bad intent but using DOT_SAGE has been raised and apparently shot down as "system wide install" is not a normal install. You don't have to support the kind of stuff I do but it felt like making it more difficult knowingly.

@jdemeyer
Copy link
Contributor Author

comment:89

How did you build sage? i.e. which shell commands did you execute, starting from the Sage source tarball, to get the totally-non-working Sage?

@jdemeyer
Copy link
Contributor Author

comment:90

Also: precisely which alpha version of Sage did you use? There have been various changes to the build process in the 4.8 series.

@jhpalmieri
Copy link
Member

comment:91

I tried the following:

  • build Sage ('make'), version 4.8.alpha2
  • move the Sage installation and run 'sage' once, as per the multi-user installation instructions
  • chmod a-w -R sage-x.y.z (so I don't have write access anymore)
  • sage -tp 18 devel/sage

Almost all tests passed; the only failures were the ones reported at #5515.

The following tests failed:

	sage -t --long devel/sage/sage/modular/hecke/submodule.py # 1 doctests failed
	sage -t --long devel/sage/sage/modular/abvar/abvar.py # 1 doctests failed
	sage -t --long devel/sage/sage/lfunctions/sympow.py # 13 doctests failed
	sage -t --long devel/sage/sage/interfaces/qepcad.py # 3 doctests failed
	sage -t --long devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 17 doctests failed

So I think the approach on this ticket should work with a multi-user installation, but as I said earlier, I may be misunderstanding the problem.

@kiwifb
Copy link
Member

kiwifb commented Nov 25, 2011

comment:92

Sorry not to reply immediately Jeroen but that last message was the last thing I did before getting some sleep.

Which helped clear my mind a little and take on board some comments properly. I, and am pretty sure that Mandriva does too, bypass the normal sage build system in our packages. You may have noticed the word ebuild (gentoo). So I didn't notice that you basically generated a file that was essential to start sage at the very end of the building process and outside of any spkg (that's a very crucial bit if you are packaging sage). I guess now that I have noticed and realized it is supposed to be there I could just add it and adjust the location.

My real mistake was to think it was created each time sage was launched - never mind that there is no process to remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants