Opened 10 years ago
Closed 9 years ago
#14480 closed task (fixed)
switch sage to the new directory layout
Reported by: | ohanar | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-6.0 |
Component: | distribution | Keywords: | |
Cc: | jdemeyer, saraedum | Merged in: | |
Authors: | R. Andrew Ohana | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | u/ohanar/build_system | Commit: | cd237e726293c48288016aa2a9cc80aa75a3bd20 |
Dependencies: | #13015, #14781 | Stopgaps: |
Description (last modified by )
To switch to a unified repository we need to include some information that are currently in the spkg tarballs, which requires modifying the sage's directory structure. Since we are switching workflows at the same time, it is an ideal time to rethink the directory structure as a whole.
The proposed significant changes
spkg -> build devel/sage -> src devel/ext -> src/ext sage_scripts -> src/bin spkg/bin -> src/bin devel/sagenb -> /dev/null (is now considered a typical upstream python library) /dev/null -> build/pkgs/* (these are now the spkg repositories)
Full changes are described at http://wiki.sagemath.org/WorkflowSEP.
The script used to create the git repository is consolidate-repos.sh
, which depends on configuration.sh
, git-filter-branch
, and the fast-export
directory (all located at https://github.com/sagemath/sage-workflow). They require bash4, git 1.8.x, and python 2.7 to run (at the very least, possibly more).
Change History (55)
comment:1 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 10 years ago by
Cc: | saraedum added |
---|
comment:3 Changed 10 years ago by
Milestone: | sage-5.10 → sage-6.0 |
---|
comment:4 Changed 10 years ago by
Summary: | switch sage to the new layout → switch sage to the new directory layout |
---|
comment:5 Changed 10 years ago by
Where is the script that you used to automatically create the git repo from the hg repos? That should be reviewed too and kept for reference.
comment:6 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:7 Changed 10 years ago by
Dependencies: | #13015 → #13015, #14781 |
---|
comment:8 Changed 10 years ago by
Branch: | → u/ohanar/build_system |
---|
comment:9 follow-up: 10 Changed 10 years ago by
Status: | needs_review → needs_info |
---|
A few small issues with this branch. I am not sure which of these should actually be fixed here, so I'm marking the ticket as need_info
rather that need_work
.
- There remain references to
src/sage/misc/hg.py
(which was removed as part of 48ff645017639d70d9b4a7107ac7fd98d8a871b5) insrc/sage/doctest/control.py
.
sage -t --long src/sage/doctest/control.py ********************************************************************** File "src/sage/doctest/control.py", line 434, in sage.doctest.control.DocTestController.add_files Failed example: DC.add_files() Exception raised: Traceback (most recent call last): File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 486, in _run self.execute(example, compiled, test.globs) File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 845, in execute exec compiled in globs File "<doctest sage.doctest.control.DocTestController.add_files[10]>", line 1, in <module> DC.add_files() File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 458, in add_files from sage.misc.hg import hg_sage ImportError: No module named hg ********************************************************************** 1 item had failures: 1 of 16 in sage.doctest.control.DocTestController.add_files [162 tests, 1 failure, 1.97 s]
- I also get a few more doctest failures such as the one below due to a missing import that seems to be fixed in the current git version of sagenb.
********************************************************************** File "local/lib/python2.7/site-packages/sagenb-0.10.4-py2.7.egg/sagenb/misc/misc.py", line 44, in sagenb.misc.misc.print_open_msg Failed example: sage.server.misc.print_open_msg('localhost', 8080, True) Exception raised: Traceback (most recent call last): File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 486, in _run self.execute(example, compiled, test.globs) File "/home/marc/co/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 845, in execute exec compiled in globs File "<doctest sagenb.misc.misc.print_open_msg[0]>", line 1, in <module> sage.server.misc.print_open_msg('localhost', Integer(8080), True) AttributeError: 'module' object has no attribute 'misc' **********************************************************************
- Finally, trying to run the notebook produces the following error:
sage: notebook() The notebook files are stored in: sage_notebook.sagenb ************************************************** * * * Open your web browser to http://localhost:8080 * * * ************************************************** Executing twistd --pidfile="sage_notebook.sagenb/sagenb.pid" -ny "sage_notebook.sagenb/twistedconf.tac" Unhandled Error Traceback (most recent call last): File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/application/app.py", line 652, in run runApp(config) File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/scripts/twistd.py", line 23, in runApp _SomeApplicationRunner(config).run() File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/application/app.py", line 386, in run self.application = self.createOrGetApplication() File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/application/app.py", line 451, in createOrGetApplication application = getApplication(self.config, passphrase) --- <exception caught here> --- File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/application/app.py", line 462, in getApplication application = service.loadApplication(filename, style, passphrase) File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/application/service.py", line 405, in loadApplication application = sob.loadValueFromFile(filename, 'application', passphrase) File "/home/marc/co/sage/local/lib/python2.7/site-packages/Twisted-12.3.0-py2.7-linux-x86_64.egg/twisted/persisted/sob.py", line 210, in loadValueFromFile exec fileObj in d, d File "sage_notebook.sagenb/twistedconf.tac", line 15, in <module> import base as flask_base exceptions.ImportError: No module named base Failed to load application: No module named base --------------------------------------------------------------------------- error Traceback (most recent call last) <ipython-input-1-3728cb3d7c7d> in <module>() ----> 1 notebook() /home/marc/co/sage/local/lib/python2.7/site-packages/sagenb-0.10.4-py2.7.egg/sagenb/notebook/notebook_object.pyc in __call__(self, *args, **kwds) 221 """ 222 def __call__(self, *args, **kwds): --> 223 return self.notebook(*args, **kwds) 224 225 notebook = run_notebook.notebook_run /home/marc/co/sage/local/lib/python2.7/site-packages/sagenb-0.10.4-py2.7.egg/sagenb/notebook/run_notebook.pyc in notebook_run(self, directory, port, interface, port_tries, secure, reset, accounts, openid, server_pool, ulimit, timeout, upload, automatic_login, start_path, fork, quiet, server, profile, subnets, require_login, open_viewer, address) 626 os.chdir(cwd) 627 if e == 256: --> 628 raise socket.error 629 630 def get_admin_passwd(): error:
comment:10 Changed 10 years ago by
Status: | needs_info → needs_review |
---|
Thanks for having a look.
Replying to mmezzarobba:
- There remain references to
src/sage/misc/hg.py
(which was removed as part of 48ff645017639d70d9b4a7107ac7fd98d8a871b5) insrc/sage/doctest/control.py
.
This is now #14954.
- I also get a few more doctest failures such as the one below due to a missing import that seems to be fixed in the current git version of sagenb.
This is fixed by #14330.
I believe that these issues are not related to consolidate-repos.sh
, so I'll put this back to needs_review. These two tickets are part of the metaticket #13015.
comment:11 Changed 10 years ago by
These are the changes that actually need review for this ticket:
git diff origin/master..origin/u/ohanar/build_system
As a diff (this usually times out on the trac server):
As a list of commits:
comment:12 follow-up: 13 Changed 10 years ago by
Status: | needs_review → needs_info |
---|
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?
comment:13 Changed 10 years ago by
Replying to tkluck:
Maybe it makes sense for at least one more person to go through this diff.
Definitely. And hopefully someone who is already familiar with Sage's build system.
why do we clean more directories than before?
In order to make sure distclean still works as expected. Previously cleaning up everything wasn't important for this since distclean would just delete the whole devel directory.
What happened to this parallel_make.cfg file?
There is no other reference to it in the source code.
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?
It seems like I didn't revert some cleanup I made to the Makefile in response to #14721.
diff --git a/build/gen_html b/build/gen_html deleted file mode 100755 index 6dbad06..0000000 --- a/build/gen_html +++ /dev/nullWhat did this script do? Why don't we need it anymore?
it is used to generate the webpages http://sagemath.org/packages/{standard,optional,experimental}, however the actual version of the script that is used in generating these patches disagrees from the version that is included in Sage. Since I was overhauling the build system (and this was in the directory related to the build system), I thought it wise to remove it to avoid any confusion.
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?)
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).
We should at least make sure that all optional packages work. Basically all changes that might be needed should be fixable within the mercurial distribution. Since we will have to through the packages anyway, it made perfect sense to me to cleanup the whole SAGE_DATA thing. I've opened #14962 for this task.
This change has been made in quite some places. Why wasn't GREP_OPTIONS being set a problem before?
It was, however I didn't bother fixing it on the mercurial side because either you have to unset it in sage-env or you have to do it in a bunch of packages (although it just came to me that you could unset it in sage-spkg). Unsetting in sage-env is less than ideal because it means that commands like sage -grep don't respect GREP_OPTIONS, and changing a simple thing like this across a bunch of packages is incredibly painful in the current workflow.
It feels wrong to me to prefer the src/bin version of the sage script to the local/bin version. Why is this?
I've gone back and forth on this a few times. The reason why I ended up with this was because the latest version of the sage script will always be at src/bin, where as the version at local/bin will only be updated if you have rerun make.
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?
This was a merging error, sage-location should still be here.
Why don't we call sage-location anymore? and sage-starts?
I moved the sage-location call to sage-real-upgrade, and I think that sage-starts should probably be moved to sage-real-upgrade as well (which I think I meant to do, but it seems like I forgot).
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?
Without an argument git clone will clone HEAD.
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?
It was, but it is more of an issue now because sage-spkg now tries a few urls for each package, which requires sage-download-file to bail if the server returns an error.
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.
Currently we are symlinking stuff into SAGE_LOCAL from the devel directory (this is to make cloning fast), however this is no longer happening in the git repository. This caused some silly breaking with the sage-sync-build, which was in need of some heavy revision anyway. There is now another ticket out there (don't remember which one) dedicated to fixing + updating this script.
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 +fiMaybe do a
git fetch
first?
Good idea.
Also, should we add pushing to these branches in the repo to a script somewhere? Maybe to sage-sdist?
Pushing should only be done by the release manager. I think the best place for such stuff would be as part of the development scripts, although I think adding the release manager portion to these scripts should be lower priority than the general developer portion of these scripts.
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?
I think so, although we really should have someone on Cygwin test. As I mentioned, the symlinks that the comment refers to are no more.
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?
This can be changed in sage-spkg. That message just prints when it realizes it has the package metadata locally.
diff --git a/src/spkg-delauto b/src/spkg-delauto deleted file mode 100755 index 61d071d..0000000 --- a/src/spkg-delauto +++ /dev/nullWhy don't we need this anymore?
no other reference in the source code (except for MANIFEST.in, which should be updated)
comment:14 Changed 10 years ago by
Mind if I start making some of the (simpler) changes?
Currently we are symlinking stuff into SAGE_LOCAL from the devel directory (this is to make cloning fast), however this is no longer happening in the git repository. This caused some silly breaking with the sage-sync-build, which was in need of some heavy revision anyway. There is now another ticket out there (don't remember which one) dedicated to fixing + updating this script.
I couldn't find the ticket. Do you mean that your edits basically fix the ticket? Or do you mean that the version of sage-sync-build
in your build_system
branch is still in need of some heavy revision?
It feels wrong to me to prefer the src/bin version of the sage script to the local/bin version. Why is this?
I've gone back and forth on this a few times. The reason why I ended up with this was because the latest version of the sage script will always be at src/bin, where as the version at local/bin will only be updated if you have rerun make.
I agree that this is the behavior, but I'm not sure whether that's what we want. After switching branches, you could end up in the situation where the sage
script from one branch is using SAGE_LOCAL
files from another branch. Imagine that a branch adds both a package imagemagick
and a command sage -convert
and it seems much more consistent to me to only run the new sage script after make
has been called.
But I guess that I don't mind much; whoever is in a situation where the difference affects them, is probably doing something with the internal workings of sage anyway, and should be able to deal with it.
comment:15 follow-up: 16 Changed 9 years ago by
I think it makes sense to move build/pkgs
to pkgs
. My current work on top of build_system
uses pkgs
for the package sources and build
is used to build packages. Would it be possible to do this move in build_system
?
comment:16 Changed 9 years ago by
Ok, I think I addressed Timo's comments, with the following exceptions:
HG configuration in sage-env: I don't see a good reason for getting rid of these in the short term, we may not be using hg anymore, but it could still be helpful for the transition period while repositories there are still HG repositories for optional packages.
GREP_OPTIONS: I'm not going to fix this now, instead leave it to a later date (unless I encounter merge conflicts, which so far I haven't).
Replying to felixs:
I think it makes sense to move
build/pkgs
topkgs
. My current work on top ofbuild_system
usespkgs
for the package sources andbuild
is used to build packages. Would it be possible to do this move inbuild_system
?
I would rather not, I don't see a huge need for change at this moment and currently everything works. We can always change this later down the road, but for now I would like to get this ticket finished.
One other thing is that some functionality (especially sdist and bdist) might be broken until I merge the git spkg, which I will do in this ticket. However, there is still plenty of other code that needs review so I'm going to mark this to needs review for the moment.
comment:17 Changed 9 years ago by
Status: | needs_info → needs_review |
---|
comment:19 Changed 9 years ago by
Commit: | → c7903c067f0fa949bcd09e6e60c6c306dee26ab4 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:20 Changed 9 years ago by
Some comments (I will probably have several, so I post them one by one):
In top-level Makefile
in lib-clean
, replace
rm -rf src/sage/libs/pari/gen.h rm -rf src/sage/modular/arithgroup/farey_symbol.h rm -rf src/sage/rings/real_mpfi.h rm -rf src/sage/symbolic/pynac.h
by
rm -f src/sage/modular/arithgroup/farey_symbol.h rm -f src/sage/rings/real_mpfi.h rm -f src/sage/symbolic/pynac.h
(the removal of gen.h
is #15124).
Actually, I would prefer a multi-level Makefile
(this is the preferred way for automake
). That is, move lib-clean
to a file src/Makefile
and have the top-level Makefile
contain:
lib-clean: cd src && $(MAKE) clean
comment:21 Changed 9 years ago by
SCRIPT_SOURCES
and the likes in build/Makefile
(generated by build/install
): why use absolute filenames instead of environment variables like $(SAGE_LOCAL)
? This might break relocation.
comment:22 Changed 9 years ago by
build/Makefile
: what does @mkdir -p "$(@D)"
do? Add a comment explaining this.
comment:23 Changed 9 years ago by
src/bin/sage-spkg
: replace
RESULT=`GREP_OPTIONS= grep "^$param\ *=" $file| sed "s/^$param\ *=\ *//"` echo $RESULT
by
sed -n "s/^${param} *= *//p" $file
comment:24 follow-up: 33 Changed 9 years ago by
Concerning checksums: why not add Python support? For the buildbots, I have this script to verify checksums of Sage tarballs:
#!/usr/bin/env python import hashlib import sys H = hashlib.sha256() while True: d = sys.stdin.read(4096) H.update(d) if not len(d): break print H.hexdigest()
You could have a variant of this be called shasum.py
and then run python shasum.py
if Python exists (command -v python
).
comment:25 Changed 9 years ago by
In shell scripts, all instances of [ A == B ]
should be replaced by [ A = B ]
.
comment:26 Changed 9 years ago by
In src/bin/sage-spkg
, what is the purpose of USE_LOCAL_SCRIPTS
? It is difficult for me to understand, perhaps some comment should be added.
comment:27 follow-up: 28 Changed 9 years ago by
I dislike local/var/tmp/sage/build
. One often needs to go into this directory to debug stuff, could you not use an easier directory instead?
comment:28 Changed 9 years ago by
Replying to jdemeyer:
I dislike
local/var/tmp/sage/build
. One often needs to go into this directory to debug stuff, could you not use an easier directory instead?
Sure, any suggestions (I would like to keep it under local
somewhere, as it in the longterm consolidates the directories that would need to be cleaned).
comment:29 Changed 9 years ago by
Commit: | c7903c067f0fa949bcd09e6e60c6c306dee26ab4 → 1a65c42fcce72b927d76fe7dfc9d6779c80c0cef |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:30 Changed 9 years ago by
Commit: | 1a65c42fcce72b927d76fe7dfc9d6779c80c0cef → 50e57d75d53601bc073a855ce995d7aa92506a92 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:31 Changed 9 years ago by
Status: | needs_review → needs_info |
---|
What's the status of this ticket? Andrew, are you going to be able to address Jeroen's concerns soon?
comment:32 Changed 9 years ago by
Commit: | 50e57d75d53601bc073a855ce995d7aa92506a92 → c29a7554ab4cfd9f0d17a8a242558db5b820b12a |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:33 Changed 9 years ago by
Commit: | c29a7554ab4cfd9f0d17a8a242558db5b820b12a → 50e57d75d53601bc073a855ce995d7aa92506a92 |
---|
Replying to jdemeyer:
Concerning checksums: why not add Python support? For the buildbots, I have this script to verify checksums of Sage tarballs:
You could have a variant of this be called
shasum.py
and then runpython shasum.py
if Python exists (command -v python
).
I don't quite understand why we should duplicate this functionality in python, since we already have a working version in shell.
comment:34 Changed 9 years ago by
Commit: | 50e57d75d53601bc073a855ce995d7aa92506a92 → 86a61958f775df75bfabb035ffeeaffd69ffdec6 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:35 Changed 9 years ago by
Commit: | 86a61958f775df75bfabb035ffeeaffd69ffdec6 → bf481816e69f86973d24a18e7df25852b69a2a8a |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:bf48181] | Trac #14908: Two small issues with make micro_release |
[changeset:0628ee3] | Fix two small issues with make micro_release |
comment:36 Changed 9 years ago by
Status: | needs_info → needs_review |
---|
Why is this needs info? Because we want to know its status? Isn't that a bit self-referential?
I think all of Jeroen's comments were addressed except the build dir (though I agree that keeping it under local is a good idea, and the old build dir wasn't particularly short either) and using Python to compute the hash (better solution than calling external program, but might depend on whether Python build hashlib correctly which IIRC sometimes causes trouble). In any case both can be easily changed later on, so I don't think we necessarily have to act on them now.
comment:37 Changed 9 years ago by
Commit: | bf481816e69f86973d24a18e7df25852b69a2a8a → 5677976a6fdd6f7a1c1f5e634868accd6be0eeac |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:5677976] | Trac #15120: Fix doctest doctests in git layout. |
[changeset:e11727c] | revert the change to .gitignore |
[changeset:6912669] | Fix --new option for doctests. |
[changeset:98d0840] | Fix doctest doctest due to directory restructure. |
comment:38 Changed 9 years ago by
Commit: | 5677976a6fdd6f7a1c1f5e634868accd6be0eeac → 56d80d07ad270272a64e8c71fd7a9bc632f37ba6 |
---|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
[changeset:56d80d0] | [FIXUP] 5.12.rc0: update git specific files |
[changeset:4aa23b7] | Merge branch 'master' into build_system |
[changeset:df16d32] | Merge 'sage-5.12.rc0' into master |
[changeset:45a79e8] | 5.12.rc0 |
[changeset:0f00389] | 5.12.rc0 |
[changeset:a77c210] | Trac #13770: Add upstream pull request 215 for multivariate factorisation |
[changeset:18be736] | Trac #13770: bug in multivariate factorization over prime fields |
[changeset:59c8447] | Trac #15194: added imports and doctests for MIPSolver exceptions |
[changeset:49dc953] | Trac #14776: implements StrongTableau? and StrongTableaux? classes |
[changeset:a854c61] | Trac #15204: fix gap cyclotomics to sage |
comment:39 Changed 9 years ago by
Commit: | 56d80d07ad270272a64e8c71fd7a9bc632f37ba6 → ca97e70c81f671ba4413ecb1530331d91560c46f |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
[changeset:ca97e70] | [FIXUP] 5.12.rc1: update git specific files |
[changeset:9cf9b36] | Merge branch 'master' into build_system |
[changeset:e061dd3] | Merge 'sage-5.12.rc1' into master |
[changeset:864c388] | 5.12.rc1 |
[changeset:dab4c3f] | 5.12.rc1 |
[changeset:9c2f74f] | Trac #15252: Doctest tolerance |
[changeset:07f335d] | Trac #13948: fix various configure tests which break with Clang |
[changeset:463ace3] | Trac #15189: fixed recursion in _equivalence method used by is_field_isomorphic |
[changeset:b856810] | Trac #15193: add method Factorization._pari_(), using itertools.chain |
[changeset:05fdb53] | Trac #13948: Document that Sage can now be bootstrapped with Clang |
comment:40 Changed 9 years ago by
Commit: | ca97e70c81f671ba4413ecb1530331d91560c46f → 60d5a0630a478a7778110581fc46ef4b9c6dd8ab |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:41 Changed 9 years ago by
I wonder if there should be a symbolic link from "doc" at the top level pointing to src/doc/output/html/en
. Not exactly a new directory layout, but in the spirit of this ticket: beginning users might be comforted to find the documentation in an obvious place. This link should probably created at the end of the build process, after building the docs (part of the doc
target in Makefile
).
This could also be done on a separate ticket, of course.
comment:42 Changed 9 years ago by
Commit: | 60d5a0630a478a7778110581fc46ef4b9c6dd8ab → ec4ed117d844ca2ce24f16e01e5e61e5683ef4ba |
---|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
[changeset:ec4ed11] | [FIXUP] 5.13.beta1: update git specific files |
[changeset:a23df3e] | Merge branch 'upstream' into build_system |
[changeset:b7dc123] | Merge 'sage-5.13.beta1' |
[changeset:8693e80] | 5.13.beta1 |
[changeset:755697a] | 5.13.beta1 |
[changeset:e3808e5] | Trac #13726: Semimonomial transformation group |
[changeset:d182ea9] | Trac #15279: init of RootSystem? corrected |
[changeset:f5adfd5] | Solaris / ZFS fix |
[changeset:d13cee2] | Trac #15270: Do not give up if the upstream shared library build fails |
[changeset:d25cd84] | Trac #10589: Updating the message for the fixdoctests option printed by --advanced |
comment:43 Changed 9 years ago by
Commit: | ec4ed117d844ca2ce24f16e01e5e61e5683ef4ba → 952f7e3d12a82a40e6c0365f98f0e3984b4308ca |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:44 Changed 9 years ago by
The docs en/reference/misc/index.rst
still reference the deleted sage/misc/hg
, leading to a docbuild error. I think this branch would be the right place to fix the fallout.
As for the doc symlink, we should probably disentangle the src
dir from build output altogether. Documentation should probably go into local/share/sage/doc
. But first we really should finalize the git transition.
comment:45 Changed 9 years ago by
Commit: | 952f7e3d12a82a40e6c0365f98f0e3984b4308ca → 874ba344029a1baae5d9ab8ba6554f5a0d0126a7 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:46 Changed 9 years ago by
Commit: | 874ba344029a1baae5d9ab8ba6554f5a0d0126a7 → f5796f2d3508aa39ef578121d0796d1bf3fecfbe |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
f5796f2 | remove reference to sage/misc/hg from docbuild |
comment:47 Changed 9 years ago by
Reviewers: | → Volker Braun |
---|---|
Status: | needs_review → positive_review |
Looks good to me!
comment:48 Changed 9 years ago by
Commit: | f5796f2d3508aa39ef578121d0796d1bf3fecfbe → 1b1d23620cb9e912c21afe6ec75aba322d6b2dd5 |
---|---|
Status: | positive_review → needs_review |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
comment:49 Changed 9 years ago by
Status: | needs_review → positive_review |
---|
For the record, this ticket is positively reviewed. Even if Andrew keeps merging the sage-hg branch until sage-5.13 is finished.
comment:50 follow-up: 51 Changed 9 years ago by
Commit: | 1b1d23620cb9e912c21afe6ec75aba322d6b2dd5 → 47cb70ddd7f01952996ba3d404d5d4c12a4dae7e |
---|---|
Status: | positive_review → needs_review |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
comment:51 Changed 9 years ago by
Hi,
The merge of 5.13-beta4
broke make build
for me, because http://www.sagemath.org/packages/upstream/r/r-3.0.2.tar is missing. See also http://trac.sagemath.org/ticket/14706#comment:119.
comment:52 Changed 9 years ago by
Commit: | 47cb70ddd7f01952996ba3d404d5d4c12a4dae7e → 33dec52b43f2fcc22318129da442e5bdb8a24b28 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:53 Changed 9 years ago by
Commit: | 33dec52b43f2fcc22318129da442e5bdb8a24b28 → cd237e726293c48288016aa2a9cc80aa75a3bd20 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
comment:54 Changed 9 years ago by
Status: | needs_review → positive_review |
---|
comment:55 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | positive_review → closed |
This can really only be reviewed on the new trac instance. For now, the branch field on http://trac.tangentspace.org/ticket/13015 links to the list of changes (to actually see the changes click the view changes button -- since this includes some massive changes, it might take awhile to load the side-by-side diff).