Opened 3 years ago

Closed 17 months ago

#29118 closed defect (invalid)

Please roll back #29038

Reported by: Erik Bray Owned by:
Priority: trivial Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: François Bissey, Antonio Rojas, Isuru Fernando, Erik Bray, Ximin Luo, Timo Kaufmann, Jeroen Demeyer, Dima Pasechnik, John Palmieri, Volker Braun Merged in:
Authors: Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: u/embray/ticket-29118 (Commits, GitHub, GitLab) Commit: b9d6cacb68902d30d10bc743c2398b2eed3a3840
Dependencies: Stopgaps:

Status badges

Description (last modified by Erik Bray)

With respect, I think it adds much additional complication without justifying itself or what concrete problems it's trying to solve.

I had to work on other things for a while so I stepped away from the discussion, but I never would have approved it.

Let's please roll this back and stop making major changes to the build system until we can get some clarity on what the overall goals of these changes are.

Please consider this an alternative to #29114 which is adding still more complications to work around a regression introduced by #29038. I think it would be simpler for now to roll back #29038 until we can re-assess.

In the meantime, please see #29119 which offers an alternative approach which I believe is more in the spirit of making sagelib buildable independently of sage-the-distribution without any non-standard external scripts.

Change History (32)

comment:1 Changed 3 years ago by Dima Pasechnik

I think we should progress rather than revert, as Matthias has a clear and mostly ready path towards making sagelib runnable in venv of a system Python, something that is close to pip installability.

Noone else is close to this, and simplifications may happen later.

Besides, there are no regressions introduced in #29038 - a way to implement something that you don't like, but it works, is not a regression, it is a design choice that you make or may not like.

comment:2 Changed 3 years ago by Erik Bray

Authors: Erik Bray
Branch: u/embray/ticket-29118
Commit: b9d6cacb68902d30d10bc743c2398b2eed3a3840
Status: newneeds_review

I hate to be so strongly negative about this because I respect Matthias's effort and I think we have shared goals overall. But I wouldn't be bothered if I weren't strongly convinced that there were a better way, and that this approach does not fully solve the problems it purports to solve.


New commits:

b9d6cacRevert "Trac #29038: Python package sage_conf: Provides optional configuration information for sagelib"

comment:3 Changed 3 years ago by Erik Bray

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

This was part of the point of my approach in #29032 which I believe accomplishes the same goals with far less intervention. I could be wrong about that, but no one has suggested otherwise yet.

comment:4 in reply to:  3 ; Changed 3 years ago by Dima Pasechnik

Replying to embray:

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

I don't think you are right here. Having sagelib working in venv of a system python is almost as good as direct pip-installing, as it allows to get away from endlessly rebuilding python system packages.

Installing sagelib directly into system python packages is useless from development point of view. It's good as a final goal, but we're far from it. You don't want to trash your system python every time things go wrong.

This was part of the point of my approach in #29032 which I believe accomplishes the same goals with far less intervention. I could be wrong about that, but no one has suggested otherwise yet.

comment:5 Changed 3 years ago by Dima Pasechnik

This is too disruptive to revert now, IMHO, as there is too much stuff in the works that depends or might depend upon it.

Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:6 in reply to:  4 ; Changed 3 years ago by Erik Bray

Replying to dimpase:

Replying to embray:

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

I don't think you are right here. Having sagelib working in venv of a system python is almost as good as direct pip-installing, as it allows to get away from endlessly rebuilding python system packages.

Installing sagelib directly into system python packages is useless from development point of view. It's good as a final goal, but we're far from it.

No we're not. Distro packagers already do it. When it comes down to it it's just another Python package, with some variables in sage.env you have to configure (the problem we are trying to solve is that that part is currently too difficult to configure, which is why packagers often just replace sage.env with their own hard-coded one).

You don't want to trash your system python every time things go wrong.

Yes, that's what virtualenvs are for. But there's nothing special about it otherwise. $SAGE_LOCAL is no different in function from a virtualenv--it's just an alternate install prefix, like /usr/local.

comment:7 in reply to:  5 ; Changed 3 years ago by Erik Bray

Replying to dimpase:

This is too disruptive to revert now, IMHO, as there is too much stuff in the works that depends or might depend upon it.

This was already disruptive by definition as it caused at least one regression. What's in the works that depends or might depend on it, and why are those things necessary? And how? None of this has been explained.

Last edited 3 years ago by Erik Bray (previous) (diff)

comment:8 in reply to:  6 ; Changed 3 years ago by Dima Pasechnik

Replying to embray:

Replying to dimpase:

Replying to embray:

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

I don't think you are right here. Having sagelib working in venv of a system python is almost as good as direct pip-installing, as it allows to get away from endlessly rebuilding python system packages.

Installing sagelib directly into system python packages is useless from development point of view. It's good as a final goal, but we're far from it.

No we're not. Distro packagers already do it.

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

When it comes down to it it's just another Python package, with some variables in sage.env you have to configure (the problem we are trying to solve is that that part is currently too difficult to configure, which is why packagers often just replace sage.env with their own hard-coded one).

You don't want to trash your system python every time things go wrong.

Yes, that's what virtualenvs are for. But there's nothing special about it otherwise. $SAGE_LOCAL is no different in function from a virtualenv--it's just an alternate install prefix, like /usr/local.

Sage with itsSAGE_LOCAL is not a Python virtual environment, it's more Conda-like virtual environment. I think it's agreed that we don't want to be another Conda, but rather try to undo this. By continuing to do custom things to SAGE_LOCAL instead of using correct Python tools, e.g. Python modules, Python venv, we work against the latter goal.

comment:9 in reply to:  7 Changed 3 years ago by Dima Pasechnik

Replying to embray:

Replying to dimpase:

This is too disruptive to revert now, IMHO, as there is too much stuff in the works that depends or might depend upon it.

This was already disruptive by definition as it caused at least one regression. What's in the works that depends or might depend on it, and why are those things necessary? And how? None of this has been explained.

Wrapping some of Sage's spaghetti configuration into a Python module --- that's what #29038 mostly does and why it is welcome, cause it's a move in the right direction, from a place where much feet-dragging has taken place (so yes indeed, disruptive, in the positive sense).

comment:10 Changed 3 years ago by Erik Bray

See #29119 for an alternative proposal which I think meets the same needs.

comment:11 in reply to:  8 Changed 3 years ago by Erik Bray

Replying to dimpase:

Replying to embray:

Yes, that's what virtualenvs are for. But there's nothing special about it otherwise. $SAGE_LOCAL is no different in function from a virtualenv--it's just an alternate install prefix, like /usr/local.

Sage with itsSAGE_LOCAL is not a Python virtual environment, it's more Conda-like virtual environment. I think it's agreed that we don't want to be another Conda, but rather try to undo this. By continuing to do custom things to SAGE_LOCAL instead of using correct Python tools, e.g. Python modules, Python venv, we work against the latter goal.

I'm sorry, but this is meaningless.

Can you please explain to me exactly what you think a virtualenv/venv is in Python? Because I think you ascribe a lot more depth and complexity to it than it actually is.

In practice there's little distinction between a virtualenv and a Conda env. In fact, Conda started out originally as part of the build system for Anaconda's spiritual predecessor, Canopy. Back then it was just a virtualenv plus some additional bells-and-whistles (such as setting the correct environment variables) to be able to install non-Python packages into it. The main difference with Conda envs--where they went beyond the original concept--was to not be tied to a specific Python interpreter--but in fact allow installing different Pythons into the environment as well.

I don't see where you get the idea that this discussion has anything to do with "custom things to SAGE_LOCAL". If anything it's the opposite. I am trying to treat SAGE_LOCAL as less special.

comment:12 Changed 3 years ago by Matthias Köppe

I would suggest to close the discussion about the python3-spkg-configure-venv proposals on this ticket. The discussion should take place on the technical tickets regarding that (#29013, #27824, #29023, #29032).

#29038 has nothing to with it.

comment:13 Changed 3 years ago by Matthias Köppe

Please review #29120 - One-line fix for "./configure is too sensitive to stray files/subdirectories".

comment:14 Changed 3 years ago by Matthias Köppe

Milestone: sage-9.1sage-duplicate/invalid/wontfix

comment:15 in reply to:  8 ; Changed 3 years ago by François Bissey

Replying to dimpase:

Replying to embray:

Replying to dimpase:

Replying to embray:

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

I don't think you are right here. Having sagelib working in venv of a system python is almost as good as direct pip-installing, as it allows to get away from endlessly rebuilding python system packages.

Installing sagelib directly into system python packages is useless from development point of view. It's good as a final goal, but we're far from it.

No we're not. Distro packagers already do it.

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

I and other distro maintainers have tools to build (and normally to run package testsuite in most case) without having to install the full package or needing root access. I even can do it with a given git branch. Of course we still need to install the dependencies but that's a given.

comment:16 in reply to:  15 ; Changed 3 years ago by Timo Kaufmann

Replying to fbissey:

Replying to dimpase:

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

I and other distro maintainers have tools to build (and normally to run package testsuite in most case) without having to install the full package or needing root access. I even can do it with a given git branch. Of course we still need to install the dependencies but that's a given.

+1

Additionally, nix doesn't even need root at all. It would be a perfectly viable replacement to sage's own build system for sage-development purposes. I already thought about suggesting that in the past, but I didn't since I don't have the time to work on it / convince people myself.

Edit: Now that I think about it I think I did suggest it once. One downside was that there would be no cygwin support, just WSL. Or someone would have to work on reviving nix on cygwin, not sure what it's current state is.

I agree with Erik and François that adding a package juts for configuration makes things a bit more difficult, at least for packagers. Instead of patching a file, we now have to add another python package and essentially re-write it.

I haven't looked at the details of the implementation though, I'm mostly commenting on what I understand from the title and the description.

In any case, thank you for your work @mkoeppe. It definitely has the right intention.

Last edited 3 years ago by Timo Kaufmann (previous) (diff)

comment:17 in reply to:  15 Changed 3 years ago by Dima Pasechnik

Replying to fbissey:

Replying to dimpase:

Replying to embray:

Replying to dimpase:

Replying to embray:

runnable in venv of a system Python

The virtualenv stuff is also a red herring. It should be pip-installable directly into a system Python's site-packages too. If that's possible then installing into a virtualenv also works more-or-less trivially. If we have to do much anything special in the case of a virtualenv then we're probably doing someting wrong.

I don't think you are right here. Having sagelib working in venv of a system python is almost as good as direct pip-installing, as it allows to get away from endlessly rebuilding python system packages.

Installing sagelib directly into system python packages is useless from development point of view. It's good as a final goal, but we're far from it.

No we're not. Distro packagers already do it.

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

I and other distro maintainers have tools to build (and normally to run package testsuite in most case) without having to install the full package or needing root access. I even can do it with a given git branch. Of course we still need to install the dependencies but that's a given.

While I made that comment, I had in mind tasks of Sage development rather than Sage's packaging. Developers don't have or don't know how to use these distro-specific tools, e.g. I hate to think of the sort of hoops one has to jump through to uninstall a system-wide Python package on MacOS.

That's why venv comes quite handy.

comment:18 Changed 3 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

And venv and Python package (sage_conf, in this case) are standard Python tools, whereas Sage distro with its SAGE_LOCAL and spaghetti text files for configuration are not.

Let me take the liberty to positively review this as won't fix.

comment:19 Changed 3 years ago by Matthias Köppe

Priority: blockertrivial

comment:20 Changed 3 years ago by Erik Bray

Milestone: sage-duplicate/invalid/wontfixsage-9.1
Status: positive_reviewneeds_review

I disagree, since this would be a prerequisite of #29119 which makes this sage_conf script unnecessary.

Last edited 3 years ago by Erik Bray (previous) (diff)

comment:21 in reply to:  16 ; Changed 3 years ago by Erik Bray

Replying to gh-timokau:

Replying to fbissey:

Replying to dimpase:

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

I and other distro maintainers have tools to build (and normally to run package testsuite in most case) without having to install the full package or needing root access. I even can do it with a given git branch. Of course we still need to install the dependencies but that's a given.

+1

Additionally, nix doesn't even need root at all. It would be a perfectly viable replacement to sage's own build system for sage-development purposes. I already thought about suggesting that in the past, but I didn't since I don't have the time to work on it / convince people myself.

Yeah, I don't have any idea where this idea of having to use root came from. Installing into the site-packages of any Python in any prefix (virtualenv or no virtualenv) is exactly the same process. virtualenv doesn't even enter into it. I cannot stress this enough, but you (Dima and Matthias) seem to be very confused about what the purpose and functionality of virtualenv is. It's just a tool to create a new install prefix under user control that can also link to an existing system Python without having to completely reinstall Python. That's it. That's all it is.

comment:22 Changed 3 years ago by Dima Pasechnik

Whatever is needed on #29119 can be handled there.

comment:23 in reply to:  21 Changed 3 years ago by Dima Pasechnik

Replying to embray:

Replying to gh-timokau:

Replying to fbissey:

Replying to dimpase:

So? You get something that you can only change is you have root. Are you seriosly proposing that Sage developent should be done as root?

I and other distro maintainers have tools to build (and normally to run package testsuite in most case) without having to install the full package or needing root access. I even can do it with a given git branch. Of course we still need to install the dependencies but that's a given.

+1

Additionally, nix doesn't even need root at all. It would be a perfectly viable replacement to sage's own build system for sage-development purposes. I already thought about suggesting that in the past, but I didn't since I don't have the time to work on it / convince people myself.

Yeah, I don't have any idea where this idea of having to use root came from. Installing into the site-packages of any Python in any prefix (virtualenv or no virtualenv) is exactly the same process. virtualenv doesn't even enter into it. I cannot stress this enough, but you (Dima and Matthias) seem to be very confused about what the purpose and functionality of virtualenv is. It's just a tool to create a new install prefix under user control that can also link to an existing system Python without having to completely reinstall Python. That's it. That's all it is.

yes, venv is a tool. It's already there, it's standard. Use it, don't re-invent the wheel with textfiles you need to parse...

comment:24 Changed 3 years ago by Erik Bray

This would still be a prerequisite and there's already a branch so why not.

I had a thought that might elucidate the issue about virtualenvs / install prefixes, etc.

Python also supports something even more basic called USER_BASE--this predates virtualenv and venv by quite a long time. It's just another notion of an alternate install prefix--the difference is it doesn't even link to an existing python interpreter or change the interpreter's sys.prefix. It's just an alternate prefix (like /usr/local) which goes to the front of the path, but that can live in individual users' home directories; on *NIX systems it is ~/.local: https://docs.python.org/3/library/site.html#site.USER_BASE

By default (unless it's disabled, which you can control with some environment variables, a usercustomize.py, etc.) it is enabled, and if ~/.local/lib/pythonX.Y/site-packages exists it supersedes the system site-packages on sys.path. Of course, since this is a full root prefix, you can also have a ~/.local/bin and such. So when you run pip install --user on a package, for example, any files it installs go into the ~/.local/ prefix, including scripts, share data, etc. Of course, it's the user's responsibly to add ~/.local/bin to their PATH if they want to use it.

This scheme is used by other tools as well I believe; for example I always install ruby gems into my ~/.local.

This is again to emphasize that virtualenv is not special and not the only context in which you'd want to install a Python package into a prefix that has nothing to with virtualenv, /usr, $SAGE_LOCAL, or any of that ($SAGE_LOCAL is sort of like a special case of ~/.local but specific to sage-the-distribution. When you run Sage's ./configure you can even do something like ./configure --prefix=$HOME/.local, and $SAGE_LOCAL is just a fancy synonym for $prefix. This is just discouraged for development in sage-the-distribution in order to keep it isolated from other install prefixes so that problems are easier to track down.

comment:25 Changed 3 years ago by Erik Bray

yes, venv is a tool. It's already there, it's standard. Use it, don't re-invent the wheel with textfiles you need to parse...

I have literally no idea what you're talking about at this point.

comment:26 Changed 3 years ago by Dima Pasechnik

Milestone: sage-9.1sage-duplicate/invalid/wontfix
Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

We may revert this when the replacement, #29119, is ready, and the revert is carefully tested. But now it should stay.

comment:27 Changed 3 years ago by Erik Bray

Milestone: sage-duplicate/invalid/wontfixsage-pending
Status: positive_reviewneeds_review

How about this? Leave it open but I'll remove the milestone. There's no reason to close this yet.

comment:28 Changed 3 years ago by Erik Bray

Description: modified (diff)

comment:29 Changed 3 years ago by Erik Bray

Maybe this line from Sage's own configure.ac can help elucidate the fact that FUD about "SAGE_LOCAL" is a red herring.

It's still just an install prefix. All the scripts related to SAGE_LOCAL are just about setting the prefix. When building SPKGs this also means setting things like header and library paths, and setting the rpath on binaries; tasks which need to be done for installing software in any non-default path. It's not all that complicated.

comment:30 Changed 2 years ago by Matthias Köppe

Authors: Erik Bray
Milestone: sage-pendingsage-duplicate/invalid/wontfix

Time to close it.

comment:31 Changed 2 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

comment:32 Changed 17 months ago by Matthias Köppe

Resolution: invalid
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.