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

Add a plain-text config file that feeds into sage.env #22652

Open
embray opened this issue Mar 20, 2017 · 19 comments
Open

Add a plain-text config file that feeds into sage.env #22652

embray opened this issue Mar 20, 2017 · 19 comments

Comments

@embray
Copy link
Contributor

embray commented Mar 20, 2017

Issues like #22628 remind me of an idea I've had before for improving the situation with the otherwise difficult to maintain and unwieldy sage.env module.

ISTM that almsot all the data in that file could be read from a plain text file format (either an ini-like file or JSON, for example). Furthermore, this file could be generated, or updated, at install time for spkgs (including Sage itself). This would help make the growing number of (sometimes contentious) spkg-specific variables (CONWAY_POLYNOMIALS_DATA_DIR, GRAPHS_DATA_DIR, GAP_ROOT_DIR, SINGULAR_SO, etc.) more manageable.

If each spkg is allowed its own "section" in this config file, the installers for those spkgs could be allowed to manage the data in that section as well, which is sometimes platform-specific. It is also data that is usually more accessible at install-time of that package than it is to Sage at runtime. For example, #22628 raises a tricky issue about how to set SINGULAR_SO. If the correct path to SINGULAR_SO were written to a config file at install time, that would push all platform-specific logic about how to set that path to the spkg-install script, and out of sage.env. For system packagers, such as for Gentoo or Debian, they would be free to write whatever they want in this config file, and it would free them having to do as much patching to sage.env and possibly other files.

I have some idea how this might work, so I'm willing to cook up a prototype if it doesn't sound like a terrible idea.

Depends on #27040

CC: @kiwifb @jdemeyer @mkoeppe @timokau

Component: porting

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

@embray embray added this to the sage-8.0 milestone Mar 20, 2017
@jdemeyer
Copy link

comment:1

Instead of one file, having one directory where each package can write a file seems like a much better idea. Think of pkgconfig: we don't have one pkgconfig file, we have a directory local/lib/pkgconfig with one file per package.

It's not clear to me if you plan to change only the Python module sage.env or also the shell script src/bin/sage-env. Ideally, your idea should also reduce duplication/complexity in sage-env.

I'm adding #21479 as dependency because of potential conflicts.

@jdemeyer
Copy link

Dependencies: #21479

@embray
Copy link
Contributor Author

embray commented Mar 20, 2017

comment:2

I agree--one file per package sounds better, and will also have fewer problems in parallel builds.

I need to take a closer look but my hope is that such a config file could reduce complexity in both sage-env and sage.env.

The only thing with sage-env is that it should be easy to read/parse the file without, say, invoking a Python interpreter. That's another good argument for one per-package config files, as that would reduce the complexity of the format. Probably a <key>=<value> format would be good enough.

@kiwifb
Copy link
Member

kiwifb commented Mar 20, 2017

comment:3

I would concur with Jeroen for a design with a directory but I am thinking of something like /etc/{env.d,profile.d,sudoers.d} and the list goes on.

Bonus point if both sage-env and env.py can feed from it avoiding duplication.

@embray
Copy link
Contributor Author

embray commented Mar 20, 2017

comment:4

To be clear, I would still allow all these options to be read from environment variables too, and environment variables would override config files. But this might also reduce the number of environment variables that need to be set.

Another possible advantage is the ability for per-user config files that override the globals, but I would leave that as a separate task.

@mkoeppe
Copy link
Member

mkoeppe commented Mar 20, 2017

comment:6

See also #15087.

@embray
Copy link
Contributor Author

embray commented Mar 21, 2017

comment:7

#15087 appears to be much more limited in scope and is slightly different, though not incompatible with this idea. #15087 is specifically for modifying the environment, but I'm hoping with this to actually modify the environment less, since options can instead be read from a config file. Good to know it exists though.

@embray
Copy link
Contributor Author

embray commented Mar 21, 2017

comment:8

One thing I'm struggling with a bit on this is, if this is implemented, what the upgrade path should look like.

The idea is to generate these config files at build time. So how will they be generated for packages that are already built? Maybe I should just increase the patch levels on those packages? I think the only packages that might get their own config files are currently:

  • conway_polynomials
  • graphs
  • elliptic_curves
  • polytopes_db
  • gap
  • thebe
  • singular

plus sagelib itself. Any others? I determined this (rough) list based on those packages that require any settings in sage.env.

There are also packages that require special environment variables to be set in sage-env. Those include:

  • jupyter
  • matplotlib
  • r
  • maxima
  • ecl
  • ncurses

This might be a job for #15087 like functionality.

@jdemeyer
Copy link

comment:9

Replying to @embray:

Maybe I should just increase the patch levels on those packages?

Yes.

@jdemeyer
Copy link

comment:10

Replying to @embray:

#15087 appears to be much more limited in scope and is slightly different, though not incompatible with this idea. #15087 is specifically for modifying the environment, but I'm hoping with this to actually modify the environment less, since options can instead be read from a config file. Good to know it exists though.

I don't see any difference between #15087 and this ticket, except that #15087 might be slightly more focused on the shell, while ticket is slightly more focused on the Sage library.

@embray
Copy link
Contributor Author

embray commented Mar 21, 2017

comment:11

I agree, that's the only difference. But it is a substantive one. We have two cases here: settings that must be reflected in the environment, and settings that would only be read by the Sage library, and not set as environment variables. Some packages could have settings that fall into both categories too (though I don't think there are any such examples currently).

It might be possible to kill two birds with one stone here, but there needs to be a way to make this distinction.

@embray
Copy link
Contributor Author

embray commented Aug 25, 2017

comment:12

Maybe a simple way to kill two birds: Just a simple script that sets variables and possibly exports them. If the line starts with export then naturally it will set an environment variable when sourced. Otherwise it will just set a shell variable but not export it to the environment. When sage.env reads these files it can parse it the same way.

@mkoeppe
Copy link
Member

mkoeppe commented Aug 29, 2017

comment:13

Replying to @embray:

Maybe a simple way to kill two birds: Just a simple script that sets variables and possibly exports them. If the line starts with export then naturally it will set an environment variable when sourced. Otherwise it will just set a shell variable but not export it to the environment. When sage.env reads these files it can parse it the same way.

This sounds like a good solution. Note src/bin/sage-env-config is already in a simple format, and we should keep it in this form (see discussion on #22646).

@embray
Copy link
Contributor Author

embray commented Aug 30, 2017

comment:14

I nearly forgot, I was working on implementing this last week. I think it was working, but I wasn't sure because there were some test failures (but it seems these failures are unrelated).

@embray
Copy link
Contributor Author

embray commented Aug 30, 2017

comment:15

I'm reconsidering this now that I've gotten a little deeper into implementation. There are some cases that are somewhat non-trivial, such as the environment settings for R (which, for consistency's sake, would be nice to move entirely to its own file).

A related, but alternative approach I'm considering is that rather than sourcing these files directly, they should be executable scripts that do nothing but print out the environment variables that they should set. So, for example, rather than source $SAGE_LOCAL/etc/sage/env.d/pkgname.sh, one would eval `$SAGE_LOAL/etc/sage/env.d/pkgname.sh`. Or something like that.

@embray embray removed this from the sage-8.0 milestone Nov 6, 2018
@timokau
Copy link
Contributor

timokau commented Jan 18, 2019

comment:18

[ Deleted this comment which was meant for #26968. ]

@jdemeyer
Copy link

Changed dependencies from #21479 to #27040

@timokau
Copy link
Contributor

timokau commented Jan 18, 2019

comment:20

My last comment was intended for #26968. Sorry for the mixup.

@mkoeppe
Copy link
Member

mkoeppe commented Jan 15, 2020

comment:21

Instead of a plain-text config file, in #29022 I generate a Python module.

The issue with the library dir configuration (SINGULAR_SO) mentioned in the ticket description has again come up in #29013 (venv).
It should indeed be fixed by just writing out the configured library path to the new configuration file.

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

5 participants