Opened 9 months ago
Last modified 3 months ago
#32913 needs_review defect
Revert #32713 (Apply "configure enableeditable" to other packages) for sageconf, sagesetup
Reported by:  mkoeppe  Owned by:  

Priority:  critical  Milestone:  sageduplicate/invalid/wontfix 
Component:  build  Keywords:  
Cc:  ghtobiasdiez, klee  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/build/trac_editable (Commits, GitHub, GitLab)  Commit:  2a302097ca6764d8305667f93f074609b76a32f1 
Dependencies:  Stopgaps: 
Description (last modified by )
(from https://trac.sagemath.org/ticket/32713#comment:11)
Users need the wheel of sageconf when creating venvs. And we need the wheels of packages sageconf and sagesetup when testing distributions using ./sage sh c '(cd pkgs/sagemathstandard && tox v v v e pythonsagewheelsnopypi)'
and similar  as documented in the developer's guide as of #32899.
Until editable wheels (PEP 660) are implemented in setuptools (or we switch the distributions to another build system that implements PEP 660 such as flit), we revert the change made in #32713 for these packages.
But we keep sagedocbuild editable.
Change History (22)
comment:1 Changed 9 months ago by
 Description modified (diff)
comment:2 Changed 9 months ago by
 Branch set to u/mkoeppe/revert__32713__apply__configure___enable_editable__to_other_packages__for_sage_conf__sage_setup
comment:3 Changed 9 months ago by
 Commit set to 2cc04976e540af5452913d34948c9a74d088bff5
 Status changed from new to needs_review
comment:4 followups: ↓ 5 ↓ 6 Changed 9 months ago by
Can you please expand on why this leads to problems? For example, endusers should normally not use an editable install anyway, neither should the build process for the wheels on ci. For the tox cmd, wouldn't be better if the tox config of sagemathstandard only applies the e only to that package if that's the desired behavior.
I would fine it confusing if make enableeditableinstall
installs some packages in editable mode and others not.
comment:5 in reply to: ↑ 4 Changed 9 months ago by
Replying to ghtobiasdiez:
For the tox cmd, wouldn't be better if the tox config of sagemathstandard only applies the e only to that package if that's the desired behavior.
Please take a look at pkgs/sagemathstandard/tox.ini
, then you'll see what it does...
comment:6 in reply to: ↑ 4 Changed 9 months ago by
Replying to ghtobiasdiez:
I would fine it confusing if
configure enableeditable
installs some packages in editable mode and others not.
Hasn't confused anyone so far...
comment:7 Changed 9 months ago by
Also note that configure help
explains:
enableeditable use an editable install of the Sage library
comment:8 Changed 9 months ago by
 Cc klee added
 Description modified (diff)
comment:9 Changed 8 months ago by
Sorry, but I still don't understand why you need to build and use wheels for sage packages if all of them use editable installs. Why can one not install them (as editable) in the tox environments?
For example, something along the following lines (in src/tox):
[testenv] usedevelop=True # Instruct tox to use editable install of main sagemath package deps= editable=file:///{toxinidir}/../path/to/sage_conf # Install sageconf as editable install
Potentially combined with a custom install_command
that invokes pip with findlinks
to discover the wheels of other dependencies that are build during make
.
comment:10 followup: ↓ 13 Changed 8 months ago by
That could be implemented but it is not implemented.
comment:11 Changed 8 months ago by
And the installation procedure would also be more difficult to document than just to say "install all wheels from ...".
comment:12 Changed 8 months ago by
#32616 may be a way to resolve this
comment:13 in reply to: ↑ 10 Changed 8 months ago by
Replying to mkoeppe:
That could be implemented but it is not implemented.
I would strongly prefer that over reverting something that works in principle. I'll have a look the coming days and create a prototype.
comment:14 Changed 8 months ago by
Any progress on this?
comment:15 Changed 8 months ago by
 Branch changed from u/mkoeppe/revert__32713__apply__configure___enable_editable__to_other_packages__for_sage_conf__sage_setup to public/build/trac_editable
 Commit changed from 2cc04976e540af5452913d34948c9a74d088bff5 to 2a302097ca6764d8305667f93f074609b76a32f1
 Status changed from needs_review to needs_work
With the new branch public/build/trac_editable
you can run tox e py38
(or 39) from the src
folder and get a tox environment with editable installs of sagemathstandard and sageconf. Some of the tests are failing (because gap and blas are not using sageconf but determine stuff at runtime during env variables) and there might be a more elegant way to set the env variables but as a prototype this should suffice.
New commits:
2a30209  Add tox env with editable installs

comment:16 Changed 8 months ago by
This branch doesn't fix what is described in the ticket description
comment:17 Changed 8 months ago by
Maybe not (as I said above in comment 4 I don't quite get what your problem is). But it shows how to use editable installs with tox and thus provides a way around the problem "we need the wheels of packages sageconf and sagesetup when testing distributions [with tox]". Also it was meant as a prototype not as a complete solution.
comment:18 Changed 8 months ago by
Well we are in the release phase for Sage 9.5 and can't merge "prototypes"
comment:19 Changed 6 months ago by
 Milestone changed from sage9.5 to sage9.6
Set milestone to sage9.6 after Sage 9.5 release.
comment:20 Changed 3 months ago by
 Milestone changed from sage9.6 to sage9.7
comment:21 Changed 3 months ago by
 Milestone changed from sage9.7 to sageduplicate/invalid/wontfix
I have a different solution in #33817
comment:22 Changed 3 months ago by
 Status changed from needs_work to needs_review
New commits:
build/pkgs/{sage_conf,sage_setup}/install: Do not use sdh_pip_editable_install