Opened 4 years ago
Closed 4 years ago
#18788 closed enhancement (fixed)
Reorganize /build
Reported by: | vbraun | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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/sage-package
- /build/bin/sage-spkg
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 4 years ago by
- Branch set to u/vbraun/reorganize_build
comment:2 Changed 4 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 4 years ago by
FYI: this conflicts heavily with #18710.
comment:4 Changed 4 years ago by
- Description modified (diff)
comment:5 Changed 4 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 sage-pkg entrypoint
|
25022c3 | move sage-download-file to the sage_bootstrap library
|
0acf070 | initial implementation of the sage-pkg tool
|
52d0126 | rename sage-pkg -> sage-package
|
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 4 years ago by
- Commit changed from fd25c7c52f2544bf287e18defa67755281ad55d3 to bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53
comment:7 follow-up: ↓ 12 Changed 4 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 4 years ago by
- Dependencies changed from #18748, to #18748, #18710
- Description modified (diff)
comment:9 Changed 4 years ago by
- Commit changed from bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53 to 0aa6cfb23685ed7a653e0ccdd1c27e67fb005140
comment:10 follow-up: ↓ 13 Changed 4 years ago by
The last commit fixes the doctest failure introduced in #18710 (but only triggered after build from scratch):
sage -t --warn-long 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 4 years ago by
- Status changed from new to needs_review
comment:12 in reply to: ↑ 7 Changed 4 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 ; follow-up: ↓ 17 Changed 4 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 --warn-long 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 4 years ago by
comment:15 follow-up: ↓ 16 Changed 4 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 ; follow-up: ↓ 18 Changed 4 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 4 years ago by
comment:18 in reply to: ↑ 16 ; follow-up: ↓ 19 Changed 4 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 large-scale software projects impossible (e.g. google built blaze/bazel when they hit that wall with makefiles).
comment:19 in reply to: ↑ 18 Changed 4 years ago by
comment:20 Changed 4 years ago by
Yes. There are various open/free implementations that do precisely that, so why shouldn't we?
comment:21 Changed 4 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 4 years ago by
- Branch changed from u/vbraun/reorganize_build to u/jdemeyer/reorganize_build
comment:23 Changed 4 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 Sage-6.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 4 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 4 years ago by
- Commit changed from b324ff0f8ba4899ae0cf58eded2c0933b2adf1b5 to 550b26e15578fc24d4c27222a99957e9162eef31
comment:26 Changed 4 years ago by
- Reviewers set to Jeroen Demeyer
This looks okay to me now, what remains is testing.
comment:27 Changed 4 years ago by
Seems to work...
I give positive_review to your commit, somebody needs to review mine.
comment:28 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:29 Changed 4 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 4 years ago by
- Branch changed from 550b26e15578fc24d4c27222a99957e9162eef31 to u/jdemeyer/reorganize_build
- Resolution fixed deleted
- Status changed from closed to new
wrong path in sage-sdist
comment:31 Changed 4 years ago by
- Branch changed from u/jdemeyer/reorganize_build to u/vbraun/reorganize_build
comment:32 Changed 4 years ago by
- Commit changed from 550b26e15578fc24d4c27222a99957e9162eef31 to c1be005d25d13620216b9ac0700e87cb8aaf3c48
- Status changed from new to needs_review
New commits:
b667765 | Move makefile-stuff 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 sage-spkg
|
comment:33 Changed 4 years ago by
The sage-sdist
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 cherry-picked onto my branch.
comment:34 Changed 4 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 sage-spkg to build/bin
|
35efc21 | Fix remaining occurence of old path in sage-spkg
|
comment:35 Changed 4 years ago by
thanks, forgot to pull...
comment:36 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:37 Changed 4 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 makefile-stuff to build/make/
Updating paths for build/make/