sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Clean up src/bin/sage-env-config.in: Move logic to src/bin/sage-env, move non-environment configuration variables to sage_conf.py #29384

Closed mkoeppe closed 4 years ago

mkoeppe commented 4 years ago

Follow-up from #29038.

We clean up sage-env-config, intended for setting direct values only, by moving conditional settings of other environment variables to sage-env instead.

Moreover, we move configuration variables that are only needed by the Sage python process but not as environment variables by its child processes, to sage_conf.py. In this way they become available in the sage.all module even when invoked outside of a sage-env.

(part of #21707.)

CC: @orlitzky @dimpase @jhpalmieri

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: 7364e2a

Reviewer: Michael Orlitzky

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

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,8 @@
 Follow-up from #29038.

-By moving variables to sage_conf, they become available in the `sage.all` module even when invoked outside of a sage-env.
+We clean up `sage-env-config`, intended for setting direct values only, by moving conditional settings of other environment variables to `sage-env` instead.

+Moreover, we move configuration variables that are only needed by the Sage python process but not as environment variables by its child processes, to `sage_conf.py`. In this way they become available in the `sage.all` module even when invoked outside of a `sage-env`.
+
+(part of #21707.)
+
mkoeppe commented 4 years ago

Branch: u/mkoeppe/move_some_configuration_variables_from_src_bin_sage_env_config_in_to_sage_conf

orlitzky commented 4 years ago
comment:4

Do we still need

var('SAGE_NAUTY_BINS_PREFIX',        '')

in src/sage/env.py after this?


New commits:

7364e2aClean up src/bin/sage-env-config.in: Move logic to src/bin/sage-env, move non-environment configuration variables to sage_conf.py
orlitzky commented 4 years ago

Commit: 7364e2a

mkoeppe commented 4 years ago
comment:5

Yes, this is what makes the variable available in the module sage.env.

orlitzky commented 4 years ago
comment:6

Replying to @mkoeppe:

Yes, this is what makes the variable available in the module sage.env.

Oh, I see. Updated question: is there any reason to be going through sage.env still, as opposed to importing the value directly from sage_conf? Something to do with distro packaging?

(I've probably asked this same question three times by now, sorry.)

mkoeppe commented 4 years ago
comment:7

Replying to @orlitzky:

Replying to @mkoeppe:

Yes, this is what makes the variable available in the module sage.env.

Oh, I see. Updated question: is there any reason to be going through sage.env still, as opposed to importing the value directly from sage_conf? Something to do with distro packaging?

Well, sage_conf is still "optional". It's yet to be seen what distribution packagers will be doing with it. For now it's best to keep the logic of handling the situation when sage_conf does not exist centrally, rather than in every place that needs a variable.

orlitzky commented 4 years ago
comment:8

Ok, my curiosity is satisfied. Needs review, or still working on it?

mkoeppe commented 4 years ago

Author: Matthias Koeppe

orlitzky commented 4 years ago

Reviewer: Michael Orlitzky

mkoeppe commented 4 years ago
comment:11

Thanks!

vbraun commented 4 years ago

Changed branch from u/mkoeppe/move_some_configuration_variables_from_src_bin_sage_env_config_in_to_sage_conf to 7364e2a