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 vbraun)

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 vbraun

  • Branch set to u/vbraun/reorganize_build

comment:2 Changed 4 years ago by vbraun

  • Cc jdemeyer added
  • Commit set to a7546b152b891b6abb501354b8f127bebedfba7e
  • Component changed from PLEASE CHANGE to build
  • Type changed from PLEASE CHANGE to enhancement

Note to self: this will require an updated confball


New commits:

b667765Move makefile-stuff to build/make/
a7546b1Updating paths for build/make/

comment:3 Changed 4 years ago by jdemeyer

FYI: this conflicts heavily with #18710.

comment:4 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 4 years ago by git

  • Commit changed from a7546b152b891b6abb501354b8f127bebedfba7e to fd25c7c52f2544bf287e18defa67755281ad55d3

Branch pushed to git repo; I updated commit sha1. New commits:

f034a44set up print and logging for sage_bootstrap
bd61b67add the sage-pkg entrypoint
25022c3move sage-download-file to the sage_bootstrap library
0acf070initial implementation of the sage-pkg tool
52d0126rename sage-pkg -> sage-package
e4aff9afix indentation
e45a0b6Do not assume that the confball has been downloaded before in test
da11bfcMerge sage_bootstrap branch nto #18788
fd25c7cMove sage_bootstrap to /build

comment:6 Changed 4 years ago by git

  • Commit changed from fd25c7c52f2544bf287e18defa67755281ad55d3 to bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53

Branch pushed to git repo; I updated commit sha1. New commits:

f3c4445Move various targets to build/Makefile
b86e0bbFix strange mixture of TABs and spaces
bd3149bMerge branch 'u/jdemeyer/move_some_make_targets_to_build_makefile' of http://trac.sagemath.org/sage into #18788

comment:7 follow-up: Changed 4 years ago by vbraun

  • 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 vbraun

  • Dependencies changed from #18748, to #18748, #18710
  • Description modified (diff)

comment:9 Changed 4 years ago by git

  • Commit changed from bd3149b68bcf18ec0660d4fa90b5c9ea9c03ea53 to 0aa6cfb23685ed7a653e0ccdd1c27e67fb005140

Branch pushed to git repo; I updated commit sha1. New commits:

51f13e8fix documentation build
0aa6cfbRemove test for faux package

comment:10 follow-up: Changed 4 years ago by 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]

comment:11 Changed 4 years ago by vbraun

  • Authors set to Volker Braun
  • Status changed from new to needs_review

comment:12 in reply to: ↑ 7 Changed 4 years ago by jdemeyer

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: Changed 4 years ago by jdemeyer

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 jdemeyer

In case I haven't made it sufficiently clear on #18748: I am still against the fact that you first create src/sage_bootstrap/sage_bootstrap on #18748 and then immediately move it to build/sage_bootstrap on this ticket. This is just making things needlessly complicated.

comment:15 follow-up: Changed 4 years ago by vbraun

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: Changed 4 years ago by jdemeyer

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 jdemeyer

Replying to jdemeyer:

I don't think this commit should be on this ticket, but on a separate blocker ticket with just this one commit.

See #18806 instead.

comment:18 in reply to: ↑ 16 ; follow-up: Changed 4 years ago by vbraun

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 jdemeyer

Replying to vbraun:

I refer to #18710 working fine on incremental builds but breaking full builds.

So the fact that one ticket changing the build system breaks it proves that it's not reliable?

What do you want, a build system that is impossible to break when changing it?

comment:20 Changed 4 years ago by vbraun

Yes. There are various open/free implementations that do precisely that, so why shouldn't we?

comment:21 Changed 4 years ago by git

  • Commit changed from 0aa6cfb23685ed7a653e0ccdd1c27e67fb005140 to a497d236579ef680a5b7171c536348fc0c3089be

Branch pushed to git repo; I updated commit sha1. New commits:

a497d23Merge 6.8.beta7 into t/18788/reorganize_build

comment:22 Changed 4 years ago by jdemeyer

  • Branch changed from u/vbraun/reorganize_build to u/jdemeyer/reorganize_build

comment:23 Changed 4 years ago by jdemeyer

  • Commit changed from a497d236579ef680a5b7171c536348fc0c3089be to 0e168b13f36e0b6af6de5d521fa18f9054cb1add

Squashed on top of #18748 for easier reviewing.


New commits:

9e2c568Merge branch 'develop' into t/18748/python_library_to_bootstrap_sage
cc213dctrac 18748: add documentation to scripts in sage_bootstrap/bin
033b56eMerge Sage-6.8.beta8 into #18748
8f4fb8eMore docstrings
f9a69a5Convert public attributes to properties and document
075228fMake package.tarball a Tarball instance
c56991bFix Python 2.6 test error
60a581btrac 18748: add URL for tox to sage_bootstrap/README
0e168b1Reorganize /build

comment:24 Changed 4 years ago by git

  • Commit changed from 0e168b13f36e0b6af6de5d521fa18f9054cb1add to b324ff0f8ba4899ae0cf58eded2c0933b2adf1b5

Branch pushed to git repo; I updated commit sha1. New commits:

b324ff0Move README inside sage_bootstrap directory

comment:25 Changed 4 years ago by git

  • Commit changed from b324ff0f8ba4899ae0cf58eded2c0933b2adf1b5 to 550b26e15578fc24d4c27222a99957e9162eef31

Branch pushed to git repo; I updated commit sha1. New commits:

538177eFix paths in documentation and comments
550b26eMove sage-spkg to build/bin

comment:26 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

This looks okay to me now, what remains is testing.

comment:27 Changed 4 years ago by jdemeyer

Seems to work...

I give positive_review to your commit, somebody needs to review mine.

comment:28 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:29 Changed 4 years ago by vbraun

  • 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 vbraun

  • 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 vbraun

  • Branch changed from u/jdemeyer/reorganize_build to u/vbraun/reorganize_build

comment:32 Changed 4 years ago by vbraun

  • Commit changed from 550b26e15578fc24d4c27222a99957e9162eef31 to c1be005d25d13620216b9ac0700e87cb8aaf3c48
  • Status changed from new to needs_review

New commits:

b667765Move makefile-stuff to build/make/
a7546b1Updating paths for build/make/
da11bfcMerge sage_bootstrap branch nto #18788
fd25c7cMove sage_bootstrap to /build
bd3149bMerge branch 'u/jdemeyer/move_some_make_targets_to_build_makefile' of http://trac.sagemath.org/sage into #18788
51f13e8fix documentation build
0aa6cfbRemove test for faux package
a497d23Merge 6.8.beta7 into t/18788/reorganize_build
c1be005Fix remaining occurence of old path in sage-spkg

comment:33 Changed 4 years ago by jdemeyer

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 git

  • Commit changed from c1be005d25d13620216b9ac0700e87cb8aaf3c48 to 35efc21ddedc54eeb57bfd5fd8df3dd7b688ae08

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

8f4fb8eMore docstrings
f9a69a5Convert public attributes to properties and document
075228fMake package.tarball a Tarball instance
c56991bFix Python 2.6 test error
60a581btrac 18748: add URL for tox to sage_bootstrap/README
0e168b1Reorganize /build
b324ff0Move README inside sage_bootstrap directory
538177eFix paths in documentation and comments
550b26eMove sage-spkg to build/bin
35efc21Fix remaining occurence of old path in sage-spkg

comment:35 Changed 4 years ago by vbraun

thanks, forgot to pull...

comment:36 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:37 Changed 4 years ago by vbraun

  • Branch changed from u/vbraun/reorganize_build to 35efc21ddedc54eeb57bfd5fd8df3dd7b688ae08
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.