Opened 3 years ago

Closed 2 years ago

#21480 closed enhancement (fixed)

Make sagelib setup.py self-contained and independent of SAGE_ROOT

Reported by: mkoeppe Owned by:
Priority: blocker Milestone: sage-7.4
Component: build Keywords:
Cc: felixs, jdemeyer, fbissey, embray, leif, vbraun, dimpase, jhpalmieri, vdelecroix, saraedum, slabbe, nthiery, mmezzarobba Merged in:
Authors: Matthias Koeppe Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 0c2ac95 (Commits) Commit: 0c2ac9583ed97ffc74e99c876a8cf4506f5b0162
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

This ticket changes the build process of sagelib in the following way:

  • src/Makefile delegates ALL building to src/setup.py
  • src/setup.py no longer depends on environment variables $SAGE_ROOT, $SAGE_SRC, $SAGE_DOC_SRC etc. (to demonstrate this, Makefile poisons these environment variables). It still depends on $SAGE_LOCAL and environment variables that point below it.

This ticket is meant as:

  • preparation for VPATH builds of sage-the-distribution (#21469)
  • working towards the goal of making sagelib pip-installable -- see #21507 for the eventual goal of having sagelib on PyPI
  • making the flow of directory information at build time clearer for developers

More specifically, the goal of this ticket is that only SAGE_LOCAL needs to be set when the user does 'pip install' of the sagelib. (This ticket almost achieves this, except it also needs SAGE_PKGS and SAGE_CYTHONIZED to be set. The hope is that #20382 and other future tickets will develop better mechanisms to communicate package and directory information to the build.)

. . . . . . .

Some possibly useful information:

  • Documentation on distutils (https://docs.python.org/2/install/), describing use of --build-base to do VPATH builds.
  • pip install keeps the source directory clean, building instead in a temporary directory, by copying the sources. pip install also offers options --build to select a build directory, but there are some pip issues: 2060, 2053, 804 that affect this
  • #14807 has some tricks to making VPATH builds work without copying all python source files. But it uses automake instead of setup.sh; we will not do this in our ticket.

configure tarball: http://sage.ugent.be/www/jdemeyer/sage/configure-185.tar.gz

Change History (144)

comment:1 Changed 3 years ago by fbissey

Am interesting issue is that technically cython_debug has to be installed somewhere (preferably somewhere standard) to be accessible at runtime separately from the source.

I do something in sage-on-gentoo but that's not really satisfactory.

Last edited 3 years ago by fbissey (previous) (diff)

comment:2 in reply to: ↑ description ; follow-up: Changed 3 years ago by leif

Replying to mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

comment:3 Changed 3 years ago by leif

... and sagelib perhaps with a version suffix.

comment:4 in reply to: ↑ 2 ; follow-up: Changed 3 years ago by mkoeppe

Replying to leif:

Replying to mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

Yes, the plan *after* #21469 is to use the Sage builddir -- not just for sagelib, but also for other packages.

*Before* #21469 is merged, I want to use $SAGE_ROOT/var/tmp/sage/build/sagelib to match what other packages do.

comment:5 Changed 3 years ago by mkoeppe

  • Description modified (diff)

Ah, I see what you meant, now I've found $SAGE_BUILD_DIR. Changed description accordingly.

comment:6 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by leif

Replying to mkoeppe:

Replying to leif:

Replying to mkoeppe:

In preparation for VPATH builds of sage-the-distribution (#21469), let's keep src/ clean by using setup.py --build-base=$SAGE_ROOT/var/tmp/sage/build/sagelib

Please use --build-base=$SAGE_BUILD_DIR/sagelib.

Yes, the plan *after* #21469 is to use the Sage builddir -- not just for sagelib, but also for other packages.

??? SAGE_BUILD_DIR exists since years already... (Its default is $SAGE_ROOT/var/tmp/sage/build/.)


*Before* #21469 is merged, I want to use $SAGE_ROOT/var/tmp/sage/build/sagelib to match what other packages do.

comment:7 in reply to: ↑ 6 Changed 3 years ago by mkoeppe

Replying to leif:

??? SAGE_BUILD_DIR exists since years already... (Its default is $SAGE_ROOT/var/tmp/sage/build/.)

Yes, thanks, see my other comment.

comment:8 Changed 3 years ago by leif

Ah ok, race condition.

comment:9 Changed 3 years ago by mkoeppe

By the way, help with implementing this change would be appreciated. I haven't looked at the sagelib build system at all so far.

comment:10 Changed 3 years ago by mkoeppe

  • Branch set to u/mkoeppe/keep_src__clean_by_using___build_base_when_building_sagelib

comment:11 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to e399bf41d805da7ea602daa5b554e0c7ecf2e7b5

New commits:

e399bf4First, wishful step

comment:12 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:13 Changed 3 years ago by jdemeyer

Really sagelib-$SAGE_VERSION? Do you want to fill up everybody's hard disks with Sage build directories? Not to mention that this would require to rebuild everything whenever the Sage version changes.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:14 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 3 years ago by mkoeppe

Fine with me without $SAGE_VERSION; I was just following leif's suggestion in comment 3.

comment:16 follow-up: Changed 3 years ago by felixs

i understand that SAGE_BUILD_DIR is the srcdir for toplevel configure.

as long as sagelib is rooted in $(toplevel)/src, it might be less confusing to choose $SAGE_BUILD_DIR/src (not $SAGE_BUILD_DIR/sagelib) as the builddir for sagelib.

(future: replace src by sagelib, but on both ends)

i dont know exactly how the approach using setup.py will emulate VPATH builds. i think it should imitate "what automake would do", where applicable.

comment:17 in reply to: ↑ 16 Changed 3 years ago by mkoeppe

Replying to felixs:

i understand that SAGE_BUILD_DIR is the srcdir for toplevel configure.

No, it's $SAGE_ROOT/var/tmp/sage/build/

comment:18 Changed 3 years ago by felixs

ok, nevermind (i meant to write "SAGE_BUILD_DIR is the *builddir* for toplevel"). that does not seem to be the case either.

still i am wondering why "src" does not (simply) translate to "src". (sure, i am slightly autotools biased).

comment:19 Changed 3 years ago by git

  • Commit changed from e399bf41d805da7ea602daa5b554e0c7ecf2e7b5 to a73fa065f5030c3b260c04a7e36867fd7f89362f

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

f3f275ePut .cython_version next to cythonized files
94a58f1Make SAGE_BUILD_DIR variable available in sage-env, not just in sage-spkg
a73fa06Use SAGE_BUILD_DIR

comment:20 Changed 3 years ago by mkoeppe

  • Status changed from new to needs_review

Here's a first version for review. It seems to work for me.

There are still some to-do items (see comments in src/Makefile):

  • I am now using --build-base, but setup.sh also depends on SAGE_CYTHONIZED, defined in src/sage/env.py.

I think it would be better if setup.sh instead inferred that location from the build-base that was passed to it.

However, setup.sh already does a lot of stuff depending on SAGE_CYTHONIZED before distutils.core.setup is even called. Can this be fixed?

I think I could use some help from the Python experts in the cc list of this ticket on this.

  • sage/libs/pari/auto_gen.pxi and sage/ext/interpreters/__init__.py still need to be taken care of in preparation for the VPATH build.

comment:21 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:22 follow-ups: Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Thinking about it more, I disagree with building in $SAGE_BUILD_DIR/sagelib by default.

$SAGE_LOCAL (which contains $SAGE_BUILD_DIR) is meant as install directory, not as build directory.

Let's keep $SAGE_LOCAL to be exactly the install directory and nothing else (*).

I do agree with making the build directory configurable for VPATH builds. For typical packages however, if you do not do a VPATH build, the build directory is the same as the source directory. Sage should follow the same model. This means that the build directory should be $SAGE_SRC by default.

(*) One could argue that $SAGE_BUILD_DIR is currently used as build directory for packages. That is true, but they are only used temporarily, they are not meant to actually store stuff. So this isn't so bad.

comment:23 follow-ups: Changed 3 years ago by felixs

Let's keep $SAGE_LOCAL to be exactly the install directory and nothing else (*).

this is part of the problem with the current "build system". SAGE_LOCAL is *not* the install directory. it is where stuff ends up during the build process.

outside sage, an install directory is typically what you pass to --prefix, and where stuff is put into by "make install". you do that (if you are a sysadmin), after assuring that "make check" passes...

yes i know why SAGE_LOCAL exists, but it should be clear that it's not necessary and that it only has "evolved" that way. having simplified things in the past, now it is falling on your feet.

note how i tried to fix/work\ around that in my attempt to autotoolize sage (both the library and the distribution)... my point: the best approach will be to *not use SAGE_LOCAL* at all. here's the chance to cleanup sagelib, as a start.

that said: keep up the good and interesting work @mkoeppe. i hope i will have some more time later this year ...

comment:24 in reply to: ↑ 23 Changed 3 years ago by jdemeyer

Replying to felixs:

this is part of the problem with the current "build system". SAGE_LOCAL is *not* the install directory.

Why do you think that $SAGE_LOCAL is not the install directory? It is the directory where everything is installed, which by definition makes it the install directory. The fact that Sage does not (yet) support --prefix doesn't change this fact.

the best approach will be to *not use SAGE_LOCAL* at all.

I like to know why you think that.

that said: keep up the good and interesting work @mkoeppe. i hope i will have some more time later this year ...

To be clear: I didn't say that this branch needs to be thrown out. I am just saying: keep the build directory configurable but keep the default what it currently is.

comment:25 follow-up: Changed 3 years ago by felixs

Why do you think that $SAGE_LOCAL is not the install directory?

you wrote it. "--prefix" is not implemented/supported. (but it should be). there's no way to really "install" stuff in the usual sense. e.g., there is no way to *install stuff after make check has passed*. (i don't know if toplevel make check is currently implemented at all, just a thought).

where everything is installed, which by definition makes it the install directory

actually nothing gets installed. what is done is mostly overhead. working around the fact that some packages don't work right after the build alone. when i was done with the "package content lists" for spkg-install, i noticed that it was a huge waste of time... don't repeat that, better just skip the "install" step.

I like to know why you think that [SAGE_LOCAL should not be used].

every instance/use of SAGE_LOCAL breaks sagelib on (lets call it) foreign distros a bit. that's not helpful. it will as well interfere with any attempt on rewriting sage-the-distribution (be it autotools based, or pip or ebuild). the autotools approach (not a necessary step, but an example) provides a transition path to anything...

sage-the-distribution is a platform for sage (core) development. no more, no less. other platforms will come and go. what is needed is sagelib without the dependency on this (and on SAGE_LOCAL). why: because developers should be able to use sagelib and develop sage extensions on *their own platforms*, with *their own* tools and *their own* review policies.

so: please embrace contributions that reduce the use of SAGE_LOCAL.

(yes, its getting off-topic. but i hope, this answers the question.)

comment:26 in reply to: ↑ 22 Changed 3 years ago by mkoeppe

Replying to jdemeyer:

Thinking about it more, I disagree with building in $SAGE_BUILD_DIR/sagelib by default.

$SAGE_LOCAL (which contains $SAGE_BUILD_DIR) is meant as install directory, not as build directory.

Let's keep $SAGE_LOCAL to be exactly the install directory and nothing else (*).

I agree with you; see #21479.

Last edited 3 years ago by mkoeppe (previous) (diff)

comment:27 in reply to: ↑ 23 Changed 3 years ago by mkoeppe

Replying to felixs:

outside sage, an install directory is typically what you pass to --prefix, and where stuff is put into by "make install". you do that (if you are a sysadmin), after assuring that "make check" passes...

See #21479 for a discussion of --prefix and make vs. make install.

But let's keep the discussion of the present ticket focused on the task at hand.

Last edited 3 years ago by mkoeppe (previous) (diff)

comment:28 in reply to: ↑ 22 Changed 3 years ago by mkoeppe

Replying to jdemeyer:

I do agree with making the build directory configurable for VPATH builds. For typical packages however, if you do not do a VPATH build, the build directory is the same as the source directory. Sage should follow the same model. This means that the build directory should be $SAGE_SRC by default.

I agree on this as well. #21469 (VPATH for distro) will do that.

In this ticket, I first want to make the --build-base work. In particular, get rid of SAGE_CYTHONIZED.

This also works towards the eventual goal of making 'sagelib' pip-installable.

The actual location used is a detail that will be easy to change later.

(*) One could argue that $SAGE_BUILD_DIR is currently used as build directory for packages. That is true, but they are only used temporarily, they are not meant to actually store stuff. So this isn't so bad.

Yes, the current patch follows this practice. But this is temporary until #21469 is done.

comment:29 in reply to: ↑ 25 ; follow-up: Changed 3 years ago by jdemeyer

Replying to felixs:

Why do you think that $SAGE_LOCAL is not the install directory?

you wrote it. "--prefix" is not implemented/supported.

The fact that the installation process does not implement ./configure --prefix does not mean that it's not an installation process...

every instance/use of SAGE_LOCAL breaks sagelib

But why?

Of course, it's all a matter of definition. I consider the process of copying files to $SAGE_LOCAL an "installation" and you do not (for reasons which are still unclear to me). I think things will become much simpler for you if you accept the fact that $SAGE_LOCAL is the installation directory and that copying files to $SAGE_LOCAL is an installation.

but i hope, this answers the question

I absolutely does not. You are just saying "it breaks stuff" but not explaining why.

comment:30 in reply to: ↑ 29 ; follow-up: Changed 3 years ago by dimpase

Replying to jdemeyer:

Replying to felixs:

[...]

but i hope, this answers the question

I absolutely does not. You are just saying "it breaks stuff" but not explaining why.

One reason why is that in many deployment scenarios you will want to test what you have built before installing it. And Sage does not support this.

comment:31 follow-up: Changed 3 years ago by felixs

indeed. in "build"-"check"-"install" there is not much room for definitions of "build" and "install".

this is drifting towards the question of whether or not sage should stick to common practices and terminology. you know i think it should.

apologies for being off-topic again, this may be more related to #15105 -- there is no ticket for just "practices and terminology".

comment:32 in reply to: ↑ 30 ; follow-up: Changed 3 years ago by jdemeyer

Replying to dimpase:

One reason why is that in many deployment scenarios you will want to test what you have built before installing it. And Sage does not support this.

How does this relate to the existence of $SAGE_LOCAL?

comment:33 Changed 3 years ago by git

  • Commit changed from a73fa065f5030c3b260c04a7e36867fd7f89362f to a854831e3274b69eadc52eb4642b8c0bd221a391

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

a854831Put setup.sh in charge of all sagelib building

comment:34 in reply to: ↑ 31 Changed 3 years ago by jdemeyer

Replying to felixs:

this is drifting towards the question of whether or not sage should stick to common practices and terminology. you know i think it should.

If those "common" practices and terminology make sense, of course it should.

Anyway, I am completely not following what you are trying to say. I feel that you are always wandering around my questions instead of answering them.

comment:35 Changed 3 years ago by mkoeppe

I have created #21495 as a place for such discussions.

comment:36 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:37 in reply to: ↑ 32 ; follow-up: Changed 3 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

One reason why is that in many deployment scenarios you will want to test what you have built before installing it. And Sage does not support this.

How does this relate to the existence of $SAGE_LOCAL?

$SAGE_LOCAL is fine as long as you can also install things from there somewhere else. Currently you cannot do this in a proper way (yes, you can certainly create symbolic links etc, but this is often not enough).

comment:38 in reply to: ↑ 37 Changed 3 years ago by jdemeyer

Replying to dimpase:

$SAGE_LOCAL is fine as long as you can also install things from there somewhere else. Currently you cannot do this in a proper way

Of course you can:

$ cd $package_source
$ sage --sh
$ ./configure --prefix="$SAGE_LOCAL"
$ make install

I have done this many times as first step of testing a new package or a package upgrade.

comment:39 Changed 3 years ago by mkoeppe

If I understand Dima correctly, he wants to install "from" $SAGE_LOCAL, not "to" $SAGE_LOCAL.

But really this discussion should go to #21495. Thanks!

comment:40 Changed 3 years ago by git

  • Commit changed from a854831e3274b69eadc52eb4642b8c0bd221a391 to 1a027d5d535f4dbf7314479e8c6905ad8af34bca

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

1a027d5Have setup.sh derive SAGE_CYTHONIZED from --build-base

comment:41 Changed 3 years ago by mkoeppe

  • Status changed from needs_work to needs_review

Jeroen, how does this look to you now? (I haven't changed $SAGE_BUILD_DIR yet, but will in a moment.)

comment:42 Changed 3 years ago by git

  • Commit changed from 1a027d5d535f4dbf7314479e8c6905ad8af34bca to f6555cea1f1695c99a9011ba13bd64c6995d17fc

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

f6555ceFix last change - handle --build-base earlier

comment:43 Changed 3 years ago by git

  • Commit changed from f6555cea1f1695c99a9011ba13bd64c6995d17fc to d829eb435729cf3280a5206d005bffef8e1dfd52

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

853a1aaMake src/Makefile a generated file
1931b43Define SAGE_PKGS in sage.env, not sage.misc.package
c1dc4f8Set SAGE_CYTHONIZED earlier
d829eb4Build in $(abs_builddir)/build-sagelib. Poison directory env variables

comment:44 Changed 3 years ago by mkoeppe

  • Description modified (diff)
  • Summary changed from Keep src/ clean by using --build-base when building sagelib to Make sagelib setup.sh self-contained, independent of SAGE_ROOT, and handle --build-base

I've changed the description of this ticket, as it has changed its focus. Ready for review.

comment:45 Changed 3 years ago by git

  • Commit changed from d829eb435729cf3280a5206d005bffef8e1dfd52 to ef0919c1bc7d7182d79ae725c97ce057b8336d34

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

57db699Revert "Make SAGE_BUILD_DIR variable available in sage-env, not just in sage-spkg"
ef0919cFixup for SAGE_CYTHONIZED

comment:46 Changed 3 years ago by git

  • Commit changed from ef0919c1bc7d7182d79ae725c97ce057b8336d34 to bd670afd3c7510113b686a9b1545873bda5165dc

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

8ccd440Add src/generate_py_source.mk
bd670afIgnore generated files

comment:47 Changed 3 years ago by git

  • Commit changed from bd670afd3c7510113b686a9b1545873bda5165dc to 751bd0fbde10aa234867122308c7bb76673cbaba

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

751bd0fReword TODO item

comment:48 Changed 3 years ago by mkoeppe

  • Cc jhpalmieri vdelecroix saraedum slabbe nthiery added

comment:49 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Make sagelib setup.sh self-contained, independent of SAGE_ROOT, and handle --build-base to Make sagelib setup.py self-contained, independent of SAGE_ROOT, and handle --build-base

comment:50 Changed 3 years ago by mmezzarobba

  • Cc mmezzarobba added

comment:51 follow-up: Changed 3 years ago by jdemeyer

I would prefer to do this ticket after #21479 because there might be non-trivial interactions.

comment:52 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Also in the code setup.sh -> setup.py

comment:53 Changed 3 years ago by jdemeyer

Regarding

(cd $(srcdir) && export SAGE_ROOT=/doesnotexist SAGE_SRC=/doesnotexist SAGE_SRC_ROOT=/doesnotexist SAGE_DOC_SRC=/doesnotexist SAGE_SCRIPTS_DIR=/doesnotexist SAGE_BUILD_DIR=/doesnotexist SAGE_CYTHONIZED=/doesnotexist SAGE_PKGS=$(abs_top_srcdir)/build/pkgs && python -u setup.py build --build-base=$(abs_builddir)/build-sagelib install)

Did you know that make supports backslash-continued lines? :-)

comment:54 follow-up: Changed 3 years ago by jdemeyer

For backwards compatibility, can we please keep the name build instead of changing it to build-sagelib?

comment:55 follow-up: Changed 3 years ago by jdemeyer

What is the rationale for splitting the Makefile in two (Makefile and generate_py_source.mk)? That seems like additional complication without reason.

In any case, using make is wrong. You need to use os.environ["MAKE"].

comment:56 Changed 3 years ago by git

  • Commit changed from 751bd0fbde10aa234867122308c7bb76673cbaba to 3a8cc0e1bc0c09142275a7ea6b03775e83d73e28

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

3a8cc0eFix typo in comment

comment:57 in reply to: ↑ 54 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

For backwards compatibility, can we please keep the name build instead of changing it to build-sagelib?

If I keep the default, it's hard to demonstrate that "--build-base" actually works.

Moreover, there are too many things already called "build" -- better be specific.

comment:58 in reply to: ↑ 55 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

What is the rationale for splitting the Makefile in two (Makefile and generate_py_source.mk)? That seems like additional complication without reason.

setup.sh needs to be in charge of building everything, if sagelib is to become a well-behaved Python package.

comment:59 follow-up: Changed 3 years ago by mkoeppe

generate_py_source.mk could certainly be implemented in pure Python and could become a part of setup.py. But I don't want to work on this.

comment:60 Changed 3 years ago by git

  • Commit changed from 3a8cc0e1bc0c09142275a7ea6b03775e83d73e28 to 0dd9c50021302e4d8cfc6f95e8bbdea232c60b97

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

0dd9c50Respect environment variable MAKE

comment:61 Changed 3 years ago by git

  • Commit changed from 0dd9c50021302e4d8cfc6f95e8bbdea232c60b97 to 17f90d8d1e8b35f9ae08bb98bb876083ba968824

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

17f90d8beautification

comment:62 in reply to: ↑ 57 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

Replying to jdemeyer:

For backwards compatibility, can we please keep the name build instead of changing it to build-sagelib?

If I keep the default, it's hard to demonstrate that "--build-base" actually works.

Well, it's easy for the reviewer to test a different value of --build-base.

Moreover, there are too many things already called "build" -- better be specific.

But src/build is pretty clear: it is the build directory corresponding to src, which is the Sage library. I don't like to change it just for the sake of changing it.

comment:63 in reply to: ↑ 59 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

generate_py_source.mk could certainly be implemented in pure Python and could become a part of setup.py

That doesn't answer my question. Why do you not keep this in src/Makefile?

comment:64 in reply to: ↑ 58 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

setup.sh needs to be in charge of building everything, if sagelib is to become a well-behaved Python package.

Sorry, I didn't see this comment. Okay, that makes sense but it wasn't clear to me,

I guess you should add some comments to src/Makefile to explain this better than just

## All sagelib-building is done by setup.py.
## DON'T ADD ADDITIONAL STEPS TO THIS MAKEFILE.

comment:65 Changed 3 years ago by git

  • Commit changed from 17f90d8d1e8b35f9ae08bb98bb876083ba968824 to e5f90659f21cc7ac1e39cdcf7974dba2ba223da5

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

e5f9065More comments

comment:66 in reply to: ↑ 51 Changed 3 years ago by jdemeyer

Replying to jdemeyer:

I would prefer to do this ticket after #21479 because there might be non-trivial interactions.

Or at least I will test it with #21501 applied.

comment:67 follow-up: Changed 3 years ago by mkoeppe

OK, I'll have #21479 ready in a few days.

comment:68 Changed 3 years ago by jdemeyer

I just tested this with #21501. It mostly works but there are these two doctest failures:

sage -t src/sage_setup/find.py
**********************************************************************
File "src/sage_setup/find.py", line 107, in sage_setup.find.find_extra_files
Failed example:
    find_extra_files(["sage.modular.arithgroup"], SAGE_SRC, SAGE_CYTHONIZED, ".")
Expected:
    [('./sage/modular/arithgroup',
      ['.../src/sage/modular/arithgroup/farey.pxd', ...farey_symbol.h...])]
Got:
    [('./sage/modular/arithgroup',
      ['/usr/local/src/sage-config/src/sage/modular/arithgroup/farey.pxd'])]
**********************************************************************
File "src/sage_setup/clean.py", line 87, in sage_setup.clean._find_stale_files
Failed example:
    for f in stale_iter:
        if f.endswith(skip_extensions): continue
        print('Found stale file: ' + f)
Expected nothing
Got:
    Found stale file: sage/stats/distributions/dgs_gauss.h
    Found stale file: sage/libs/lcalc/lcalc_sage.h
    Found stale file: sage/geometry/triangulation/data.h
    Found stale file: sage/misc/cython_metaclass.h
    Found stale file: sage/rings/finite_rings/stdint.h
    Found stale file: sage/stats/distributions/dgs_misc.h
    Found stale file: sage/misc/darwin_memory_usage.h
    Found stale file: sage/stats/distributions/dgs.h
    Found stale file: sage/ext/python_debug.h
    Found stale file: sage/libs/eclsig.h
    Found stale file: sage/ext/solaris_fixes.h
    Found stale file: sage/ext/pyx_visit.h
    Found stale file: sage/matroids/minorfix.h
    Found stale file: sage/modular/arithgroup/farey_symbol.h
    Found stale file: sage/ext/mod_int.h
    Found stale file: sage/combinat/matrices/dancing_links_c.h
    Found stale file: sage/ext/interpreters/wrapper_rr.h
    Found stale file: sage/symbolic/ginac_wrap.h
    Found stale file: sage/ext/interpreters/wrapper_cdf.h
    Found stale file: sage/ext/interpreters/wrapper_el.h
    Found stale file: sage/groups/perm_gps/partn_ref2/refinement_generic.h
    Found stale file: sage/libs/polybori/pb_wrap.h
    Found stale file: sage/sat/solvers/cryptominisat/solverconf_helper.h
    Found stale file: sage/combinat/partitions_c.h
    Found stale file: sage/ext/ccobject.h
    Found stale file: sage/geometry/triangulation/triangulations.h
    Found stale file: sage/libs/ntl/ntlwrap.h
    Found stale file: sage/stats/distributions/dgs_bern.h
    Found stale file: sage/sat/solvers/cryptominisat/cryptominisat_helper.h
    Found stale file: sage/libs/pari/parisage.h
    Found stale file: sage/geometry/triangulation/functions.h
**********************************************************************

comment:69 follow-up: Changed 3 years ago by jdemeyer

One thing I don't like about the approach of this ticket is that SAGE_CYTHONIZED is set by the value of --build-base. I would much rather have it the other way around: that some environment variable is set by configure which is then used to define the value for --build-base and for the environment variable SAGE_CYTHONIZED.

This would be more analogous to #21501 where I want to set $SAGE_LOCAL just once and then derive everything from that.

comment:70 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:71 in reply to: ↑ 69 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

One thing I don't like about the approach of this ticket is that SAGE_CYTHONIZED is set by the value of --build-base. I would much rather have it the other way around: that some environment variable is set by configure which is then used to define the value for --build-base and for the environment variable SAGE_CYTHONIZED.

This would be more analogous to #21501 where I want to set $SAGE_LOCAL just once and then derive everything from that.

I've updated the description to explain better why --build-base needs to be in charge of setting all build directories.

comment:72 follow-up: Changed 3 years ago by mkoeppe

But yes, I'll introduce something like SAGE_SRC_BUILDBASE that can be used in src/Makefile.in and to define SAGE_CYTHONIZED for that doctest.

comment:73 in reply to: ↑ 71 ; follow-up: Changed 3 years ago by jdemeyer

Replying to mkoeppe:

I've updated the description to explain better why --build-base needs to be in charge of setting all build directories.

Sorry, but the paragraph you added does not explain this. It explains better the what but not the why. This reminds me of what I recently said at https://trac.sagemath.org/ticket/21469#comment:17

comment:74 in reply to: ↑ 72 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

But yes, I'll introduce something like SAGE_SRC_BUILDBASE that can be used in src/Makefile.in and to define SAGE_CYTHONIZED for that doctest.

Now I am even more lost...

First you say that --build-base should determine everything but this comment seems to contradict that.

comment:75 Changed 3 years ago by mkoeppe

  • Description modified (diff)

comment:76 in reply to: ↑ 73 Changed 3 years ago by mkoeppe

Replying to jdemeyer:

Replying to mkoeppe:

I've updated the description to explain better why --build-base needs to be in charge of setting all build directories.

Sorry, but the paragraph you added does not explain this. It explains better the what but not the why.

I've created ticket #21507 to put the technical goals of the present ticket in the context of a bigger goal. Please take a look.

comment:77 follow-up: Changed 3 years ago by jdemeyer

For this ticket, can you please keep SAGE_CYTHONIZED independent of --build-base (and set them both from the common environment variable you mention in 72). That will also solve the doctest failures from 68.

The current code which figures out SAGE_CYTHONIZED from the command line is quite ugly and fragile. I think that we can do a bigger reorganization of setup.py which will make things simpler. Also Cython 0.25 (not released yet) has some potential to simplify cythonization.

PS: I like the fact that you are splitting this up over many tickets, it gets things moving more quickly.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:78 in reply to: ↑ 67 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

OK, I'll have #21479 ready in a few days.

Actually, maybe this doesn't really need to depend on #21479, since it doesn't involve ./configure. If you address the points I made, then this might be good to go.

comment:79 in reply to: ↑ 77 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

For this ticket, can you please keep SAGE_CYTHONIZED independent of --build-base (and set them both from the common environment variable you mention in 72). That will also solve the doctest failures from 68.

I disagree. I consider setup.py deriving SAGE_CYTHONIZED from --build-base a crucial feature of this patch. What I meant above is simply that setup.py will set an intermediate variable, rather than SAGE_CYTHONIZED, and SAGE_CYTHONIZED would be determined by env.py.

The current code which figures out SAGE_CYTHONIZED from the command line is quite ugly and fragile. I think that we can do a bigger reorganization of setup.py which will make things simpler. Also Cython 0.25 (not released yet) has some potential to simplify cythonization.

I am aware that the arg parsing code is ad hoc, but it's easy to understand code.

I do not want to defer getting --build-base working to some distant future, when someone finds the time to clean up setup.py.

comment:80 in reply to: ↑ 79 ; follow-up: Changed 3 years ago by jdemeyer

Replying to mkoeppe:

I do not want to defer getting --build-base working to some distant future, when someone finds the time to clean up setup.py.

At least you shouldn't determine the directory from the command line. That's too fragile to work correctly. You should find a way to get the directory from distutils which is going to require some refactoring anyway.

comment:81 in reply to: ↑ 80 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

At least you shouldn't determine the directory from the command line. That's too fragile to work correctly. You should find a way to get the directory from distutils which is going to require some refactoring anyway.

The current structure of setup.py does not make this easy. SAGE_CYTHONIZED is used before setup is even invoked. It can't be the job of this ticket to bring setup.py to standard distutils behavior.

comment:82 Changed 3 years ago by mkoeppe

I've created a ticket #21508 for setup.py cleanup issues.

comment:83 in reply to: ↑ 81 ; follow-up: Changed 3 years ago by jdemeyer

Replying to mkoeppe:

The current structure of setup.py does not make this easy.

I know! That exactly why I proposed in 77 to postpone this piece of your patch.

If you really want to go for #21507, you will need a lot of changes to setup.py anyway. So the cleanup will need to happen sooner or later.

comment:84 in reply to: ↑ 83 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

Replying to mkoeppe:

The current structure of setup.py does not make this easy.

I know! That exactly why I proposed in 77 to postpone this piece of your patch.

I want the feature of --build-base now, not in the distant future. It is easily achieved with this patch. The arg parsing is ad hoc; but given the current state of setup.py, which does not take care at all about standard distutils behavior, it is an improvement.

If you really want to go for #21507, you will need a lot of changes to setup.py anyway. So the cleanup will need to happen sooner or later.

I prefer to do one concrete step (technical goal) at a time. Let's get --build-base working right now.

Last edited 3 years ago by mkoeppe (previous) (diff)

comment:85 in reply to: ↑ 84 ; follow-up: Changed 3 years ago by jdemeyer

Replying to mkoeppe:

Let's get --build-base working right now.

In my opinion, this branch is not making --build-base work. This branch is adding hacks to pretend like --build-base is working.

In general, hacks can have their place, but only if there is a clear reason for them. In this case, I see no reason. This ticket has no reason to exist except for going towards #21507. And this hack that you added will get us no closer to #21507 since we will need to refactor the SAGE_CYTHONIZED code anyway.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:86 Changed 3 years ago by jdemeyer

Anyway, I will leave this issue for other people to comment, since we probably won't come to an agreement.

comment:87 in reply to: ↑ 85 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jdemeyer:

Replying to mkoeppe:

Let's get --build-base working right now.

In my opinion, this branch is not making --build-base work. This branch is adding hacks to pretend like --build-base is working.

It does implement --build-base correctly.

In general, hacks can have their place, but only if there is a clear reason for them. In this case, I see no reason. This ticket has no reason to exist except for going towards #21507. And this hack that you added will get us no closer to #21507 since we will need to refactor the SAGE_CYTHONIZED code anyway.

Apart from #21507, it is also needed for the VPATH build, as explained in the description.

comment:88 follow-ups: Changed 3 years ago by fbissey

Hum. I am not sure myself. I install sagelib in sage-on-gentoo like a normal package (I just don't install sage_setup and do not clean before hand since this is done by the package manager) with python setup.py build and then python setup.py install --root=.... I guess pip may have extra requirements. Of course I patch a lot of the sagelib code itself to help with various issues, and this ticket does nothing for them. But given your goals, I'd say you'll have to deal with most of them.

There are definitely things to do and getting setup.py to control the autogenerated files may help me. I will have to see it in action before forming a definitive opinion.

My other concerns while they could influence design here are separate issues:

  • separation of build time, doctest time and runtime variables. SAGE_SRC shouldn't be found in runtime code for example, neither does package.py belong there.
  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

comment:89 Changed 3 years ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:90 in reply to: ↑ 88 Changed 3 years ago by mkoeppe

Replying to fbissey:

  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

Are you saying that it should be installed under SAGE_LOCAL? This should be a separate ticket.

comment:91 follow-up: Changed 3 years ago by fbissey

Agreed on a separate ticket. But I mention cython_debug because it is mentioned in the summary. I am pointing to the fact that it should be installable when it is currently not. I am on an awareness campaign :)

comment:92 in reply to: ↑ 87 Changed 3 years ago by jdemeyer

Replying to mkoeppe:

Apart from #21507, it is also needed for the VPATH build, as explained in the description.

You don't need the --build-base hack for VPATH builds to work. You could just set SAGE_CYTHONIZED correctly from the Makefile.

comment:93 in reply to: ↑ 91 Changed 3 years ago by mkoeppe

Replying to fbissey:

Agreed on a separate ticket. But I mention cython_debug because it is mentioned in the summary. I am pointing to the fact that it should be installable when it is currently not. I am on an awareness campaign :)

I've created #21509 for this; but I guess you should explain more on that ticket.

comment:94 Changed 3 years ago by git

  • Commit changed from e5f90659f21cc7ac1e39cdcf7974dba2ba223da5 to 74169e75b0c650e6b1070c6beca231422cf62eaa

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

7791cd9Remove --buildbase code
74169e7Pass SAGE_SRC to generate_py_source.mk

comment:95 Changed 3 years ago by mkoeppe

  • Description modified (diff)
  • Summary changed from Make sagelib setup.py self-contained, independent of SAGE_ROOT, and handle --build-base to Make sagelib setup.py self-contained and independent of SAGE_ROOT

comment:96 Changed 3 years ago by mkoeppe

I have simplified this ticket to facilitate easier review. Jeroen, please take a look.

comment:97 Changed 3 years ago by git

  • Commit changed from 74169e75b0c650e6b1070c6beca231422cf62eaa to 0394333a54e51c53b9404f248220a412cf13e00e

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

0394333Add new file to MANIFEST.in

comment:98 Changed 3 years ago by jdemeyer

Looks good on first sight, but I need to test it.

comment:99 Changed 3 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:100 Changed 3 years ago by mkoeppe

Thanks!

comment:101 Changed 3 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Trouble with Jupyter...

comment:102 Changed 3 years ago by jdemeyer

src/setup.py runs code from src/sage/repl/ipython_kernel/install.py which still uses some of the directories that you blacklist.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:103 Changed 2 years ago by git

  • Commit changed from 0394333a54e51c53b9404f248220a412cf13e00e to fdedb029434cba49db4249c5511af8c8efc08217

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

fdedb02Install SAGE_LOCAL/bin/sage instead of SAGE_ROOT/sage as Jupyter kernel

comment:104 follow-up: Changed 2 years ago by mkoeppe

With this change I've tested that I can do sage -n jupyter and then run a kernel from the Jupyter notebook. SAGE_LOCAL/bin/sage picks up its environment variables from the server that runs it, so everything works. Does it have to work in other settings that I'm unaware of as well?

comment:105 Changed 2 years ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:106 Changed 2 years ago by jdemeyer

What about the use of SAGE_DOC and SAGE_EXTCODE in that file?

comment:107 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:108 Changed 2 years ago by mkoeppe

SAGE_DOC and SAGE_EXTCODE are fine as they are subdirectories of SAGE_LOCAL. The patch does not poison any environment variables that are subdirectories of SAGE_LOCAL.

comment:109 Changed 2 years ago by embray

Very interesting--I'd like to take some time to review this and all the comments on it. I probably won't get to it in detail until next week but I'd really like a chance to look it over.

comment:110 in reply to: ↑ 104 ; follow-up: Changed 2 years ago by jdemeyer

Replying to mkoeppe:

With this change I've tested that I can do sage -n jupyter and then run a kernel from the Jupyter notebook.

Did you test with a custom value for SAGE_LOCAL (#21501)? Because that should be done (I could do that, but not right now).

comment:111 follow-up: Changed 2 years ago by mkoeppe

I would probably rather test with --prefix (#21479).

comment:112 in reply to: ↑ 111 ; follow-ups: Changed 2 years ago by fbissey

Replying to mkoeppe:

I would probably rather test with --prefix (#21479).

From a distro point of view, I would also want it to be tested with the underdocumented --root.

comment:113 in reply to: ↑ 112 Changed 2 years ago by mkoeppe

Replying to fbissey:

Replying to mkoeppe:

I would probably rather test with --prefix (#21479).

From a distro point of view, I would also want it to be tested with the underdocumented --root.

Is --root an option of setup.sh? (I was referring to ./configure --prefix)

comment:114 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:115 Changed 2 years ago by mkoeppe

(I meant setup.py of course -- I keep making the typo "setup.sh")

comment:116 Changed 2 years ago by fbissey

Yes, setup.py I should abstain from commenting until my fever goes down :(

comment:117 in reply to: ↑ 112 Changed 2 years ago by mkoeppe

Replying to fbissey:

From a distro point of view, I would also want it to be tested with the underdocumented --root.

This is now #21573: Make sure src/setup.py respects --install-base and --root.

comment:118 in reply to: ↑ 110 Changed 2 years ago by mkoeppe

Replying to jdemeyer:

Replying to mkoeppe:

With this change I've tested that I can do sage -n jupyter and then run a kernel from the Jupyter notebook.

Did you test with a custom value for SAGE_LOCAL (#21501)? Because that should be done (I could do that, but not right now).

I've now tested with #21501 and a fresh build with SAGE_LOCAL set to $SAGE_ROOT/very/far/away/but/still/local. The Jupyter notebook works.

comment:119 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Good enough for me.

comment:120 follow-up: Changed 2 years ago by embray

Not that it really matters, but python -u setup.py build install is superfluous--the install command always runs build as a sub-command.

comment:121 Changed 2 years ago by embray

For the sake of keeping this ticket focused I think it's fine for now, but something definitely needs to be done about using make for the autogenerated sources. My comment in #21507 has some suggestions toward that. Is there already a separate ticket for dealing with that? If so I can work on it.

comment:122 follow-up: Changed 2 years ago by embray

Also, if you'd like, I can suggest a much less "hacky" way of handling --build-base, is there already a separate ticket for that?

comment:123 in reply to: ↑ 122 Changed 2 years ago by jdemeyer

Replying to embray:

Also, if you'd like, I can suggest a much less "hacky" way of handling --build-base, is there already a separate ticket for that?

#21573 I guess.

comment:124 Changed 2 years ago by mkoeppe

#21573 is for --install-base and --root.

--build-base work should go on either one of the following two:

  • #21508: Clean up src/setup.py to bring it to standard distutils behavior
  • #21535: Make src/setup.py independent of SAGE_CYTHONIZED

comment:125 in reply to: ↑ 120 Changed 2 years ago by mkoeppe

Replying to embray:

Not that it really matters, but python -u setup.py build install is superfluous--the install command always runs build as a sub-command.

I've left it like this as a little reminder that --build-base can go in between.

comment:126 follow-up: Changed 2 years ago by embray

  • Description modified (diff)

For generate_py_source.mk, we could even get rid of that and not invoke make at all. For simplicity's sake I could do that as a separate ticket with this one as a dependency.

Version 0, edited 2 years ago by embray (next)

comment:127 in reply to: ↑ 88 ; follow-up: Changed 2 years ago by jdemeyer

Replying to fbissey:

  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

On the other hand, debugging also requires the sources to be available. So it is not a crazy idea to keep the debug info close to the sources.

Maybe the location for cython_debug can be improved, but I think you should only install it in SAGE_LOCAL if you also install the sources somewhere in SAGE_LOCAL.

comment:128 in reply to: ↑ 126 ; follow-up: Changed 2 years ago by jdemeyer

Replying to embray:

For generate_py_source.mk, we could even get rid of that and not invoke make at all.

But make does automatic dependency checking for you. Whatever Python alternative you come up with will be more complex than a simple makefile.

comment:129 Changed 2 years ago by embray

I would disagree. In fact as the discussion in #20730 demonstrates that writing a technically correct makefile for this case results in a makefile that's "too complicated to understand". I can do this correctly in Python with about the same amount of code it takes to invoke make as a subprocess (which is also pretty non-standard to do anything in a Python setup). Not to mention the contents of the Makefile itself (no matter how "correct" it is).

Part of the idea is also not to make non-Sage developers blanche when they look inside of sage's setup.py.

Last edited 2 years ago by embray (previous) (diff)

comment:130 in reply to: ↑ 127 ; follow-up: Changed 2 years ago by embray

Replying to jdemeyer:

Replying to fbissey:

  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

On the other hand, debugging also requires the sources to be available. So it is not a crazy idea to keep the debug info close to the sources.

This is why I also like to output the C(++) sources themselves to be close to the cython sources but you said you don't like :)

comment:131 in reply to: ↑ 128 Changed 2 years ago by felixs

Replying to jdemeyer:

But make does automatic dependency checking for you. Whatever Python alternative you come up with will be more complex than a simple makefile.

true.

there is a python alternative that is no more complex than a simple makefile *plus* make. it's the simple makefile + a *subset* of make implemented in python. (yes, you could use GNU make as an intermediate solution...)

but seriously: what's all the fuss about reinventing and replacing make? it should obviously be the other way round: python is not the right tool for the job, it's make. unlike python, make is *designed* for this. no matter which language you implement it in.

if there's a need for a make replacement (completely different[tm]), is sage the right community or the right project to invent it in?

comment:132 in reply to: ↑ 130 Changed 2 years ago by fbissey

Replying to embray:

Replying to jdemeyer:

Replying to fbissey:

  • Is there a standard place where cython_debug can be installed? It is needed for debugging and shouldn't live under SAGE_SRC.

On the other hand, debugging also requires the sources to be available. So it is not a crazy idea to keep the debug info close to the sources.

This is why I also like to output the C(++) sources themselves to be close to the cython sources but you said you don't like :)

For the record: in sage-on-gentoo I ship the .pyx files next to the matching .so files. In any case distro ship debugging symbols without the sources. Of course at some point you may need the source code but this is trivial to get and unfold separately in most cases.

comment:133 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

This ticket introduces an autotools dependency (src/Makefile.in) and fails on OSX

comment:134 Changed 2 years ago by jdemeyer

The is the usual breakage of your release management scripts in case of changes to configure.

comment:135 Changed 2 years ago by jdemeyer

  • Branch changed from u/mkoeppe/keep_src__clean_by_using___build_base_when_building_sagelib to u/jdemeyer/keep_src__clean_by_using___build_base_when_building_sagelib

comment:136 Changed 2 years ago by jdemeyer

  • Commit changed from fdedb029434cba49db4249c5511af8c8efc08217 to 78ab5d7f5a8b6f1b89ba3be59eda6ff9ce77a91b
  • Description modified (diff)
  • Status changed from needs_work to positive_review

New commits:

2cf7352Merge tag '7.4.beta6' into t/21480/keep_src__clean_by_using___build_base_when_building_sagelib
78ab5d7Bump configure version

comment:137 follow-up: Changed 2 years ago by mkoeppe

This now conflicts with the configure spkg version change in 7.4.rc0. Seems to me incrementing configure spkg versions is something that should be done by the release manager.

comment:138 in reply to: ↑ 137 Changed 2 years ago by jdemeyer

Replying to mkoeppe:

This now conflicts with the configure spkg version change in 7.4.rc0. Seems to me incrementing configure spkg versions is something that should be done by the release manager.

I know, it is really annoying.

comment:139 Changed 2 years ago by jdemeyer

This problem happens every time that you make non-trivial changes to configure.ac

comment:140 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:141 Changed 2 years ago by git

  • Commit changed from 78ab5d7f5a8b6f1b89ba3be59eda6ff9ce77a91b to 0c2ac9583ed97ffc74e99c876a8cf4506f5b0162
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

2871769Merge tag '7.4.rc0' into t/21480/keep_src__clean_by_using___build_base_when_building_sagelib
0c2ac95Bump configure version

comment:142 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:143 Changed 2 years ago by jdemeyer

  • Priority changed from major to blocker

This should be merged as soon as possible to avoid further problems with configure.

comment:144 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/keep_src__clean_by_using___build_base_when_building_sagelib to 0c2ac9583ed97ffc74e99c876a8cf4506f5b0162
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.