Fix installations with system Python on Ubuntu/Fedora by vsoch · Pull Request #31939 · spack/spack

vsoch · 2022-08-04T23:19:00Z

If docutils doesn't have a bin, then don't try to iterate over files in it.

adamjstewart · 2022-08-04T23:27:58Z

Why would it not have a bin directory? If it's version-specific, we can use @when().

vsoch · 2022-08-04T23:33:15Z

I don't ask the questions, I just am trying to install flux-core and this is the error I hit.

adamjstewart · 2022-08-05T00:04:52Z

Can you upload the build log here? The fact that part of the installation is just missing is alarming...

vsoch · 2022-08-05T00:15:29Z

okay so this is weird, the build log looks okay?

spack-build-out.txt

but in my terminal (and it exits on error):

==> Error: FileNotFoundError: [Errno 2] No such file or directory: '/opt/spack/opt/spack/linux-ubuntu22.04-x86_64/gcc-11.2.0/py-docutils-0.18.1-dcncvjgwszuwmanlasey434q3irm7q6d/bin'

/opt/spack/var/spack/repos/builtin/packages/py-docutils/package.py:42, in post_install:
         40    def post_install(self):
         41        bin_path = self.prefix.bin
  >>     42        for file in os.listdir(bin_path):
         43            if file.endswith(".py"):
         44                os.symlink(os.path.join(bin_path, file), os.path.join(bin_path, file[:-3]))

See build log for details:
  /tmp/root/spack-stage/spack-stage-py-docutils-0.18.1-dcncvjgwszuwmanlasey434q3irm7q6d/spack-build-out.txt

vsoch · 2022-08-12T00:11:42Z

ping we'd like to get this working because it's failing the flux container build

https://github.com/flux-framework/flux-core/runs/7797716827?check_suite_focus=true

ping @trws

trws · 2022-08-12T16:21:58Z

I'm trying to repro this now, because trying it outside the container makes it work correctly and actually produces a bin directory no issue.

trws · 2022-08-12T16:47:47Z

@adamjstewart, something really strange is going on here. For some reason in this specific container (ubuntu 22.04 based) the directory tree in the prefix is placed under <prefix>/local/{bin,lib} instead of having them at the top level. I have little idea why, the pip command is identical to one that works outside of the container:
'/usr/bin/python3.10' '-m' 'pip' '-vvv' '--no-input' '--no-cache-dir' '--disable-pip-version-check' 'install' '--no-deps' '--ignore-installed' '--no-build-isolation' '--no-warn-script-location' '--no-index' '--prefix=/opt/spack/opt/spack/linux-ubuntu22.04-x86_64/gcc-11.2.0/py-docutils-0.18.1-dcncvjgwszuwmanlasey434q3irm7q6d' '.'

Is this maybe another crazy debianism we need to work around?

trws · 2022-08-12T16:50:36Z

It seems that if I switch the flag from --prefix to --target a bin directory is produced correctly, but there is no longer a lib directory, we get bin/ and docutils/.

vsoch · 2022-08-12T17:08:59Z

And reading their docs it sounds like there is a third location, tools? https://docutils.sourceforge.io/docs/dev/distributing.html

adamjstewart · 2022-08-12T17:14:31Z

That is quite odd, I wouldn't expect pip to install things to <prefix>/local. It's just this package, no other package, right? I don't see anything suspicious in their setup.py. Does python -s or python -S make a difference here? @pradyunsg any ideas?

trws · 2022-08-12T17:17:07Z

Oh that's a good question. Yes it's just that one package, it's really, really odd. I didn't know that --target and --prefix would act that differently either.

trws · 2022-08-12T17:22:02Z

-s has no effect, -S makes it so the python can't find pip.

vsoch · 2022-08-12T17:27:46Z

Why does flux need docutils? Could we make a variant that doesn't require it?

trws · 2022-08-12T17:30:04Z

Oh crap, I think I figured out why it's adding the local, but I'm arguing with myself about how to fix it... So, this python is set with its default --root to be /, and the offset from root for prefix to be usr/local, if I specify --root it puts everything at <prefix>/usr/local/.... If I specify --target, the package gets installed into the prefix and forms a bin directory, but it doesn't set the prefix for the top-level bin and lib directories (attempting to set both to the same value causes an error). Setting only the --prefix seems to be equivalent to only overriding the usr portion for some reason, and not the local/ part. I'm betting this is some crazy property of a debian-based distro's system python, but how do we override it and get something that works?

trws · 2022-08-12T17:30:52Z

Why does flux need docutils? Could we make a variant that doesn't require it?

Documentation, yes we could have a +docs variant we could turn off, that would drop the deps on docutils and asciidoc.

vsoch · 2022-08-14T17:30:45Z

To follow up here - we tweaked the variant so it will build without error #32099, but if someone wants the docs they are still going to hit this issue.

adamjstewart · 2022-08-14T17:42:04Z

I'm hoping @pradyunsg will have an idea of why only this package is installing to the wrong location and what the best flags to use are.

In the meantime, I marked this PR as a draft since the current PR is more of a workaround than a solution.

pradyunsg · 2022-08-15T18:01:46Z

Could someone share a clearer explaination of the behaviour you're seeing here, vs what you expected? 🙈

Something that shows the output of pip, as well the final filesystem structure you're seeing would help orient me as well.

$ pip install ...
$ tree ...

Right now, I've just read through the discussion and am not fully sure what the question is and what location is unexpected.

vsoch · 2022-08-15T18:04:52Z

The package is hard coded to expect docutils to have a bin. It does not, it has a local directory instead.

adamjstewart · 2022-08-15T18:08:38Z

@pradyunsg we are seeing the same issue as reported in pypa/pip#10978

Expected behavior

We would expect pip install . --prefix=<prefix> to install things to:

<prefix>/bin
<prefix>/lib/pythonX.Y/dist-packages

Actual behavior

Things are being installed to:

<prefix>/local/bin
<prefix>/local/lib/pythonX.Y/dist-packages

This causes all of our assumptions about the install layout to break, including patching of scripts in bin.

pradyunsg · 2022-08-15T18:28:46Z

Are you using Fedora/RHEL-provided Python, outside of a virtual environment? If so, I'm pretty sure I've seen this before, and it's because of a patch that they've made to Python's site or sysconfig module that changes the installation scheme to be under a local directory when not inside a virtual environment.

https://github.com/fedora-python/cpython/blame/fedora-3.7/Lib/sysconfig.py#L98

I've told at least two folks to report this to Fedora, but I don't know if anyone has reported this to them yet. :)

vsoch · 2022-08-15T18:30:07Z

Nope it was an ubuntu container.

trws · 2022-08-15T18:30:14Z

It's a Debian or Ubuntu one, they use a similar patch sadly, and yes we've complained about their munging of python a number of times, but this is the first time it's broken installation into an explicit prefix.

pradyunsg · 2022-08-15T18:33:26Z

Here's the patch, on 3.10:

https://salsa.debian.org/cpython-team/python3/-/blob/python3.10/debian/patches/sysconfig-debian-schemes.diff

They likely have a similar one on older Python versions.

pradyunsg · 2022-08-15T18:36:49Z

To confirm whether we're right about this hypothesis, check if posix_local is the name of sysconfig.get_preferred_scheme()?

python -c "import sysconfig; print(sysconfig.get_preferred_scheme())"

pradyunsg · 2022-08-15T18:57:39Z

I've opened https://discuss.python.org/t/linux-distro-patches-to-sysconfig-are-changing-pip-install-prefix-outside-virtual-environments/18240 because this is broader than just Fedora and... non-trivial.

Assuming this is the source of your issues, my pragmatic suggestion would be to check if there's a single local directory and getting rid of that layer of indirection -- because that shouldn't really be there (and is a bug coming out of the Linux distro patches to Python). That way, when/if this gets fixed in the Linux distros or if Spack is used with a not-distro-patched Python, the logic would gracefully do the right thing with slightly fewer filesystem operations.

adamjstewart · 2022-08-15T19:12:55Z

I think @trws had the same idea, we can symlink everything in <prefix>/local to <prefix> until this is fixed in each distro. Now if only someone could convince Apple to fix their sysconfig patching...

If docutils doesn't have a bin, then don't try to iterate over files in it.

Work around the debian and Fedora bug by symlinking the contents of the local directory if it appears.

trws · 2022-08-15T23:04:57Z

Basic patch implementing that added here @adamjstewart.

lib/spack/spack/build_systems/python.py

adamjstewart · 2022-08-15T23:11:12Z

var/spack/repos/builtin/packages/py-pip/package.py

+        local_path = os.path.join(prefix, "local")
+        if os.path.isdir(local_path) and not os.path.isdir(os.path.join(prefix, "lib")):
+            for p in os.listdir(local_path):
+                os.symlink(os.path.join(local_path, p), os.path.join(prefix, p))


If you're going to be thorough, setuptools, wheel, and dozens of other packages that don't use PythonPackage need this as well:

$ grep std_pip_args */package.py cmor/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] faiss/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] fenics/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."] flatbuffers/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."] jsonnet/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."] mxnet/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-dgl/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-jaxlib/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."] py-keras/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-or-tools/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-pip/package.py: args = [os.path.join(whl, "pip")] + std_pip_args + ["--prefix=" + prefix, whl] py-setuptools/package.py: args = ["-m", "pip"] + std_pip_args + ["--prefix=" + prefix, whl] py-tensorboard-data-server/package.py: args = std_pip_args + ["--prefix=" + prefix, wheel] py-tensorflow-estimator/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-tensorflow-hub/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-tensorflow-probability/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-tensorflow/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-tfdlpack/package.py: args = std_pip_args + ["--prefix=" + prefix, "."] py-wheel/package.py: args = std_pip_args + ["--prefix=" + prefix, self.stage.archive_file] sgpp/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."] treelite/package.py: args = std_pip_args + ["--prefix=" + self.prefix, "."]

If these all have the same structure, should we make PythonManual (could use a better name) a new build system?

Oof... some of these are much worse, Cmor is an autotools package that uses pip to install a module into its prefix, neither solution actually works for that without doing a view-like traversal.

Curious what @alalazo thinks. We could extend both the current base class and PythonPackage and have this be a separate function that can be called in each package.

Yeah, could do that. Could make it part of an Executable wrapper: pip_install say so that the command includes it? Even make it part of the existing callable pip from setup_dependent_packages?

adamjstewart · 2022-08-15T23:14:56Z

@pradyunsg this is going to be a bit intrusive since we need to do this in dozens of places. Was this always the case with pip or is this bug only present in newer versions of pip? We could instead patch pip to restore the previous behavior if this is new.

adamjstewart · 2023-05-12T15:21:19Z

I'm going to close this. It won't work unless we do it in the base class and literally every package that invokes pip directly. I think the fix should either be on pip's side or on the side of the Debian/RHEL developers who broke things in the first place.

spackbot-app bot added python update-package labels Aug 4, 2022

spackbot-app bot requested a review from adamjstewart August 4, 2022 23:19

vsoch mentioned this pull request Aug 4, 2022

add start of spack base image for flux-core flux-framework/flux-core#4471

Merged

adamjstewart self-assigned this Aug 4, 2022

adamjstewart marked this pull request as draft August 14, 2022 17:40

spackbot-app bot added build-systems core PR affects Spack core functionality labels Aug 15, 2022

vsoch and others added 2 commits August 15, 2022 16:04

Check for docutils bin

f764ead

If docutils doesn't have a bin, then don't try to iterate over files in it.

if only a "local" directory is generated, symlink

bedbc2c

Work around the debian and Fedora bug by symlinking the contents of the local directory if it appears.

trws force-pushed the packages/py-docutils-bug branch from 422af55 to bedbc2c Compare August 15, 2022 23:04

adamjstewart changed the title ~~Check for docutils bin~~ Fix installations with system Python on Ubuntu/Fedora Aug 15, 2022

adamjstewart reviewed Aug 15, 2022

View reviewed changes

lib/spack/spack/build_systems/python.py Outdated Show resolved Hide resolved

remove extra check

c4691fa

adamjstewart reviewed Aug 15, 2022

View reviewed changes

adamjstewart mentioned this pull request Sep 15, 2022

If using an external Python, the bin directory might not be in PATH #32662

Closed

3 tasks

adamjstewart mentioned this pull request Sep 22, 2022

Installation issue: py-docutils (symlink error) #32769

Closed

4 tasks

adamjstewart mentioned this pull request Oct 21, 2022

Bug fix for py-cython with python 3.10+ (installs in subdir local) JCSDA/spack#186

Merged

adamjstewart mentioned this pull request Nov 15, 2022

PYTHONPATH set by spack is broken on Fedora 37 #33923

Closed

3 tasks

adamjstewart mentioned this pull request May 12, 2023

Installation issue: meson 1.1.0 binary goes into local/bin/ instead of bin/ and dependent builds fail #37632

Closed

4 tasks

adamjstewart closed this May 12, 2023

alalazo deleted the packages/py-docutils-bug branch May 12, 2023 15:24

adamjstewart mentioned this pull request Aug 3, 2023

Installation issue: glib cannot find meson binary #39245

Closed

4 tasks

adamjstewart mentioned this pull request Dec 1, 2023

python: always use a venv #40773

Merged

Conversation

vsoch commented Aug 4, 2022

Uh oh!

adamjstewart commented Aug 4, 2022

Uh oh!

vsoch commented Aug 4, 2022

Uh oh!

adamjstewart commented Aug 5, 2022

Uh oh!

vsoch commented Aug 5, 2022

Uh oh!

vsoch commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

vsoch commented Aug 12, 2022

Uh oh!

adamjstewart commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

vsoch commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

trws commented Aug 12, 2022

Uh oh!

vsoch commented Aug 14, 2022

Uh oh!

adamjstewart commented Aug 14, 2022

Uh oh!

pradyunsg commented Aug 15, 2022

Uh oh!

vsoch commented Aug 15, 2022

Uh oh!

adamjstewart commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Expected behavior

Actual behavior

Uh oh!

pradyunsg commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsoch commented Aug 15, 2022

Uh oh!

trws commented Aug 15, 2022

Uh oh!

pradyunsg commented Aug 15, 2022

Uh oh!

pradyunsg commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pradyunsg commented Aug 15, 2022

Uh oh!

adamjstewart commented Aug 15, 2022

Uh oh!

trws commented Aug 15, 2022

Uh oh!

Uh oh!

adamjstewart Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

trws Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

trws Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

adamjstewart Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

trws Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

adamjstewart commented Aug 15, 2022 •

edited

Loading

pradyunsg commented Aug 15, 2022 •

edited

Loading

pradyunsg commented Aug 15, 2022 •

edited

Loading