Opened 10 years ago

Closed 10 years ago

#11991 closed enhancement (fixed)

record time, version in sage-starts

Reported by: jhpalmieri Owned by: leif
Priority: minor Milestone: sage-4.8
Component: scripts Keywords:
Cc: Merged in: sage-4.8.alpha1
Authors: John Palmieri Reviewers: Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11926 Stopgaps:

Status badges

Description (last modified by leif)

The message printed by sage-starts should record the time and the version of Sage, especially since the output is now logged.


Apply trac_11991-sage-starts.v3.patch to the scripts repo.

Attachments (3)

trac_11991-sage-starts.patch (997 bytes) - added by jhpalmieri 10 years ago.
scripts repo
trac_11991-sage-starts.v2.patch (610 bytes) - added by jhpalmieri 10 years ago.
scripts repo
trac_11991-sage-starts.v3.patch (1.7 KB) - added by jhpalmieri 10 years ago.
scripts repo

Download all attachments as: .zip

Change History (21)

comment:1 Changed 10 years ago by jhpalmieri

  • Status changed from new to needs_review

Sample output (after running sage -sh):

$ ./sage-starts

Testing that Sage starts...
[2011-11-04 14:02:56] Sage Version 4.7.2.rc1, Release Date: 2011-10-27.  
Yes, Sage starts.

The last two lines are logged to start.log.

comment:2 follow-up: Changed 10 years ago by leif

  • Reviewers set to Leif Leonhardy

Ahem, what if Sage doesn't start? Then we get no date...

So you could either do something like

echo "[`date +'%Y-%m-%d %H:%M:%S'`]" \
     "`sage --version | sed -n -e '/Version/s/^[ |]\+//;s/[ |]\+$//p'`." \
    | tee -a "$SAGE_ROOT"/start.log

spkg/pipestatus "sage -c \"print 'Yes, Sage starts.'\" 2>&1" "tee -a '$SAGE_ROOT'/start.log"

or, directly,

echo "[`date +'%Y-%m-%d %H:%M:%S'`]" \
     "`sed -n -e '/Version/s/^[ |]\+//;/Version/s/[ |]\+$//p' "$SAGE_LOCAL"/bin/sage-banner`." \
    | tee -a "$SAGE_ROOT"/start.log

spkg/pipestatus "sage -c \"print 'Yes, Sage starts.'\" 2>&1" "tee -a '$SAGE_ROOT'/start.log"

(sage --version is currently broken w.r.t. the removal, so you could actually move the sed command to sage-sage.)

or use sage --python -c 'your favourite Python code' for printing the date and Sage's version (without importing sage.*).

comment:3 Changed 10 years ago by leif

Instead of sage-banner (which should have a .txt extension btw.), you could also use one of our VERSION.txt files.

comment:4 in reply to: ↑ 2 ; follow-up: Changed 10 years ago by jhpalmieri

Replying to leif:

Ahem, what if Sage doesn't start? Then we get no date...

Oops, of course you're right. Here's a new patch, using python instead of sage for the date and version. (Note that the file sage/version.py is extremely simple, so if Python functions, we should be able to import it.)

Changed 10 years ago by jhpalmieri

scripts repo

comment:5 in reply to: ↑ 4 ; follow-up: Changed 10 years ago by leif

Replying to jhpalmieri:

(Note that the file sage/version.py is extremely simple, so if Python functions, we should be able to import it.)

It is, but note that also sage/__init__.py is executed when you import sage.version.

This is not a problem right now, but may become one if Sage's import mechanics change (which is work in progress). You could perhaps modify PYTHONPATH and import version, but I haven't tried that yet*. Maybe run Sage's Python directly then (instead of ./sage --python ...).


* PYTHONPATH="devel/sage/sage" local/bin/python -c 'import version; ...' should work, both from within and outside a Sage environment.

comment:6 in reply to: ↑ 5 Changed 10 years ago by jhpalmieri

Here's another version, using VERSION.txt. This will therefore record any update information stored there.

comment:7 Changed 10 years ago by leif

How about

version=spkg/standard/VERSION.txt
echo "[`date +'%Y-%m-%d %H:%M:%S'`] `cat $version 2>/dev/null`" | tee -a start.log
...

or

version=spkg/standard/VERSION.txt
(echo -n "[`date +'%Y-%m-%d %H:%M:%S'`] "; cat $version 2>/dev/null) | tee -a start.log
...

?

comment:8 follow-up: Changed 10 years ago by jhpalmieri

Sure, that's fine.

comment:9 in reply to: ↑ 8 Changed 10 years ago by leif

Replying to jhpalmieri:

Sure, that's fine.

Well, if you prepend $SAGE_ROOT/ (which isn't necessary, since we've cd'ed to it), then you have to quote $version (with double quotes). ;-)

Changed 10 years ago by jhpalmieri

scripts repo

comment:10 Changed 10 years ago by jhpalmieri

  • Description modified (diff)

Another attempt (without $SAGE_ROOT).

comment:11 Changed 10 years ago by leif

  • Dependencies set to #11926
  • Status changed from needs_review to positive_review

Actually only depends on one part (the one to sage-starts) of one patch (ticket:11926:11926_sage_starts.patch) at #11926.

sage-starts could by the way be more robust, i.e., it shouldn't print the following if the current working directory just happens to not be $SAGE_ROOT, and SAGE_ROOT isn't set.

$ bin/sage-starts 

Testing that Sage starts...
[2011-11-05 23:15:43] 
bin/sage-starts: line 14: spkg/pipestatus: No such file or directory
Sage failed to start up.
Please email sage-devel (http://groups.google.com/group/sage-devel)
explaining the problem and send the log file
  /home/leif/Sage/sage-4.7.2-gcc-4.5.1/local/start.log
Describe your computer, operating system, etc.

(Also note the logfile's name / location. The log just contains the time stamp, and left-over files are likely to pollute repositories; only the root repository ignores *.log files. I'm not saying that this is something a user will typically run into [unless e.g. he/she includes $SAGE_LOCAL/bin into his/her PATH, which apparently some do, although they clearly shouldn't*], but one shouldn't print a message that encourages people to submit useless error reports.)


Otherwise positive review, since that's not really related to the purpose of this ticket. Feel free to fix it here (or elsewhere) though.


* We may add this to the Installation Guide (that one shouldn't put $SAGE_ROOT/local/bin/ into PATH), but IMHO most scripts there should do the "usual" sanity check, i.e. for example test whether SAGE_LOCAL is defined, since most (or at least many) depend on the Sage environment being already set up.

comment:12 follow-up: Changed 10 years ago by jhpalmieri

We could add something like this:

  • sage-starts

    diff --git a/sage-starts b/sage-starts
    a b if [ -n "$SAGE_ROOT" ]; then 
    77    cd "$SAGE_ROOT"
    88fi
    99
     10if [[ ! -x spkg/pipestatus ]]; then
     11    echo >&2 "Error: The file spkg/pipestatus was not found or is not"
     12    echo >&2 "executable, perhaps because the 'sage-starts' script was"
     13    echo >&2 "not run from the \$SAGE_ROOT directory.  Exiting."
     14    exit 1
     15fi
     16
    1017echo
    1118echo "Testing that Sage starts..."
    1219version=spkg/standard/VERSION.txt

comment:13 in reply to: ↑ 12 Changed 10 years ago by leif

Replying to jhpalmieri:

We could add something like this:

  • sage-starts

    diff --git a/sage-starts b/sage-starts
    a b if [ -n "$SAGE_ROOT" ]; then 
    77    cd "$SAGE_ROOT"
    88fi
    99
     10if [[ ! -x spkg/pipestatus ]]; then
     11    echo >&2 "Error: The file spkg/pipestatus was not found or is not"
     12    echo >&2 "executable, perhaps because the 'sage-starts' script was"
     13    echo >&2 "not run from the \$SAGE_ROOT directory.  Exiting."
     14    exit 1
     15fi
     16
    1017echo
    1118echo "Testing that Sage starts..."
    1219version=spkg/standard/VERSION.txt

I had exactly the same in mind, although I would just print: "This script has to be run from the SAGE_ROOT directory or with SAGE_ROOT properly set."

I won't mind if you add your suggestion to your patch. You could prepend `pwd`/ to spkg/pipestatus (in the message) and perhaps then put that filename on a line by its own.

comment:14 Changed 10 years ago by leif

Although we could be a bit more clever and move the test up, i.e.

if [[ -x "$SAGE_ROOT"/spkg/pipestatus ]]; then
    cd "$SAGE_ROOT"
elif [[ ! -x spkg/pipestatus ]]; then
    # Complain that either SAGE_ROOT isn't set properly (if it's non-empty)
    # or that we're [presumably] in the wrong directory (otherwise).
    exit 1
fi

comment:15 Changed 10 years ago by leif

s/Complain that either/Complain either that/, i.e.

    ...
    if [[ -z $SAGE_ROOT ]]; then
        echo >&2 "Error: This script ($0) has to be run from the SAGE_ROOT directory."
    else
        echo >&2 "Error: SAGE_ROOT(=$SAGE_ROOT) isn't properly set."
    fi
    ...

comment:16 follow-up: Changed 10 years ago by jhpalmieri

Here's version 3. If you think it's okay, please change the ticket description to refer to this patch instead of v2.

Changed 10 years ago by jhpalmieri

scripts repo

comment:17 in reply to: ↑ 16 Changed 10 years ago by leif

  • Description modified (diff)

Replying to jhpalmieri:

Here's version 3. If you think it's okay, please change the ticket description to refer to this patch instead of v2.

Done and done.

As a side-effect, even SAGE_ROOT=/foo/bar local/bin/sage-starts now works. :)

comment:18 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.