sagemath/sage

sage_setup: Use paths within SAGE_LOCAL when provided via sage_conf

Closed this issue · 30 comments

We add a simple mechanism to sagelib's setup.py (via a new module in sage_setup) to prepend SAGE_LOCAL values to key environment variables needed for the build: PATH, LIBRARY_PATH, CPATH, LDFLAGS. This allows use to build sagelib outside of an environment set by sage-env (which would set these variables among many more) if only sage_conf is installed in the build environment.

To test:

./configure --enable-editable
make sagelib-build-deps      # installs sage_conf and other things
(cd src && ../local/bin/python3 setup.py develop)

Note - the last line is not within a sage-env!

Follow-up, if necessary: Because for misconfigured Pythons, -I options may leak in as described in #31335 and take precedence over the CPATH that we set, we may want to essentially revert #29697, adding some refinements:

  • SAGE_LOCAL vs SAGE_VENV
  • make sure that if $SAGE_LOCAL is unset, nothing is added.

Relevant tickets regarding include/library search paths: #13348, #14709, #29562 (+), #29607 (+), #29697 (?), #31041 (), #30818 (), #30013 (~)

Depends on #31593

CC: @jhpalmieri @kiwifb @orlitzky @antonio-rojas @dimpase

Component: build

Author: Matthias Koeppe

Branch: 31eaf59

Reviewer: Dima Pasechnik

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

Description changed:

--- 
+++ 
@@ -4,5 +4,8 @@
 - `SAGE_LOCAL` vs `SAGE_VENV`
 - make sure that if `$SAGE_LOCAL` is unset, nothing is added.
 
+Also, we want to enable building sagelib outside of an environment set by `sage-env`.
+
+
 Relevant tickets regarding include/library search paths: #13348, #14709, #29562 (+), #29607 (+), #29697 (?), #31041 (~), #30818 (~), #30013 (~)
 

Author: Matthias Koeppe

New commits:

5161d65build/pkgs/sage_conf/src/sage_conf.py.in: Move SAGE_ROOT, SAGE_LOCAL to beginning of file; only use substitution @prefix@ once
986ca18build/pkgs/sage_conf/src/sage_conf.py.in: replace subst by replace
6f05cc2sage_setup.setenv: New, use it in setup.py so that build works outside of sage-env

Description changed:

--- 
+++ 
@@ -1,11 +1,17 @@
-(from #31335)
+We add a simple mechanism to sagelib's `setup.py` (via a new module in `sage_setup`) to prepend `SAGE_LOCAL` values to key environment variables needed for the build: `PATH`, `LIBRARY_PATH`, `CPATH`, `LDFLAGS`. This allows use to build sagelib outside of an environment set by `sage-env` (which would set these variables among many more) if only `sage_conf` is installed in the build environment.
 
-To make sure that libraries installed in $SAGE_LOCAL (if set) take precedence over leaks described in #31335, we may want to essentially revert #29697, adding some refinements:
+To test:
+
+```
+./configure --enable-editable
+make sagelib-build-deps      # installs sage_conf and other things
+(cd src && ../local/bin/python3 setup.py develop)
+```
+Note - the last line is not within a sage-env!
+
+Follow-up, if necessary: Because for misconfigured Pythons, `-I` options may leak in as described in #31335 and take precedence over the CPATH that we set, we may want to essentially revert #29697, adding some refinements:
 - `SAGE_LOCAL` vs `SAGE_VENV`
 - make sure that if `$SAGE_LOCAL` is unset, nothing is added.
 
-Also, we want to enable building sagelib outside of an environment set by `sage-env`.
-
-
 Relevant tickets regarding include/library search paths: #13348, #14709, #29562 (+), #29607 (+), #29697 (?), #31041 (~), #30818 (~), #30013 (~)
 

Dependencies: #31593

Commit: 6f05cc2

Changed commit from 6f05cc2 to 6ea676d

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

6ea676dMerge tag '9.3' into t/31338/sage_setup__use_paths_within_sage_local_when_provided_via_sage_conf

Reviewer: Dima Pasechnik

comment:5

lgtm

comment:6

Thank you!

Changed commit from 6ea676d to 31eaf59

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

2078fa6Merge tag '9.4.beta0' into t/31593/configure__paths_within__sage_local___prefix__for_sage_conf
31eaf59Merge #31593
comment:8

ran into missing toml dependency in build/pkgs/pytoml/dependencies while testing (building backall errored out with toml module not found - pytoml was built, but toml was not).

comment:9

I think this is actually coming from #31280

comment:10

OK, fine, this works otherwise, will look at #31280 again.

comment:11

Thanks!

Changed commit from 31eaf59 to none

comment:13

A bit late to that party. sage_setup/setenv.py is highly "polluting" - i.e. it all works but fill my build logs with useless junk

/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libm.so when searching for -lm
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libm.a when searching for -lm
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libc.so when searching for -lc
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: skipping incompatible /usr/lib/libc.a when searching for -lc

I am trying to follow the logic of the ticket but I fail to see when SAGE_LOCAL is not defined, which is the only way to avoid these. In env.py we have

SAGE_LOCAL = var("SAGE_LOCAL", SAGE_VENV)

So either something from the environment or SAGE_VENV and it is defined by

SAGE_VENV = var("SAGE_VENV", os.path.abspath(sys.prefix))

So if neither SAGE_LOCAL or SAGE_VENV is in the environment SAGE_LOCAL becomes os.path.abspath(sys.prefix).

I am not sure why the if SAGE_LOCAL: conditional is necessary since it is always defined. What I don't see is a way to avoid the content of that block becoming added to the environment.

Is there another ticket that will make SAGE_LOCAL default to None in some circumstances? Or a process that I overlooked?

comment:14

The idea for distribution packaging would be that SAGE_LOCAL will not be set at all, which is why we have the if SAGE_LOCAL there.

That it is always set, by the traditional catch-all defaults in sage.env, is a nice catch; let's fix this in #32036.

comment:15

I am OK with eliminating SAGE_LOCAL the way SAGE_ROOT is virtually extinct but we are not there yet. It still need to be set at runtime for a few things. Some would gain to be replaced by more specific variables. But let's move things to #32036

comment:16

Let's see if we can solve this problem in a different way. Where is this incompatible /usr/lib/libm.so business exactly coming from?

comment:17

Replying to @mkoeppe:

Let's see if we can solve this problem in a different way. Where is this incompatible /usr/lib/libm.so business exactly coming from?

Because you are add -L/usr/lib (and some rpath to boot) at linking time, since SAGE_LOCAL is /usr. But of of course on my system the correct location is /usr/lib64, /usr/lib is for 32bit libraries. So, when you add -L/usr/lib it looks for the 32bit version of the libraries first before looking for the correct ones in the default location.

It is not strictly harmful unless you have done something dodgy to your system. I described it as pollution, because my logs gets full of it. Which makes reading them difficult when you are looking for real problems.

comment:18

OK, this is essentially the same issue that we were hitting in #31578. Should we be looking for a general way to find the multilib path for situations like this? Perhaps something in python3 -m sysconfig gives it?

comment:19

It is similar indeed and yes it appears that python -m sysconfig has the correct value in LIBDIR but we should check on a few setup to make sure.

comment:20

Hum, on ubuntu I get /usr/lib when I am fairly sure it should be /usr/lib/x86_64-linux-gnu.

comment:21

This is really annoying, ubuntu systems have a MULTIARCH defined where my gentoo system doesn't. But conda on ubuntu defines MULTIARCH even so it doesn't use it. There must be a better way to get the right info.

comment:22

Another possible solution: #32057