Ticket #10494: 10494_reviewer.patch

sdist

@@ -1 +1 @@
 #!/usr/bin/env bash
 
-# Just curious: *Which* 'sage' is supposed to be called here?
-# Should we check if SAGE_LOCAL or SAGE_ROOT is defined?
-# ... and perhaps call $SAGE_ROOT/sage or $SAGE_LOCAL/bin/...? -leif
-
-# At least check that 'sage' is in the path to specifically catch
-# one potential error [earlier, with an appropriate error message]:
-if ! command -v sage >/dev/null; then
+# Check whether "sage" can be run (i.e. whether it's in the PATH)
+if ! sage -v >/dev/null; then
     echo >&2 "Error: $0 requires 'sage' to be in your PATH"
     echo >&2 "Maybe first call 'SAGE_ROOT/sage -sh'"
     exit 1
 fi
 
 echo "Trying to commit changes (if any)..."
-sage -hg ci
+sage -hg commit
 status=$?
 if [ $status -eq 1 ]; then
-    # 'hg ci' returns 1 (!) if there are no changes to commit...
+    # 'hg commit' returns 1 if there are no changes to commit...
     echo "No changes to commit (not an error)"
 elif [ $status -ne 0 ]; then
     echo >&2 "Error: $0: Couldn't commit changes" \
-             "('hg ci' failed with exit code $status)"
+             "('hg commit' failed with exit code $status)"
     exit 1
 else
     # $status == 0
@@ -39 +34 @@
 echo "* If this is an official SageNB release, don't forget to *"
 echo "* push the changes to the public Mercurial repository.   *"
 echo "**********************************************************"
-#./push

spkg-dist

@@ -1 +1 @@
 #!/usr/bin/env python
 # Create a new Sage Notebook spkg.
-
+#
 # Since the 'sdist' shell script called below requires (at least) 'sage'
 # to be in the path, we could equally check *here* if it is; SAGE_ROOT
 # or SAGE_LOCAL don't have to be set, since 'sdist' calls 'sage -hg ...'
 # and 'sage -python ...'.
 # (We also later run 'sage -pkg ...' from *this* script.)
 #
-# We *might* get import errors earlier if some system-wide Python executes
-# this script (which is probably not intended), so we could really check
-# here first if SAGE_LOCAL is defined, which is a strong indication that
-# we're actually using Sage's Python. -leif
+# This spkg-dist doesn't need any Sage components, it is fine to run
+# it from a system-wide Python. -- Jeroen Demeyer
 
 import os
 import sys
@@ -29 +27 @@
 except IndexError:
     sys.stderr.write("Error: Found no version string in 'setup.py'\n")
     sys.exit(1)
+
 i = version_line.find("'")
 j = version_line.rfind("'")
 if i==-1 or j==-1 or j-i<3:
     sys.stderr.write(
-      "Error: Illegal version string in 'setup.py':\n    %s\n" % version_line)
+        "Error: Illegal version string in 'setup.py':\n    %s\n" % version_line)
     sys.stderr.write("Perhaps no single quotes used?\n")
     sys.exit(1)
 version = version_line[i + 1:j]
@@ -44 +43 @@
 sys.stdout.flush()
 proc = subprocess.Popen([os.path.join(os.path.curdir, 'sdist')], shell=True)
 if proc.wait():
-    # The previous error message was most probably inadequate, since
-    # 'sdist' tries to commit any changes, and 'hg ci' returns 1 (!) (which
-    # usually indicates an error) if there are *no changes to commit*.
-    # The other potential error in 'sdist' was that 'python setup.py sdist'
-    # failed, in which case the error message would have also been wrong.
-    #print "Uncommitted changes in repository.  Stopping."
     sys.stderr.write(
-      "Error: './sdist' failed. See message(s) above for the specific error.\n")
+        "Error: './sdist' failed. See message(s) above for the specific error.\n")
     sys.exit(1)
 
 # Create the spkg.
@@ -179 +172 @@
 fi
 
 cd "$SAGE_ROOT/local/lib/python/site-packages"
-# Dave says Solaris' non-POSIX grep in the default path
-# doesn't understand "-q" (which *is* POSIX):
+# Use >/dev/null instead of grep -q (which doesn't work on Solaris)
 if ! grep sagenb easy-install.pth >/dev/null; then
     # Ugly work-around, we haven't found the real cause yet (see #10176):
     echo "No sagenb path found in 'easy-install.pth'"'!'
@@ -198 +190 @@
         exit 1
     fi
 fi
-if true; then # Aids debugging (cf. #10176)
-    echo "Old path: \"`grep sagenb easy-install.pth`\""
-    echo "New path: \"`grep sagenb easy-install.pth.$$`\""
-fi
+
+# Print paths for debugging
+echo "Old path: '`grep sagenb easy-install.pth`'"
+echo "New path: '`grep sagenb easy-install.pth.$$`'"
+
 # The following fails only on wrong file permissions etc.:
 mv -f easy-install.pth.$$ easy-install.pth
 if [ $? -ne 0 ]; then
@@ -216 +209 @@
 # We are still in dist/, now package sagenb-x.y.z/ :
 print "Running 'sage -pkg %s'..."%base
 sys.stdout.flush()
-subprocess.call(['sage -pkg ' + base], shell=True)
-# XXX Perhaps also check exit code here... (but 'sage -pkg' is verbose)
-
+if subprocess.call(['sage', '-pkg', base]):
+    print "sage -pkg %s failed. See message(s) above for the specific error.\n"%base
+    sys.exit(1)