Opened 5 years ago
Closed 5 years ago
#18788 closed enhancement (fixed)
Reorganize /build
Reported by:  vbraun  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  build  Keywords:  
Cc:  jdemeyer  Merged in:  
Authors:  Volker Braun  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  35efc21 (Commits)  Commit:  35efc21ddedc54eeb57bfd5fd8df3dd7b688ae08 
Dependencies:  #18748, #18710  Stopgaps: 
Description (last modified by )
Proposal: Make /build the root for the sage_bootstrap
Python module
 /build/sage_bootstrap/...
 /build/setup.py
 /build/tox.ini
 /build/bin/sagepackage
 /build/bin/sagespkg
Keep the current package metadata at
 /build/pkgs/...
Move the makefile stuff to
 /build/make/Makefile
 /build/make/deps
 /build/make/pipestatus
 /build/make/install
 /build/make/...
Change History (37)
comment:1 Changed 5 years ago by
 Branch set to u/vbraun/reorganize_build
comment:2 Changed 5 years ago by
 Cc jdemeyer added
 Commit set to a7546b152b891b6abb501354b8f127bebedfba7e
 Component changed from PLEASE CHANGE to build
 Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 5 years ago by
FYI: this conflicts heavily with #18710.
comment:4 Changed 5 years ago by
 Description modified (diff)
comment:5 Changed 5 years ago by
 Commit changed from a7546b152b891b6abb501354b8f127bebedfba7e to fd25c7c52f2544bf287e18defa67755281ad55d3
Branch pushed to git repo; I updated commit sha1. New commits:
f034a44  set up print and logging for sage_bootstrap

bd61b67  add the sagepkg entrypoint

25022c3  move sagedownloadfile to the sage_bootstrap library

0acf070  initial implementation of the sagepkg tool

52d0126  rename sagepkg > sagepackage

e4aff9a  fix indentation

e45a0b6  Do not assume that the confball has been downloaded before in test

da11bfc  Merge sage_bootstrap branch nto #18788

fd25c7c  Move sage_bootstrap to /build

comment:6 Changed 5 years ago by
 Commit changed from fd25c7c52f2544bf287e18defa67755281ad55d3 to bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53
comment:7 followup: ↓ 12 Changed 5 years ago by
 Dependencies changed from #18748 to #18748,
IMHO the pipestatus and install scripts are just a symptom of the makefile hack. They should be removed when we switch to a sane build system, so they shouldn't be in build/bin
comment:8 Changed 5 years ago by
 Dependencies changed from #18748, to #18748, #18710
 Description modified (diff)
comment:9 Changed 5 years ago by
 Commit changed from bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53 to 0aa6cfb23685ed7a653e0ccdd1c27e67fb005140
comment:10 followup: ↓ 13 Changed 5 years ago by
The last commit fixes the doctest failure introduced in #18710 (but only triggered after build from scratch):
sage t warnlong 27.4 src/sage/misc/package.py ********************************************************************** File "src/sage/misc/package.py", line 193, in sage.misc.package.is_package_installed Failed example: is_package_installed('sage') Expected: True Got: False ********************************************************************** 1 item had failures: 1 of 4 in sage.misc.package.is_package_installed [15 tests, 1 failure, 0.08 s]
comment:11 Changed 5 years ago by
 Status changed from new to needs_review
comment:12 in reply to: ↑ 7 Changed 5 years ago by
Replying to vbraun:
IMHO the pipestatus and install scripts are just a symptom of the makefile hack. They should be removed when we switch to a sane build system, so they shouldn't be in build/bin
For the record: I don't think makefiles are a hack. The current system surely is far from perfect, but it actually does work pretty well.
comment:13 in reply to: ↑ 10 ; followup: ↓ 17 Changed 5 years ago by
Replying to vbraun:
The last commit fixes the doctest failure introduced in #18710 (but only triggered after build from scratch):
sage t warnlong 27.4 src/sage/misc/package.py ********************************************************************** File "src/sage/misc/package.py", line 193, in sage.misc.package.is_package_installed Failed example: is_package_installed('sage') Expected: True Got: False ********************************************************************** 1 item had failures: 1 of 4 in sage.misc.package.is_package_installed [15 tests, 1 failure, 0.08 s]
I don't think this commit should be on this ticket, but on a separate blocker ticket with just this one commit.
Also: don't you test building from scratch when closing a ticket?
comment:14 Changed 5 years ago by
comment:15 followup: ↓ 16 Changed 5 years ago by
Well we just saw that the current system can't do reliable incremental builds, I think that this is pretty bad. That was ok 20 years ago but today this is hardly state of the art.
For the record, I'm only doing incremental builds for each ticket on the buildbot. In addition, there is a weekly full build.
You can review both tickets at the same time if the location of sage_bootstrap
irks you.
comment:16 in reply to: ↑ 15 ; followup: ↓ 18 Changed 5 years ago by
Replying to vbraun:
Well we just saw that the current system can't do reliable incremental builds
What do you refer to?
comment:17 in reply to: ↑ 13 Changed 5 years ago by
comment:18 in reply to: ↑ 16 ; followup: ↓ 19 Changed 5 years ago by
Replying to jdemeyer:
Replying to vbraun:
Well we just saw that the current system can't do reliable incremental builds
What do you refer to?
I refer to #18710 working fine on incremental builds but breaking full builds. By definition, that means our build system cannot reliably do incremental builds. Really, the build system has one job to do and it failed at it. Essentially, improper handling of build artifacts. Thats ok if you have a small library, but it hurts when a full rebuild takes an hour, and it makes truly largescale software projects impossible (e.g. google built blaze/bazel when they hit that wall with makefiles).
comment:19 in reply to: ↑ 18 Changed 5 years ago by
comment:20 Changed 5 years ago by
Yes. There are various open/free implementations that do precisely that, so why shouldn't we?
comment:21 Changed 5 years ago by
 Commit changed from 0aa6cfb23685ed7a653e0ccdd1c27e67fb005140 to a497d236579ef680a5b7171c536348fc0c3089be
Branch pushed to git repo; I updated commit sha1. New commits:
a497d23  Merge 6.8.beta7 into t/18788/reorganize_build

comment:22 Changed 5 years ago by
 Branch changed from u/vbraun/reorganize_build to u/jdemeyer/reorganize_build
comment:23 Changed 5 years ago by
 Commit changed from a497d236579ef680a5b7171c536348fc0c3089be to 0e168b13f36e0b6af6de5d521fa18f9054cb1add
Squashed on top of #18748 for easier reviewing.
New commits:
9e2c568  Merge branch 'develop' into t/18748/python_library_to_bootstrap_sage

cc213dc  trac 18748: add documentation to scripts in sage_bootstrap/bin

033b56e  Merge Sage6.8.beta8 into #18748

8f4fb8e  More docstrings

f9a69a5  Convert public attributes to properties and document

075228f  Make package.tarball a Tarball instance

c56991b  Fix Python 2.6 test error

60a581b  trac 18748: add URL for tox to sage_bootstrap/README

0e168b1  Reorganize /build

comment:24 Changed 5 years ago by
 Commit changed from 0e168b13f36e0b6af6de5d521fa18f9054cb1add to b324ff0f8ba4899ae0cf58eded2c0933b2adf1b5
Branch pushed to git repo; I updated commit sha1. New commits:
b324ff0  Move README inside sage_bootstrap directory

comment:25 Changed 5 years ago by
 Commit changed from b324ff0f8ba4899ae0cf58eded2c0933b2adf1b5 to 550b26e15578fc24d4c27222a99957e9162eef31
comment:26 Changed 5 years ago by
 Reviewers set to Jeroen Demeyer
This looks okay to me now, what remains is testing.
comment:27 Changed 5 years ago by
Seems to work...
I give positive_review to your commit, somebody needs to review mine.
comment:28 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:29 Changed 5 years ago by
 Branch changed from u/jdemeyer/reorganize_build to 550b26e15578fc24d4c27222a99957e9162eef31
 Resolution set to fixed
 Status changed from positive_review to closed
comment:30 Changed 5 years ago by
 Branch changed from 550b26e15578fc24d4c27222a99957e9162eef31 to u/jdemeyer/reorganize_build
 Resolution fixed deleted
 Status changed from closed to new
wrong path in sagesdist
comment:31 Changed 5 years ago by
 Branch changed from u/jdemeyer/reorganize_build to u/vbraun/reorganize_build
comment:32 Changed 5 years ago by
 Commit changed from 550b26e15578fc24d4c27222a99957e9162eef31 to c1be005d25d13620216b9ac0700e87cb8aaf3c48
 Status changed from new to needs_review
New commits:
b667765  Move makefilestuff to build/make/

a7546b1  Updating paths for build/make/

da11bfc  Merge sage_bootstrap branch nto #18788

fd25c7c  Move sage_bootstrap to /build

bd3149b  Merge branch 'u/jdemeyer/move_some_make_targets_to_build_makefile' of http://trac.sagemath.org/sage into #18788

51f13e8  fix documentation build

0aa6cfb  Remove test for faux package

a497d23  Merge 6.8.beta7 into t/18788/reorganize_build

c1be005  Fix remaining occurence of old path in sagespkg

comment:33 Changed 5 years ago by
The sagesdist
fix is obviously ok.
But I'm confused with the git history, did you just completely forget about my branch? If that was a mistake, I give positive review to your last commit cherrypicked onto my branch.
comment:34 Changed 5 years ago by
 Commit changed from c1be005d25d13620216b9ac0700e87cb8aaf3c48 to 35efc21ddedc54eeb57bfd5fd8df3dd7b688ae08
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
8f4fb8e  More docstrings

f9a69a5  Convert public attributes to properties and document

075228f  Make package.tarball a Tarball instance

c56991b  Fix Python 2.6 test error

60a581b  trac 18748: add URL for tox to sage_bootstrap/README

0e168b1  Reorganize /build

b324ff0  Move README inside sage_bootstrap directory

538177e  Fix paths in documentation and comments

550b26e  Move sagespkg to build/bin

35efc21  Fix remaining occurence of old path in sagespkg

comment:35 Changed 5 years ago by
thanks, forgot to pull...
comment:36 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:37 Changed 5 years ago by
 Branch changed from u/vbraun/reorganize_build to 35efc21ddedc54eeb57bfd5fd8df3dd7b688ae08
 Resolution set to fixed
 Status changed from positive_review to closed
Note to self: this will require an updated confball
New commits:
Move makefilestuff to build/make/
Updating paths for build/make/