#14480: switch sage to the new directory layout
-----------------------------------------+-------------------------------
Reporter: ohanar | Owner: tbd
Type: task | Status: needs_info
Priority: major | Milestone: sage-6.0
Component: distribution | Resolution:
Keywords: | Merged in:
Authors: R. Andrew Ohana | Reviewers:
Report Upstream: N/A | Work issues:
Branch: u/ohanar/build_system | Dependencies: #13015, #14781
Stopgaps: |
-----------------------------------------+-------------------------------
Changes (by tkluck):
* status: needs_review => needs_info
Comment:
The following is my review for the changes between master and
u/ohanar/build_system. I'm assuming that there are no other changes
apart from the directory structure between the old hg repositories
and master.
Overall, I'm impressed by the amount of work this has been.
My review contains mostly questions that you can probably just answer
or address with minor patches.
Maybe it makes sense for at least one more person to go through this diff.
{{{
diff --git a/Makefile b/Makefile
doc-clean:
- @echo "Deleting devel/sage/doc/output..."
- rm -rf devel/sage/doc/output
+ @echo "Deleting generated docs..."
+ rm -rf src/doc/en/reference/*/sage
+ rm -rf src/doc/en/reference/*/sagenb
+ rm -rf src/doc/en/reference/sage
+ rm -rf src/doc/en/reference/sagenb
+ rm -rf src/doc/output
}}}
why do we clean more directories than before?
{{{
+
+bdist-clean: clean
+ @echo "Deleting miscellaneous artifacts generated by build system
..."
rm -rf logs
- rm -f spkg/parallel_make.cfg
rm -rf dist
}}}
What happened to this parallel_make.cfg file?
Do I understand correctly that, and is it okay that the micro_release code
now
only strips the binaries and does not remove build directories?
{{{
diff --git a/build/deps b/build/deps
new file mode 100644
index 0000000..2d2d19c
--- /dev/null
+++ b/build/deps
@@ -0,0 +1,523 @@
+###############################################################################
+# This file ($SAGE_ROOT/spkg/standard/deps) will be copied into
+# $SAGE_ROOT/spkg/Makefile by $SAGE_ROOT/spkg/install
+###############################################################################
}}}
There's still a reference to the spkg directory here.
{{{
diff --git a/build/gen_html b/build/gen_html
deleted file mode 100755
index 6dbad06..0000000
--- a/build/gen_html
+++ /dev/null
}}}
What did this script do? Why don't we need it anymore?
{{{
diff --git a/build/install b/build/install
index 780fe2a..bb14f76 100755
--- a/build/install
+++ b/build/install
@@ -99,11 +95,6 @@ if [ -d "$SAGE_ROOT/data" ] && [ ! -h "$SAGE_ROOT/data"
]; then
rm -rf "$SAGE_ROOT/data"
fi
-# Symlink old SAGE_DATA to SAGE_SHARE for optional/experimental
-# packages that haven't yet been updated.
-rm -f "$SAGE_ROOT/data"
-ln -s local/share "$SAGE_ROOT/data"
-
}}}
Should we make sure that this does not break any optional/experimental
packages?
Personally, I'm fine with any package that installs anything into this
data directory,
which I'm guessing has been deprecated for some time now. (how long?)
{{{
@@ -352,25 +343,17 @@ newest_version_base() {
}
# Usage: newest_version $pkg
-# Print version number of latest (according to modification time)
-# standard or optional package $pkg
+# Print version number of latest standard package $pkg
newest_version() {
<SNIP>
}}}
This reminds me: will we, at some point, test the installation of the
optional packages into the git framework? And, since we're already
distributing
them officially, couldn't we just as well put their metadata in the
repository, too?
(We don't need to do that right away).
{{{
+# glpk's build system uses the output of grep
+unset GREP_OPTIONS
+
}}}
This change has been made in quite some places. Why wasn't GREP_OPTIONS
being set
a problem before?
{{{
diff --git a/sage b/sage
index 6597c54..46c69ba 100755
--- a/sage
+++ b/sage
@@ -124,11 +124,10 @@ fi
export SAGE_ROOT
# Run the actual Sage script
-if [ -x "$SAGE_ROOT/spkg/bin/sage" ]; then
- "$SAGE_ROOT/spkg/bin/sage" "$@"
-elif [ -x "$SAGE_ROOT/local/bin/sage-sage" ]; then # Support sage-4.x
- export CUR=`pwd`
- "$SAGE_ROOT/local/bin/sage-sage" "$@"
+if [ -x "$SAGE_ROOT/src/bin/sage" ]; then
+ "$SAGE_ROOT/src/bin/sage" "$@"
+elif [ -x "$SAGE_ROOT/local/bin/sage" ]; then # if in a striped binary
+ "$SAGE_ROOT/local/bin/sage" "$@"
else
echo >&2 "$0: no Sage installation found in \$SAGE_ROOT=$SAGE_ROOT"
exit 1
}}}
It feels wrong to me to prefer the src/bin version of the sage script to
the local/bin version. Why is this?
{{{
diff --git a/src/bin/sage b/src/bin/sage
index 5c886e3..3762d5d 100755
--- a/src/bin/sage
+++ b/src/bin/sage
<SNIP>
@@ -864,12 +825,8 @@ if [ "$1" = '-pkg_nc' -o "$1" = "--pkg_nc" ]; then
fi
if [ "$1" = '-sdist' -o "$1" = "--sdist" ]; then
- if [ $# -ne 2 ]; then
- echo >&2 "** MISSING VERSION NUMBER! **"
- exit 2
- fi
- maybe_sage_location
- exec sage-sdist $2 "$SAGE_ROOT"
+ shift
+ exec sage-sdist "$@"
}}}
Why doesn't this need sage location anymore?
@@ -904,25 +861,7 @@ fi
{{{
if [ "$1" = '-upgrade' -o "$1" = "--upgrade" ]; then
shift
- cd "$SAGE_ROOT"
-
- # 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-location || exit $?
-
- # Run sage-upgrade twice since when installing sage-scripts and a
- # running script changes, it gets confused and exits with an error.
- # Running again (with the script replaced) then fixes the problem.
sage-upgrade "$@"
- if [ $? -eq 2 ]; then # this exit code means the user elected not
to do the upgrade at all.
- exit 2
- fi
- echo "Double checking that all packages have been installed."
- sage-upgrade "$@" || exit $?
-
- # Check that Sage still works
- sage-starts
exit $?
fi
}}}
Why don't we call sage-location anymore? and sage-starts?
{{{
diff --git a/src/bin/sage-bdist b/src/bin/sage-bdist
index a776900..c765017 100755
--- a/src/bin/sage-bdist
+++ b/src/bin/sage-bdist
<SNIP>
+# Clone Sage repository
+echo "Cloning Sage repository..."
+git clone "$SAGE_ROOT" "$TMP_DIR/$TARGET"
+( cd "$TMP_DIR/$TARGET" && git remote set-url origin
git://github.com/sagemath/sage.git )
}}}
We need to set this to the trac server. Also, what branch does this clone?
It
should probably be the current one, because those are the binaries it
copies.
Is it?
{{{
diff --git a/src/bin/sage-download-file b/src/bin/sage-download-file
index cf7dce0..a3428df 100755
--- a/src/bin/sage-download-file
+++ b/src/bin/sage-download-file
@@ -33,7 +33,7 @@ if [ -z "$URL_GRABBER" ]; then
if [ -n "$SAGE_LOCAL" ] && [ -x "$SAGE_LOCAL/bin/python" ]; then
URL_GRABBER="$SAGE_LOCAL/bin/python"
elif command -v curl &>/dev/null; then
- URL_GRABBER="curl"
+ URL_GRABBER="curl --fail"
elif command -v wget &>/dev/null; then
URL_GRABBER="wget -nv -O-"
# Pick Python last because we don't know which version it is,
}}}
This looks fine, but why wasn't it an issue before?
{{{
diff --git a/src/bin/sage-env b/src/bin/sage-env
index 6c2408a..5e5af1b 100644
--- a/src/bin/sage-env
+++ b/src/bin/sage-env
<SNIP>
# Make Mercurial not use any user defaults (like enabling the pager
extension)
# This is needed in order to use mercurial in scripts. When running hg
-# directly through "sage -hg", we will disable HGPLAIN in spkg/bin/sage.
+# directly through "sage -hg", we will disable HGPLAIN in src/bin/sage.
# See Trac Ticket #12058
HGPLAIN="yes" && export HGPLAIN
}}}
This can probably go.
{{{
+if [ -z "$SAGE_REPO" ]; then
+ SAGE_REPO="git://github.com/sagemath/sage.git"
+ export SAGE_REPO
+fi
}}}
This should be set to the trac server.
{{{
diff --git a/src/bin/sage-spkg b/src/bin/sage-spkg
index 20c56db..86c03c3 100755
--- a/src/bin/sage-spkg
+++ b/src/bin/sage-spkg
}}}
This file has had major modifications so it needs updated author credits.
Also,
the comment at the start still explains its old functionality.
{{{
+DOWNLOAD_ONLY=0
+if [ "$1" = '-d' ]; then
+ DOWNLOAD_ONLY=1
+ shift
+fi
+
}}}
I think this is an odd way to change the functionality. Maybe download-
only
should be in a script of itself.
{{{
diff --git a/src/bin/sage-sync-build.py b/src/bin/sage-sync-build.py
index 8e45386..b5d476a 100755
--- a/src/bin/sage-sync-build.py
+++ b/src/bin/sage-sync-build.py
<SNIP>
}}}
Why does sage-sync-build need so many changes? Naively, it would need only
a few
changes to some directory names.
{{{
diff --git a/src/bin/sage-upgrade b/src/bin/sage-upgrade
index 46c1935..55432d1 100755
--- a/src/bin/sage-upgrade
+++ b/src/bin/sage-upgrade
-./install
+if [ "$#" -gt 0 ]; then
+ BRANCH="$1"
+ shift
+else
+ BRANCH=$(git symbolic-ref HEAD) ||
+ BRANCH=$(git describe --exact-match --tags HEAD | \
+ sed -e 's;^;refs/tags/;')
+ if [ -n "$BRANCH" ]; then
+ case $BRANCH in
+ refs/heads/master) BRANCH=master;;
+ refs/heads/release) BRANCH=release;;
+ refs/heads/beta) BRANCH=beta;;
+ refs/tags/*) BRANCH=release;;
+ esac
+ fi
+fi
}}}
Maybe do a `git fetch` first? Also, should we add pushing to these
branches
in the repo to a script somewhere? Maybe to sage-sdist?
{{{
diff --git a/src/c_lib/SConstruct b/src/c_lib/SConstruct
index fd4719a..1813de8 100644
--- a/src/c_lib/SConstruct
+++ b/src/c_lib/SConstruct
-#Here we only copy the files over if we are on Cygwin. Otherwise, the
-#library will be handled by the symlinks created in
-#$SAGE_ROOT/devel/sage/spkg-install
-if os.environ['UNAME'] == 'CYGWIN':
- env.Alias("install", "$SAGE_LOCAL/lib")
-else:
- env.Alias("install", [lib])
-
+env.Alias("install", "$SAGE_LOCAL")
}}}
Will this still work on Cygwin?
{{{
diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py
index 9f0bbe3..12e8434 100644
--- a/src/sage/misc/sagedoc.py
+++ b/src/sage/misc/sagedoc.py
@@ -407,9 +407,9 @@ def format(s, embedded=False):
EXAMPLES::
sage: from sage.misc.sagedoc import format
- sage: identity_matrix(2).rook_vector.__doc__[115:184]
+ sage: identity_matrix(2).rook_vector.__doc__[107:176]
'Let `A` be a general `m` by `n`\n (0,1)-matrix with `m
\\le n`. '
- sage: format(identity_matrix(2).rook_vector.__doc__[115:184])
+ sage: format(identity_matrix(2).rook_vector.__doc__[107:176])
'Let A be a general m by n\n (0,1)-matrix with m <= n.\n'
If the first line of the string is 'nodetex', remove 'nodetex' but
}}}
why did this change? Is this related to the removal of whitespace? If so:
okay.
{{{
diff --git a/src/sage/tests/cmdline.py b/src/sage/tests/cmdline.py
index 342c3ff..7e45f0f 100644
--- a/src/sage/tests/cmdline.py
+++ b/src/sage/tests/cmdline.py
@@ -208,15 +198,9 @@ def test_executable(args, input="", timeout=100.0,
**kwds):
Test ``sage --info [packages]``, unless this is a binary (bdist)
distribution which doesn't ship spkgs::
- sage: if os.path.isfile(os.path.join(SAGE_ROOT, 'spkg',
'standard', '.from_bdist')):
- ... out = "Found package sqlalchemy in
spkg/standard/sqlalchemy-...spkg\n= SQLAlchemy =\n...\nSQLAlchemy is the
Python SQL toolkit..."
- ... err = ''
- ... ret = 0
- ... else:
- ... (out, err, ret) = test_executable(["sage", "--info",
"sqlalchemy"])
- ...
+ sage: out, err, ret = test_executable(["sage", "--info",
"sqlalchemy"])
sage: print out
- Found package sqlalchemy in spkg/standard/sqlalchemy-...spkg
+ Using local scripts to install sqlalchemy-...
= SQLAlchemy =
...
SQLAlchemy is the Python SQL toolkit...
}}}
Do I understand correctly that we install sqlalchemy when running
doctests? What is
happening here?
{{{
diff --git a/src/spkg-delauto b/src/spkg-delauto
deleted file mode 100755
index 61d071d..0000000
--- a/src/spkg-delauto
+++ /dev/null
}}}
Why don't we need this anymore?
--
Ticket URL: <http://trac.sagemath.org/ticket/14480#comment:12>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
