Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Jan 18, 2020 · 86 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 18, 2020

This ticket introduces a new Python package sage_conf.

  • sage-the-distribution will generate this Python package at ./configure time and install it at build time before starting to build/install sagelib using src/setup.py.
  • Distributions will generate and install their own sage_conf by a method of their choice before starting to build/install sagelib.

A console_script allows to query individual variable values from the shell, or output all variables in .env format:

$ local/bin/sage-config MAXIMA
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/maxima
$ local/bin/sage-config
VERSION=9.1.beta0
MAXIMA=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/maxima
SAGE_LOCAL=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local
SAGE_ROOT=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring

The module and the script are used as follows:

  1. It provides configuration information to sagelib at installation time (src/setup.py).

    Ultimately we want to be able to install sagelib with pip. To support installation by pip from PyPI, from an URL, etc., we cannot expect to configure the build like we do now, by writing the configuration to the file src/sage-env-config. Moreover, if pip is running in --isolated mode, also no environment variables are passed. Then the only information flow is through arguments to pip build and through the installed Python packages. By installing sage_conf first, we make the configuration available to the pip install of sagelib.

  2. It provides configuration information to the docbuild, in particular about the install locations of documentation of external packages.

  3. It provides configuration information to the runtime of sagelib, making sagelib more independent from the environment variables set by src/bin/sage-env (local/bin/sage-env).

a. By providing SAGE_LOCAL as one the configuration variables, it removes assumptions regarding install locations of sagelib relative to $SAGE_LOCAL. This enables the following:

CC: @kiwifb @antonio-rojas @isuruf @embray @infinity0 @timokau @jdemeyer @dimpase @jhpalmieri @vbraun

Component: build

Author: Matthias Koeppe

Branch: af18a01

Reviewer: Dima Pasechnik, Erik Bray

Issue created by migration from https://trac.sagemath.org/ticket/29038

@mkoeppe mkoeppe added this to the sage-9.1 milestone Jan 18, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 18, 2020

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 18, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 18, 2020

New commits:

fe5c89029038: Python package sage_conf: Provides configuration information for sagelib

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 18, 2020

Commit: fe5c890

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 18, 2020

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

aefd827build/pkgs/sage_conf/spkg-install: Fix up path

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 18, 2020

Changed commit from fe5c890 to aefd827

@dimpase
Copy link
Member

dimpase commented Jan 18, 2020

comment:5

why are you checking in files which apparently are meant to be generated from .in templates?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 18, 2020

comment:6

A mistake, which I will fix in a moment. Thanks!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 18, 2020

Changed commit from aefd827 to 88fd03c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 18, 2020

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

981e864Move .gitignore to the right place, remove generated files
88fd03cAdd console_scripts entry_point sage-config

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jan 19, 2020

comment:12

the example with sage --python -m venv - is it going to work if Sage is using venv on its own?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 19, 2020

comment:13

Replying to @dimpase:

the example with sage --python -m venv - is it going to work if Sage is using venv on its own?

Not without installing sage packages into the new venv. As far as I can see, one can only control whether system-packages are made available but not whether the originating venv's packages are made available. But I'd expect that this option will be invented at some point in Python.

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jan 19, 2020

comment:15

is sage_conf also meant to be used while building Sage's dependencies?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 19, 2020

comment:16

Replying to @dimpase:

is sage_conf also meant to be used while building Sage's dependencies?

No.

@dimpase
Copy link
Member

dimpase commented Jan 19, 2020

comment:17

author ought to be either "The Sage Developers" (cf https://wiki.sagemath.org/Publications_using_SageMath?action=show&redirect=Publications_using_SAGE)
or yourself...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 19, 2020

comment:18

I copied the metadata from src/setup.py

@dimpase
Copy link
Member

dimpase commented Jan 19, 2020

comment:19

Replying to @mkoeppe:

I copied the metadata from src/setup.py

that line is 5.5 years old. With all respect, William has not contributed a line of code to sagemath since 2017.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2020

Changed commit from 88fd03c to f286376

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2020

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

f286376build/pkgs/sage_conf/src/setup.cfg.in: Change author to 'The Sage Developers'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

comment:55

Replying to @embray:

The idea of making this type=script make sense to me, but that isn't used very often so it hasn't been well tested. If you look at what happens with a type=script package, [...] it already sources sage-env, but not sage-dist-helpers.

Thanks! I've simplified spkg-install accordingly.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

comment:56

Replying to @embray:

In some ways this makes things even worse for packagers, because now to package sage_config they have to hand-patch this Python module.

No, to the contrary. This ticket defines a functional interface to configuration: to the shell, by sage-config, to Python, by importing the module sage_conf and reading its variables.

The version of sage_conf provided in build/pkgs is just a reference implementation of the functional interface that sage-the-distribution uses.

Packagers will just be able to generate a sage_conf module in any way they like.

I have chosen this design (external to the src directory, a Python module rather than a configuration file) for the following reasons:

  • committing to a functional interface is more flexible than (1) committing to configuration by a static file written at installation time; (2) (less importantly) to a particular file format for that.
  • for example, a binary distribution may decide to ship a version of sage_conf that is more dynamic: It could discover some settings at its runtime.
  • it is not part of the src directory; in this way it is working towards the goals of (a) distributors being able to use the whole unmodified src tree; (b) pip installability of the unmodified src tree.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

Changed commit from 8f8d8e3 to 7e34340

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

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

7e34340build/pkgs/sage_conf/src/sage_conf.py.in: Reimplement _main using argparse

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

comment:58

Replying to @embray:

For the argument parsing please just use argparse.

Thanks for the suggestion. Done.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

Changed reviewer from Dima Pasechnik to Dima Pasechnik, Erik Bray

@dimpase
Copy link
Member

dimpase commented Jan 22, 2020

comment:61
from __future__ import print_function
+del print_function

by the way, can you explain these 2 lines

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

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

af18a01Use sys.stdout.write instead of print

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2020

Changed commit from 7e34340 to af18a01

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2020

comment:63

Replying to @dimpase:

from __future__ import print_function
+del print_function

by the way, can you explain these 2 lines

Without the del print_function, sage-config would print a line print_function=_Feature((2, 6, 0, 'alpha', 2), (3, 0, 0, 'alpha', 0), 65536).

But I have simplified the module by avoiding print altogether.

@dimpase
Copy link
Member

dimpase commented Jan 22, 2020

comment:64

OK for me.

@vbraun
Copy link
Member

vbraun commented Jan 25, 2020

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:66

Replying to @mkoeppe:

Replying to @embray:

In some ways this makes things even worse for packagers, because now to package sage_config they have to hand-patch this Python module.

No, to the contrary. This ticket defines a functional interface to configuration: to the shell, by sage-config, to Python, by importing the module sage_conf and reading its variables.

The version of sage_conf provided in build/pkgs is just a reference implementation of the functional interface that sage-the-distribution uses.

Packagers will just be able to generate a sage_conf module in any way they like.

I have chosen this design (external to the src directory, a Python module rather than a configuration file) for the following reasons:

  • committing to a functional interface is more flexible than (1) committing to configuration by a static file written at installation time; (2) (less importantly) to a particular file format for that.
  • for example, a binary distribution may decide to ship a version of sage_conf that is more dynamic: It could discover some settings at its runtime.
  • it is not part of the src directory; in this way it is working towards the goals of (a) distributors being able to use the whole unmodified src tree; (b) pip installability of the unmodified src tree.

But it's not even documented what this is supposed to provide or how it's supposed to work. This is all very confusing.

I don't really know what you mean by "functional" here. Do you mean executable? Why? Is there a specific reason for that? In any case I know of all that's needed here is at most a list of environment variables. In many cases this will just be static. If a packager has a need to generate some values for those variables programmatically they can do so however they like, but now they have to write a whole Python script that implements some undocumented interface.

@embray
Copy link
Contributor

embray commented Jan 31, 2020

Changed commit from af18a01 to none

@dimpase
Copy link
Member

dimpase commented Jan 31, 2020

comment:67

Once again, it's more Pythonic to wrap Python-needed things into Python modules, as sysconfig does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:68

Replying to @dimpase:

Once again, it's more Pythonic to wrap Python-needed things into Python modules

![citation needed]

as sysconfig does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".

Perhaps you missed it, but as I already explained above this is exactly what sysconfig does. You are saying that sysconfig is "unpythonic".

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 31, 2020

comment:69

The larger plan is laid out in #21707 - Split sage-env into 5. It explains how pieces fit together. A recommended read.

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:70

For reference, again sysconfig, which is a hard-coded Python module which reads from a plain text-file.

Also recommended, the venv module which is actually a lot less mysterious than you guys seem to be making it out to be. The only magic involved is some support bits coded directly into the Python interpreter used for setting sys.prefix correctly (this is one of the few ways venv differs from the original virtualenv which needed some hacks to achieve this).

@embray
Copy link
Contributor

embray commented Jan 31, 2020

comment:71

Replying to @mkoeppe:

The larger plan is laid out in #21707 - Split sage-env into 5. It explains how pieces fit together. A recommended read.

Thanks, that is useful. Obviously I've participated in that ticket before, but I wasn't sure if it was the ticket tying the rest of this together. I still believe that steps 3/4/5 can be achieved by different means which merit more discussion. But I'm on board with the overall plan :)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 31, 2020

comment:72

No problem. I know that it is not always possible to keep up with all developments when there are more important things to do.

As you see in that plan in #21707, the present ticket on sage_conf was a step to cleaning up the sage-env mess.

It has little to do with my venv-python3 tickets (which I haven't had time to work on). (In fact, I now think that your version at #29032 may, after all, be the simpler way to go at it -- but I have to get back to the python3-spkg-configure business later.)

So there's no need to mix any "venv" discussion into the "sagelib configuration" discussion.

The "regression" caused by this ticket is one that is affecting developers who switch between new and old branches. It is fixed by just merging current develop into the old branch.

@dimpase
Copy link
Member

dimpase commented Jan 31, 2020

comment:73

Replying to @embray:

Replying to @dimpase:

Once again, it's more Pythonic to wrap Python-needed things into Python modules

![citation needed]

cf. [sysconfig].

as sysconfig does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".

Perhaps you missed it, but as I already explained above this is exactly what sysconfig does. You are saying that sysconfig is "unpythonic".

No, obviously Python needs to do some non-Pythonic things under the hood, but then the interface is Pythonic, and sysconfig is an example of such an approach.
You are somehow against this approach, you say that such wrapping is too complicated, so it should not be done - if you were Python BDFL, you'd not have allowed sysconfig in.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 31, 2020

comment:74

Replying to @mkoeppe:

The "regression" caused by this ticket is one that is affecting developers who switch between new and old branches. It is fixed by just merging current develop into the old branch.

For tickets where merging the current develop is not easy, I have prepared #29120 - One-line fix for "./configure is too sensitive to stray files/subdirectories".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants