#11926 closed defect (fixed)
"make" should run Sage once
Reported by:  jdemeyer  Owned by:  GeorgSWeber 

Priority:  major  Milestone:  sage4.8 
Component:  build  Keywords:  Makefile build sagestarts 
Cc:  pcpa  Merged in:  sage4.8.alpha0 
Authors:  Jeroen Demeyer  Reviewers:  John Palmieri, Leif Leonhardy 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
In a multiuser environment, the user compiling Sage must run it at least once to run sagelocation
and generate .pyc
files.
The proposed fix is: in the default make
rule, run Sage when local/bin/sagestarted.txt
does not exist and create that file in sagelocation
. 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 pari2.4.3.alpha.p7 ************************************************************************ Please email sagedevel (http://groups.google.com/group/sagedevel) explaining the problem and including the relevant part of the log file /usr/local/src/sage4.7.2.rc0/spkg/logs/pari2.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/sage4.7.2.rc0/spkg/build/pari2.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/sage4.7.2.rc0/spkg/build/pari2.4.3.alpha.p7' && '/usr/local/src/sage4.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:
 11926.patch to SAGE_ROOT
 11926_sage_starts.patch, trac_11926errormsg.patch, 11926errormsgreview.patch to SCRIPTS (
local/bin
)  11926_sage.patch and 11926_doc.patch to the Sage library.
Attachments (6)
Change History (98)
comment:1 Changed 9 years ago by
 Description modified (diff)
comment:2 Changed 9 years ago by
 Status changed from new to needs_review
comment:3 followups: ↓ 4 ↓ 11 Changed 9 years ago by
comment:4 in reply to: ↑ 3 Changed 9 years ago by
 Status changed from needs_review to needs_work
 Work issues set to temporary DOTSAGE
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.
comment:5 followup: ↓ 6 Changed 9 years ago by
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 sagelocation
from there if make
did anything, but we should in principle run it from elsewhere.
Someone who is going to install Sage systemwide 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 sagelocation
) should be run (at least) after it has been moved to its final installation location, which is likely to happen after having built Sage.
comment:6 in reply to: ↑ 5 Changed 9 years ago by
Replying to leif:
I don't like the idea of running
sage
aftermake 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 theMakefile
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 toplevel make all
or make build
.
Or run
sagelocation
from there ifmake
did anything, but we should in principle run it from elsewhere.
Running sagelocation
is certainly not sufficient to generate all files needed to run Sage.
Someone who is going to install Sage systemwide 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
(orsagelocation
) 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.
comment:7 Changed 9 years ago by
 Status changed from needs_work to needs_review
 Work issues temporary DOTSAGE deleted
Updated version of the patch using a temporary DOT_SAGE
directory. I have not really tested it well.
comment:8 Changed 9 years ago by
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.
comment:9 followup: ↓ 10 Changed 9 years ago by
I don't see anything in the new patch about DOT_SAGE
.
comment:10 in reply to: ↑ 9 Changed 9 years ago by
Replying to jhpalmieri:
I don't see anything in the new patch about
DOT_SAGE
.
So true! I seems I forget a hg qrefresh
.
comment:11 in reply to: ↑ 3 Changed 9 years ago by
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).
comment:12 Changed 9 years ago by
 Description modified (diff)
comment:13 followup: ↓ 14 Changed 9 years ago by
I'm somewhat tempted to add a new commandline option to Sage:

sagesage
diff git a/sagesage b/sagesage
a b if [ $# gt 0 ]; then 247 247 fi 248 248 fi 249 249 250 # The following function creates a temporary file and stores its name 251 # in $FILE. (It would be nice to replace this with 'mktemp', but that 252 # command has different syntax on OS X compared to linux or Solaris.) 253 sagetempfile() { 254 FILE=`mktemp d 2>/dev/null` 255 if [ $? ne 0 ]; then 256 # presumably because the "d" option to mktemp expects an argument, 257 # e.g., on OS X. 258 FILE=`mktemp d t dotsage` 259 fi 260 } 261 262 if [ "$1" = 'norc' o "$1" = 'nodotsage' ]; then 263 OLD_DOT_SAGE=$DOT_SAGE 264 sagetempfile 265 DOT_SAGE=$FILE && export DOT_SAGE 266 shift 267 sage "$@" 268 rm rf "$DOT_SAGE" 269 DOT_SAGE=$OLD_DOT_SAGE && export DOT_SAGE 270 fi 250 271 251 272 LOGFILE="$SAGE_ROOT/sage.log" 252 273 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.
comment:14 in reply to: ↑ 13 Changed 9 years ago by
 Status changed from needs_review to needs_work
Replying to jhpalmieri:
I'm somewhat tempted to add a new commandline 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.
comment:15 Changed 9 years ago by
 Status changed from needs_work to needs_review
comment:16 followups: ↓ 23 ↓ 25 Changed 9 years ago by
s/succesfully/successfully/
Still, running sage
(especially the full bunch of shell scripts) there is quite dangerous, as almost everything can happen. Worse, there's no "DO NOT INTERRUPT THIS"*, and error messages aren't even logged.
Better write some install
target (and / or a sagefirst_time
script), and update the Sage Installation Guide, especially with respect to files that are created upon first startup, hardcoded path etc. etc.
IMHO we should in general do certain things upon installing Sage (some actually during the build), rather than checking them and doing them for the first time when someone tries to start Sage.
 This message is btw. printed too late, as
install_moved()
already does some stuff before the script (eventually!) gets to this.
P.S.: No serious sysadmin builds in, say, /usr/local/
, or leaves an "installation" in /tmp/
.
comment:17 Changed 9 years ago by
s/does not run/failed to start up/.
comment:18 followup: ↓ 24 Changed 9 years ago by
Also, upon upgrading the message is useless (at least in the first place), as a second attempt to upgrade / build Sage is made.
comment:19 followup: ↓ 20 Changed 9 years ago by
Instead of running Sage, you could run sagelocation
and sagepython c 'from sage.all import *'
, along with printing messages like "Generating necessary pyc files; do not interrupt". This should generate the pyc files and the other files in local/lib, right? This avoids the shell script sagesage, but is there any real advantage to doing this instead of just running sage
?
comment:20 in reply to: ↑ 19 Changed 9 years ago by
Replying to jhpalmieri:
Instead of running Sage, you could run
sagelocation
andsagepython c 'from sage.all import *'
, along with printing messages like "Generating necessary pyc files; do not interrupt". This should generate the pyc files and the other files in local/lib, right? This avoids the shell script sagesage, but is there any real advantage to doing this instead of just runningsage
?
Not much I think. But we should really separate the things that have to (or should better) be done before running Sage the first time. Compiling Python files does IMHO not belong to this, and executing from sage.all import *
can again do almost everything (or lead to arbitrary constellations if the build is broken), and is hardly guaranteed to at all exit by itself.
Creating a (fully working) "readonly" installation (from the perspective of an arbitrary user) is certainly not something that belongs to a regular build; you'd do such if you intend to install Sage systemwide for example of course, hence there should be some (separate) target for this. Such target could be the default target (all
), although I think it shouldn't, at least not unless there's a (welldocumented) way to specify the destination in advance, during some manually run configure
for example.
comment:21 followup: ↓ 22 Changed 9 years ago by
On one hand, there is something attractive about building and installing Sage by running (perhaps) "configure", then "make", then "make install". However, this would require some work, and also a potential change of behavior for most Sage developers. This sounds like a possibility, but a longterm one. I don't see it happening any time soon, anyway. Jeroen's suggestion here, on the other hand, is a quick solution for a problem with Sage now. I suggest we merge this now (once the typos are fixed, along with any other simple fixes), and consider a longer term solution separately.
I wouldn't object if this were implemented by adding a new target in the Makefile, but I think it should be run by default to address the issues Jeroen has raised here. I also wouldn't object if it were implemented as in the current patch, perhaps with a followup ticket to make it a separate target for make.
comment:22 in reply to: ↑ 21 Changed 9 years ago by
Replying to jhpalmieri:
Jeroen's suggestion here, on the other hand, is a quick solution for a problem with Sage now.
I don't see any problem, except that the current behaviour of Sage regarding onthefly creation or modification of files in the "static" installation tree is suboptimal and  IMHO worse  AFAIK not documented, at least not very well.
And the "problem" is all but new.
Sysadmins  at least those that have installed Sage before (others will quickly notice)  are likely to be aware of that; most other users don't care at all, so this is rather a nonissue.
Adding running sage
at the end of every (perhaps partial re) build is not the right way to solve this; in contrast, it is likely to obscure errors that don't show up during the build but when you try to start Sage, in addition with the potential to make the build hang (without any output).
So the minimal things to do are  if you really want to run Sage there:
 Print a message that we're attempting to run
sage
.  Disable keyboard interrupts during important operations (
sagelocation
, maybe elsewhere, too), and issue a warning that this process must not be interrupted, at least for a few minutes.  Perhaps set a reasonably long timeout for running
sage
.  Log the output instead of redirecting it to
/dev/null
, such that we can print at least a reference to the error log in case of failures.
I suggest we merge this now (once the typos are fixed, along with any other simple fixes), and consider a longer term solution separately.
There are various possible longterm solutions; the primary goal should be to document potential issues, i.e., at least prominently state that Sage should be run at least once at its final location for use in a multiuser environment, or more precisely, for a systemwide installation.
I wouldn't object if this were implemented by adding a new target in the Makefile, but I think it should be run by default to address the issues Jeroen has raised here.
An echo
statement at the end of the build, perhaps in the Makefile
, would IMHO fully suffice for the moment; no need to hurry or for "quick [and dirty] fixes" (which btw. often cause more trouble than they solve).
Have there been any "error" reports regarding this? Changing the permissions of all files in a couple of spkgs wasn't necessary either.
Note that "ordinary" users won't be able to install / upgrade spkgs anyway, so the pkgconfig
and .la
files for example aren't an issue in a systemwide installation, nor will such users be able to move Sage, unless they copy the whole installation (in which case they're likely to gain ownership, or at least can change the owner, such that they'll have write permission on the copied installation).
comment:23 in reply to: ↑ 16 ; followup: ↓ 26 Changed 9 years ago by
Replying to leif:
Still, running
sage
(especially the full bunch of shell scripts) there is quite dangerous, as almost everything can happen.
What's your point really? Whoever built Sage will have to run it sooner or later, otherwise it's quite pointless building Sage. So the danger is there anyway.
comment:24 in reply to: ↑ 18 ; followup: ↓ 27 Changed 9 years ago by
Replying to leif:
Also, upon upgrading the message is useless (at least in the first place), as a second attempt to upgrade / build Sage is made.
What do you mean by "a second attempt to upgrade / build Sage is made"?
comment:25 in reply to: ↑ 16 Changed 9 years ago by
Replying to leif:
Better write some
install
target (and / or asagefirst_time
script), and update the Sage Installation Guide, especially with respect to files that are created upon first startup, hardcoded path etc. etc.IMHO we should in general do certain things upon installing Sage (some actually during the build), rather than checking them and doing them for the first time when someone tries to start Sage.
In principle you are of course right but Sage is quite different from other software. It currently has no make install
rule, and it is not at all clear to me what make install
should do anyway. Also, Sage is special in that the source code is really in the installation. Normally, you would have a source tree and a build tree, but with Sage this is one. Sage developers are used to this, so people might even complain if you try to change this. So for this ticket, let's not overhaul the whole Sage build process, let's stick to the simple solution.
 This message is btw. printed too late, as
install_moved()
already does some stuff before the script (eventually!) gets to this.
See #5155.
Leif: would you agree with the following: add a make install
rule in Sage which simply runs Sage (and hence indirectly sagelocation
)? And then not run Sage from spkg/install
.
comment:26 in reply to: ↑ 23 Changed 9 years ago by
Replying to jdemeyer:
Replying to leif:
Still, running
sage
(especially the full bunch of shell scripts) there is quite dangerous, as almost everything can happen.Whoever built Sage will have to run it sooner or later, otherwise it's quite pointless building Sage. So the danger is there anyway.
:D I didn't know that. I only build Sage, but have never run it.
Seriously, the difference is whether a user intentionally runs sage
(for the first time after a build / upgrade / extraction of a binary distribution), and sees what's happening. Your new patch omits the redirection to /dev/null
, but I think it would be better to log the output to something like $SAGE_ROOT/first_sage_startup`date +"%Y%m%d%H:%M"`.log
, which we could delete afterwards if apparently nothing went wrong, otherwise point the user to this file, e.g. for bug reports (also to sagerelease btw.).
We have enough "error reports" where people claim something didn't work but couldn't provide error logs.
I'd also prefer running Sage from the Makefile
; rather than changing the build
target, we could modify the default target to run sage
once after a successful build (or after building the documentation, but that's likely to fail in "weird" ways if sage
itself doesn't work properly), perhaps just if no (recent) "first run" logfile exists.
There we should IMHO separate what's currently implicitly performed by sagelocation
, for example by adding an option to sagesage
for running just that. For upgrades, running sagelocation
afterwards is not strictly necessary; although some parts should be run by sagespkg
anyway, such that they also get run upon e.g. sage i ...
. If SAGE_UPGRADING=yes
, we could add a message to spkg/install
(or sagesage
, or sageupdate
) recommending to try to run sage
right now (provided the build succeeded). make upgrade
(and make rebuild
) are overdue anyway; if/when we drop the creation of pipestatus
, we could also change sageupdate
to just download new files and then run make rebuild
, i.e., make it use the toplevel Makefile
as well (which is now under revision control and hence subject to upgrades, too).
comment:27 in reply to: ↑ 24 Changed 9 years ago by
Replying to jdemeyer:
Replying to leif:
Also, upon upgrading the message is useless (at least in the first place), as a second attempt to upgrade / build Sage is made.
What do you mean by "a second attempt to upgrade / build Sage is made"?
if [ "$1" = 'upgrade' o "$1" = "upgrade" ]; then # People often move the Sage install right before doing the upgrade, so it's # important to fix any path hardcoding issues first, or certain library # links will fail. "$SAGE_ROOT/local/bin/"sagelocation # Do it twice since when installing sagescripts and a running # script changes, it gets confused and exits with an error. # Running again (with the script replaced) then fixes the problem. # Run from a temporary copy of sagesage shift sageupgrade "$@" if [ $? = 2 ]; then # this exit codes means the user elected not to do the upgrade at all. exit $? fi echo "Double checking that all packages have been installed." sageupgrade "$@" exit $? fi
comment:28 Changed 9 years ago by
 Description modified (diff)
comment:29 Changed 9 years ago by
Updated version, needs review (even though I have not yet tested building from scratch).
comment:30 Changed 9 years ago by
 Keywords sagestarts added
comment:31 Changed 9 years ago by
Any reviewers for this quite simple patch?
comment:32 Changed 9 years ago by
I haven't tested it out, but it looks good at first glance. If you're going to make changes to the error messages printed by sagespkg, it would be good to mention $SAGE_ROOT/spkg/logs/$PKG_NAME.log
. If people build in parallel (and I hope they do), $SAGE_ROOT/install.log
can be a real mess, and so may not be the best way to extract information. What do you think about my attached patch?
comment:33 Changed 9 years ago by
 Description modified (diff)
comment:34 followup: ↓ 35 Changed 9 years ago by
I think that in sagestarts, if Sage fails to start, you should delete SAGE_ROOT/local/bin/sageflags.txt
: this is created by sagelocation, which gets run by sage, which gets run by sagestarts. That is, the file sageflags.txt might be created successfully, then
spkg/pipestatus "./sage c \"print 'Yes, Sage starts.'\" 2>&1" "tee a start.log"
fails, and then if you run "make" again, it won't test whether Sage starts because of the presence of sageflags.txt.
Either that, or there needs to be a different test in the Makefile to decide whether to run sagestarts.
comment:35 in reply to: ↑ 34 Changed 9 years ago by
 Status changed from needs_review to needs_work
Replying to jhpalmieri:
Either that, or there needs to be a different test in the Makefile to decide whether to run sagestarts.
How about having a third file besides the existing sageflags.txt
and sagecurrentlocation.txt
? Say, a file sagestarted.txt
whose existence means Sage was run at least once successfully.
I think we could also use the nonexistence of this file as a trigger in sagelocation
to force some actions. For example, on upgrading, even if the Sage tree was not moved, it might make sense to call update_library_files()
and update_pkgconfig_files()
.
I would write the file sagestarted.txt
in sagelocation
and potentially delete it in sagestarts
(that's what you proposed for sageflags.txt
instead).
comment:36 followup: ↓ 37 Changed 9 years ago by
You could instead test whether the file starts.log ends with "Yes, Sage starts." You could test this in sagestarts rather than in the Makefile.
comment:37 in reply to: ↑ 36 Changed 9 years ago by
Replying to jhpalmieri:
You could instead test whether the file starts.log ends with "Yes, Sage starts."
Sorry, but I disagree. I think neither the existence nor contents of any logfiles should influence the runtime at all. Users should be able to do whatever they want with logfiles without it making any difference.
comment:38 Changed 9 years ago by
 Description modified (diff)
 Reviewers set to John Palmieri
 Status changed from needs_work to needs_review
comment:39 Changed 9 years ago by
New patches up for review. It uses a new file local/lib/sagestarted.txt to check whether to run sagestarts
. I also added a reviewer patch for your changed error message.
comment:40 Changed 9 years ago by
 Description modified (diff)
comment:41 Changed 9 years ago by
 Description modified (diff)
comment:42 Changed 9 years ago by
 Description modified (diff)
comment:43 Changed 9 years ago by
 Description modified (diff)
comment:44 Changed 9 years ago by
 Description modified (diff)
Changed 9 years ago by
comment:45 Changed 9 years ago by
 Description modified (diff)
comment:46 Changed 9 years ago by
 Description modified (diff)
 Summary changed from "make build" should run Sage once to "make" should run Sage once
comment:47 followup: ↓ 52 Changed 9 years ago by
For the record:
Installation in a Multiuser Environment  This section addresses the question of how a system administrator can install a single copy of Sage in a multiuser computer network. Systemwide install ~~~~~~~~~~~~~~~~~~~ #. After you build Sage, you may optionally copy or move the entire build tree to ``/usr/local``. #. There are some initial files that have to be created the first time Sage is run. Try starting up Sage once as root (or, to be more thorough, try ``make test`` as root to run all the standard test code). You can stop the tests by pressing ``ctrlz`` followed by typing ``kill %1`` (assuming you had no other jobs in the background of that shell). ...
(I.e., RTFM.)
comment:48 followup: ↓ 54 Changed 9 years ago by
The patch to spkg/install
will conflict with #11959, so should be removed from the patch. (The changes are irrelevant anyway.)
comment:49 followup: ↓ 57 Changed 9 years ago by
I don't think it's a good idea to write the "sage started" file from sagelocation
, as running sagelocation
doesn't imply that Sage will or will get run.
Btw., .pyc
wasn't a typo; while the .pyc
files don't get "regenerated" by sagelocation
, they get deleted, such that they'll indeed get regenerated the next time Sage is run.
(The .pc
files don't get regenerated either; if at all, they get updated when necessary.)
comment:50 followups: ↓ 51 ↓ 55 Changed 9 years ago by
The patch to sagespkg
is also unrelated (and of just cosmetic nature) and will just cause unnecessary merge conflicts.
comment:51 in reply to: ↑ 50 Changed 9 years ago by
comment:52 in reply to: ↑ 47 Changed 9 years ago by
comment:53 Changed 9 years ago by
 Description modified (diff)
comment:54 in reply to: ↑ 48 Changed 9 years ago by
comment:55 in reply to: ↑ 50 Changed 9 years ago by
Replying to leif:
The patch to
sagespkg
is also unrelated (and of just cosmetic nature) and will just cause unnecessary merge conflicts.
I agree it is unrelated and of cosmetic nature but it improves the error messages which is quite important I think. And #11021 is a ticket which has no activity since 3 months so I don't think moving it there is such a good solution.
comment:56 Changed 9 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
comment:57 in reply to: ↑ 49 Changed 9 years ago by
 Status changed from needs_work to needs_review
Replying to leif:
I don't think it's a good idea to write the "sage started" file from
sagelocation
, as runningsagelocation
doesn't imply that Sage will or will get run.
Okay, moved this to sage/all.py
, surely a better place. Needs review.
comment:58 followup: ↓ 59 Changed 9 years ago by
If someone runs 'make build' instead of 'make' or 'make start', is there a good way to let them know that they should probably run Sage once to initialize various files, or should we just hope that they've read enough of README.txt or the installation guide?
comment:59 in reply to: ↑ 58 ; followup: ↓ 63 Changed 9 years ago by
Replying to jhpalmieri:
If someone runs 'make build' instead of 'make' or 'make start', is there a good way to let them know that they should probably run Sage once to initialize various files, or should we just hope that they've read enough of README.txt or the installation guide?
Well, we could print a message after make build
is finished, but this will also be printed when doing a simple make
and might confuse users. So I would rather not print anything and assume that users who know about make build
also know what they are doing.
comment:60 followup: ↓ 61 Changed 9 years ago by
 Reviewers changed from John Palmieri to John Palmieri, Leif Leonhardy
 Status changed from needs_review to positive_review
I think this looks good. There are a few small cosmetic changes; if you want to make them, no need for rereview.
 In 11926_sage.patch, line 859, I would change "such that" to "so that".
 After having rewritten the error message in trac_11926errormsg.patch myself, I now think it would look better to add blank lines above and below line 56, the name of the log file. (That's line 59 in 11926errormsgreview.patch). The same might go for line 65 (in the review patch), the commands to try to debug the error, but it's not as important there.
Changed 9 years ago by
comment:61 in reply to: ↑ 60 Changed 9 years ago by
Replying to jhpalmieri:
In 11926_sage.patch, line 859, I would change "such that" to "so that".
Done
After having rewritten the error message in trac_11926errormsg.patch myself, I now think it would look better to add blank lines above and below line 56, the name of the log file. (That's line 59 in 11926errormsgreview.patch). The same might go for line 65 (in the review patch), the commands to try to debug the error, but it's not as important there.
Well, I think the indentation already makes them stand out. Looking at the new error message in the description of this ticket, I quite like it.
comment:62 Changed 9 years ago by
 Merged in set to sage4.7.3.alpha0
 Resolution set to fixed
 Status changed from positive_review to closed
comment:63 in reply to: ↑ 59 ; followups: ↓ 64 ↓ 65 Changed 9 years ago by
Replying to jdemeyer:
Replying to jhpalmieri:
If someone runs 'make build' instead of 'make' or 'make start', is there a good way to let them know that they should probably run Sage once to initialize various files, or should we just hope that they've read enough of README.txt or the installation guide?
Well, we could print a message after
make build
is finished, but this will also be printed when doing a simplemake
and might confuse users. So I would rather not print anything and assume that users who know aboutmake build
also know what they are doing.
ifeq ($(MAKECMDGOALS),build) RUN_SAGE_REMINDER = echo "Make sure to run Sage once before ..." endif ... build: ... ... $(RUN_SAGE_REMINDER)
IMHO this ticket still needs work.
First of all, doc
should also depend on start
[sic], to avoid attempting to run both at the same time.
Second, the output of sagestarts
still isn't logged, which is important for error reports at least.
comment:64 in reply to: ↑ 63 ; followup: ↓ 66 Changed 9 years ago by
Replying to leif:
First of all,
doc
should also depend onstart
[sic], to avoid attempting to run both at the same time.
Nothing wrong with running doc
and start
[sic] at the same time.
Second, the output of
sagestarts
still isn't logged
That's not true. It is logged in $SAGE_ROOT/start.log
, see the new local/bin/sagestarts
. If sagestarts
fails, you get an error message pointing to this logfile.
comment:65 in reply to: ↑ 63 Changed 9 years ago by
Replying to leif:
Second, the output of
sagestarts
still isn't logged, which is important for error reports at least.
Ok, I see it is in $SAGE_ROOT/start.log
. Not immediately clear from the Makefile
, and having it in install.log
as well wouldn't be bad either.
From start.log
alone (which is always appended to), it isn't clear what happened inbetween (i.e., the relevant parts of install.log
), and conversely, from install.log
one doesn't necessarily see whether running Sage succeeded. Having the date and time (UTC) there (in start.log
, but in general in install.log
, too) wouldn't be bad either.
comment:66 in reply to: ↑ 64 Changed 9 years ago by
Replying to jdemeyer:
Replying to leif:
First of all,
doc
should also depend onstart
[sic], to avoid attempting to run both at the same time.Nothing wrong with running
doc
andstart
[sic] at the same time.
Seriously? If both get run at the same time, the [screen] output is likely to get messed up (the message of sagestarts
almost certainly not being the last). But more important, it doesn't make sense to try to build the documentation if Sage doesn't start up (since it will fail at some later point), and are you 100% sure that there aren't any further race conditions between these two?
comment:67 followup: ↓ 68 Changed 9 years ago by
In the patch to sagespkg
(which is unrelated to this ticket anyway), error_msg()
prints "sage: An error occurred while testing $PKG_NAME.
regardless of $1
.
comment:68 in reply to: ↑ 67 ; followup: ↓ 71 Changed 9 years ago by
Replying to leif:
In the patch to
sagespkg
(which is unrelated to this ticket anyway),error_msg()
prints"sage: An error occurred while testing $PKG_NAME.
regardless of$1
.
I see. Fixed in yet another reviewer patch... Sorry for the noise, although the patch is still unrelated to the ticket's topic. (More precisely, I already made similar changes at / for #11021, where it IMHO belongs.)
comment:69 followup: ↓ 70 Changed 9 years ago by
W.r.t. the Installation Guide:
One shouldn't edit /usr/local/sage*/sage
(after copying it; one can of course), but /usr/local/bin/sage
, i.e., one should either edit the copy, or the original and copy it afterwards.
No need to remove the recommendation to run make test
or whatever (make ptestlong
would IMHO be more appropriate).
comment:70 in reply to: ↑ 69 Changed 9 years ago by
Replying to leif:
W.r.t. the Installation Guide:
One shouldn't edit
/usr/local/sage*/sage
(after copying it; one can of course), but/usr/local/bin/sage
, i.e., one should either edit the copy, or the original and copy it afterwards.
P.S.: One should preferably edit the copy, since the original is under revision control, so changes to that can e.g. again cause "errors" during upgrading.
comment:71 in reply to: ↑ 68 Changed 9 years ago by
Replying to leif:
I see. Fixed in yet another reviewer patch... Sorry for the noise, although the patch is still unrelated to the ticket's topic. (More precisely, I already made similar changes at / for #11021, where it IMHO belongs.)
I don't see why it belongs more to #11021 than here, but in any case, that's not a very relevant discussion. I was unaware of #11021 when I made the changes and I'm not sure whether that ticket is ready to be merged.
comment:72 Changed 9 years ago by
 Resolution fixed deleted
 Status changed from closed to new
comment:73 Changed 9 years ago by
 Status changed from new to needs_review
comment:74 Changed 9 years ago by
Added an extra patch with the documentation changes proposed by leif, needs_review.
comment:75 Changed 9 years ago by
 Description modified (diff)
comment:77 Changed 9 years ago by
 Merged in changed from sage4.7.3.alpha0 to sage4.8.alpha0
 Milestone set to sage4.8
Changed 9 years ago by
comment:78 Changed 9 years ago by
Can somebody please review the last patch here?
comment:79 followup: ↓ 80 Changed 9 years ago by
 Status changed from needs_review to positive_review
Looks okay to me.
comment:80 in reply to: ↑ 79 ; followups: ↓ 81 ↓ 82 Changed 9 years ago by
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 sagestarts
also appended to install.log
, for the reasons given earlier.
W.r.t. #11021:
Despite its current title, it involves a "major" rewrite of sagespkg
, 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 sagespkg
, 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 metaticket for changes to the Makefile
, #11622.
comment:81 in reply to: ↑ 80 Changed 9 years ago by
Replying to leif:
But still
doc
should depend onstart
, 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 sagestarts
. 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
sagespkg
, 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.
comment:82 in reply to: ↑ 80 Changed 9 years ago by
 Resolution set to fixed
 Status changed from positive_review to closed
Replying to leif:
And it would be far better to have the output of
sagestarts
also appended toinstall.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.
comment:83 Changed 9 years ago by
Replying to jdemeyer:
Replying to leif:
But still
doc
should depend onstart
, such that one immediately gets a single, meaningful error messageI don't find it logical that
doc
would depend onstart
. I still think the best solution would be to havemake build
runsagestarts
. 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.
comment:84 Changed 9 years ago by
See #11991 for what I hope is a quick a followup: sagestarts should record the time and version number.
comment:85 followup: ↓ 87 Changed 9 years ago by
 Cc pcpa added
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 sagestarted.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?
comment:86 Changed 9 years ago by
I think that sage_started.txt should be viewed as a file like sagecurrentlocation.txt: a file which is produced during the build/test process, which ordinary users don't need writeaccess 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?
comment:87 in reply to: ↑ 85 Changed 9 years ago by
Replying to fbissey:
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 sagestarted.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 systemwide install harder? In fact, this ticket was exactly supposed to make it easier to do a systemwide 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#installationinamultiuserenvironment. Of course this ticket (and probably others to be merged in sage4.8) will change this documentation, but the essence should remain the same.
comment:88 followup: ↓ 89 Changed 9 years ago by
OK. I just completed my first ebuild of a sage4.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 "sagestarted.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.
comment:89 in reply to: ↑ 88 Changed 9 years ago by
How did you build sage? i.e. which shell commands did you execute, starting from the Sage source tarball, to get the totallynonworking Sage?
comment:90 Changed 9 years ago by
Also: precisely which alpha version of Sage did you use? There have been various changes to the build process in the 4.8 series.
comment:91 Changed 9 years ago by
I tried the following:
 build Sage ('make'), version 4.8.alpha2
 move the Sage installation and run 'sage' once, as per the multiuser installation instructions
 chmod aw R sagex.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 multiuser installation, but as I said earlier, I may be misunderstanding the problem.
comment:92 Changed 9 years ago by
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.
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).