Skip to content

gh-75229: ensurepip does not honour the value of $(prefix) #17634

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

Closed

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Dec 17, 2019

Co-Authored-By: Xavier de Gaye <xdegaye@gmail.com>
@vstinner
Copy link
Member

@pganssle: Would you mind to have a look at this PR? Thanks in advance.

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a look on request by @pganssle.

The change looks good to me. I'm not sure why someone would want to have --prefix specified to ensurepip but if they want to be able to, this looks right to me.

@pganssle
Copy link
Member

I can't say that I know much about ensurepip and whether this is the right solution (I don't even fully understand the problem). Looks like @ncoghlan seemed to think it is, and the code seems clean enough.

My one reservation is that this only adds a test using a mock, it doesn't actually test that this solves the problem - it assumes that pip solves the problem. That seems fair since pip has its own test suite, but it's still something to consider.

@pganssle
Copy link
Member

The change looks good to me. I'm not sure why someone would want to have --prefix specified to ensurepip but if they want to be able to, this looks right to me.

I think this is the crux of whether or not to accept this PR to me. To be honest I'm not entirely sure what ensurepip does, because any time I build python, it comes with pip already installed, so I'm not sure why end-users need to invoke this.

If this is just about cross-compiling Python, would it be enough to set PIP_PREFIX=$PREFIX in Makefile?

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ZackerySpytz! Would you be willing to update this PR with the current main, so that it is up to date?

(the second scenario here has helpful guidance)

@pradyunsg pradyunsg added the 3.13 bugs and security fixes label Oct 12, 2023
pradyunsg
pradyunsg previously approved these changes Oct 12, 2023
pradyunsg
pradyunsg previously approved these changes Oct 12, 2023
@pradyunsg pradyunsg dismissed their stale review October 12, 2023 15:07

Oops. That was meant for the bot.

@vstinner
Copy link
Member

@pradyunsg: @ZackerySpytz is not very active these days, you should copy this PR to create a new one.

@erlend-aasland erlend-aasland changed the title bpo-31046: ensurepip does not honour the value of $(prefix) gh-75229: ensurepip does not honour the value of $(prefix) Jan 5, 2024
@ZackerySpytz ZackerySpytz requested a review from pfmoore as a code owner January 5, 2024 14:46
erlend-aasland and others added 2 commits January 5, 2024 15:46
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
@erlend-aasland erlend-aasland removed the 3.13 bugs and security fixes label Jan 5, 2024
@@ -120,27 +120,27 @@ def _disable_pip_configuration_settings():
os.environ['PIP_CONFIG_FILE'] = os.devnull


def bootstrap(*, root=None, upgrade=False, user=False,
def bootstrap(*, root=None, prefix=None, upgrade=False, user=False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add prefix at the end of the args list, as noted above. Also, supplying both root and prefix arguments doesn't really make sense (and may even be rejected by pip at some point in the future, if it isn't already) so it would be better to make these arguments mutually exclusive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it, I'm not sure they should be mutual exclusive.

If root equals $(DESTDIR) and prefix equals $(prefix), I would guess we want to install into $(DESTDIR)/$(prefix). I need to look more closely at this later today.

@bedevere-app
Copy link

bedevere-app bot commented Jan 5, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@erlend-aasland erlend-aasland requested a review from pfmoore January 5, 2024 16:32
@@ -1981,7 +1981,7 @@ install: @FRAMEWORKINSTALLFIRST@ commoninstall bininstall maninstall @FRAMEWORKI
install|*) ensurepip="" ;; \
esac; \
$(RUNSHARED) $(PYTHON_FOR_BUILD) -m ensurepip \
$$ensurepip --root=$(DESTDIR)/ ; \
$$ensurepip --prefix=$(prefix) ; \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about the Unix build process to know if using --prefix instead of --root is the right thing to do here. I'm going to mark my review as resolved (and approve the PR) but I'd recommend getting someone who knows how Unix/MacOS installs work to check this part of the change before merging (unless, of course, you are such a person and you're happy it's right 🙂)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can test it on *nix and macOS, but I know very little about pip/ensurepip :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Looking at the pip source, --prefix corresponds roughly to the sysconfig scheme posix_prefix. And --root can be used with --prefix, it basically rebases the installation directories on the root directory.

So if you have prefix=/foo/bar and root=/bax/quux, then you'd get purelib pointing to /baz/quux/foo/bar/lib/pythonX.Y/site-packages. With just prefix you'd get /foo/bar/lib/pythonX.Y/site-packages. And with just root you'd get /baz/quux/lib/pythonX.Y/site-packages.

There's all sorts of qualifications and special cases (because this is packaging 🙁) but that's the basic idea.

Given that the original issue was around cross-compilation for Android, I have zero idea how any of this would impact that actual scenario, though...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would guess you'd use --root for the directory where your target rootfs is being populated, and then you'd use --prefix for the path within the target rootfs.

cc. @gpshead, since you dabble with embedded systems IIRC.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not the right person to answer this either, i've poked #build on discord to see if anyone else is.

Copy link

@JasonGantner JasonGantner May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that using --prefix for the expected location in the destination filesystem and --root(=$DESTDIR) for the current location of the virtual filesystem where the package is being prepared makes total sense. (and is how setup.py works )

Having only one or the other leads to headaches;

TL;DR: We expect --prefix to change the install location of files and the path they use to reference one another while --root should change the location we put files without changing how they refer to one another

With a basic build process and environment as such :

SOURCE_DIR=/tmp/py3.12.3
TARGET_ROOT=/tmp/build.out
TARGET_PREFIX=/opt/python/py3.12.3
cd ${SOURCE_DIR}
configure --prefix=${TARGET_PREFIX} --with-ensurepip=no [...]
make
make DESTDIR=${TARGET_ROOT} install altinstall

We get these results :

${TARGET_ROOT}${TARGET_PREFIX}/bin/python3 -m ensurepip
head -n 1 ${TARGET_ROOT}${TARGET_PREFIX}/bin/pip
#!/tmp/build.out/opt/python/py3.12.3/bin/python3
${TARGET_ROOT}${TARGET_PREFIX}/bin/python -m ensurepip --root ${TARGET_ROOT}
head -n 1 ${TARGET_ROOT}${TARGET_ROOT}${TARGET_PREFIX}/bin/pip
#!/tmp/build.out/tmp/build.out/opt/python/py3.12.3/bin/python3

In both cases the script installed contains the ${TARGET_ROOT} in the shebang when it shouldn't, but specificaly using --root behaves the way I would expect --prefix to work if no prefix had been set in the configure script.

The expected install locations for pip3 should be:

  • --prefix : ${PREFIX}/bin/pip3 with shebang #!${PREFIX}/bin/python3
  • --root (assuming no prefix) : ${ROOT}/bin/pip3 with shebang #!/bin/python3
  • --prefix and --root : ${ROOT}${PREFIX}/bin/pip3 with shebang #!${PREFIX}/bin/python3

The behaviour becomes even weirder when using --with-ensurepip=(install|upgrade) in configure.
It triggers a call to something equivalent to :

LD_LIBRARY_PATH=${SOURCE_DIR} ${SOURCE_DIR}/python -E -m ensurepip [--upgrade] --root=${DESTDIR}

But this call ignores the --root parameter completely and just acts on the host system if an installation using the same prefix exists. The -E parameter makes setting PYTHONHOME irrelevant but omitting it doesn't resolve the issue.

With thing the way they are currently, with-ensurepip=no is the only option making sense in configure. After that, I see two quick and dirty solutions :

  • calling ${TARGET_ROOT}${TARGET_PREFIX}/bin/python3 -m ensurepip on the build host, and fixing the shebang in the scripts afterwards
  • adding ${TARGET_PREFIX}/bin/python3 -m ensurepip as a post-install action on the destination host

I agree that the proper solution would be to have the makefile call something along the lines of:

Suggested change
$$ensurepip --prefix=$(prefix) ; \
$$ensurepip --root=$(DESTDIR)/ --prefix=$(prefix) ; \

I'm not sure the trailing / is necessary.

One issue remains when cross-compiling (eg. AARCH64 binaries on a X86_64 host), the build host (x86_64) must be able to execute the target's binaries (AARCH64) for the actual call to work. Assuming ensurepip is pure python, the call should use the host's python3 binary (x86_64) with --prefix and --root rather than the one being built (AARCH64).

I don't see it as a blocking issue since the person building the package should be able to tell if they can use --with-ensurepip or if they should rather call ensurepip from the build host's python rather than letting the Makefile guess/try.

EDIT:
I forgot to mention that the actual module installation path follows the behaviour shown with the scripts and shebang. I just used them to illustrate the expected and actual behaviours

EDIT2: Added TL;DR an some clarifications

@@ -122,6 +123,12 @@ Module API
*verbosity* controls the level of output to :data:`sys.stdout` from the
bootstrapping operation.

*prefix* specifies the directory prefix to use when installing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should explain the difference between root and prefix better. Per now, it is not obvious how they differ.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's not obvious. And that's speaking as a pip maintainer... It's all shrouded in mystery and legacy behaviour dating back to the origins of distutils, I think. And it's very much to do with Unix - on Windows, those options are essentially never used (which is mostly why I don't understand them...)

But I think it's fair to assume that anyone using these options is doing so because they know of the same options in pip, and so they understand what they are doing (or at least their confusion isn't our problem). It's not ideal, but OTOH, this isn't the place for a tutorial on packaging concepts, either.

Copy link
Member

@merwok merwok May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root vs prefix goes back to the base unix model I think, and may be explained in make documentation or something similarly old.

Something along the lines: the prefix (for installation) can be /usr or /usr/local or /opt/my-program but the root directory (for building) is /tmp/something, so the actual part where files are created is /tmp/something/usr/local/bin/python but strings embedded in the programs/libraries/docs look like /usr/local/bin/python so that they’re good at runtime.

(don’t ask me which of the three values corresponds to DESTDIR)

(edit: better replies at #17634 (comment))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DESTDIR is the make equivalent of specifying --root

@mcepl
Copy link
Contributor

mcepl commented May 10, 2025

What is the status of this PR?

@pfmoore
Copy link
Member

pfmoore commented May 10, 2025

Not sure. The OP, @ZackerySpytz might no longer be active (there was a comment suggesting that above). I don't know if @erlend-aasland's concerns have been addressed - the Makefile.pre.in change certainly needs to be validated by someone with Unix expertise. It needs rebasing. Personally, I think it's mostly harmless, but extremely niche, and very much "use at your own risk". It's certainly not part of the intended use case for ensurepip.

Given that the motivation was related to Android builds of Python, and Android is now a supported platform even though this PR has had almost no interest in 6 years, I'm inclined to say that it probably isn't necessary.

@JasonGantner
Copy link

@pfmoore I've raised some concerns in my last comment a year ago (#17634 (comment)) about the changes in Makefile.pre.in, mainly:

  • --root must not be removed since it's needed when packaging or cross-compiling
  • --prefix and --root are complementary

I just noticed that there's and added check (https://github.com/python/cpython/pull/17634/files#diff-ecfb0d1f34501dd30722053b9b39dea9139ffdd30dfcdc0964bcb18b95e443e4R147-R148) that makes --root and --prefix mutually exclusive which they aren't.

In summary, the current state of --with-ensurepip is broken on unix systems for packaging and cross-compiling but working as intended when building python directly on the target system (in the final location). This patch would break things further if merged as is. (and force packagers to adjust their workarounds or create new ones rather than just deleting them)

@pfmoore
Copy link
Member

pfmoore commented May 16, 2025

@JasonGantner Thanks for the summary. Given that, and the lack of any further activity from @ZackerySpytz or anyone else interested in taking this PR forward, I'm going to close the PR. If anyone wants this functionality, they can create a new PR, either building on this one, or starting from scratch.

I understand the need for the functionality, and would support someone working on this, but it doesn't seem to be a simple change, so it needs someone able and willing to do the necessary work.

@pfmoore pfmoore closed this May 16, 2025
mcepl pushed a commit to openSUSE-Python/cpython that referenced this pull request Jun 13, 2025
When cross-compiling, the local Python interpreter that is used
to run `ensurepip` may not have the same value of `sys.prefix` as
the value of the 'prefix' variable that is set in the Makefile.

With the following values used to install Python locally for
a later copy to the files hierarchy owned by the 'termux'
application on an Android device:

    DESTDIR=/tmp/android
    prefix=/data/data/com.termux/files/usr/local

'make install' causes ensurepip to install pip in
$(DESTDIR)/usr/local instead of the expected $(DESTDIR)/$(prefix)
where is installed the standard library.

The attached patch fixes the problem. The patch was implemented
assuming that pip uses distutils for the installation (note that
setup.py also uses the --prefix option in the Makefile), but I
know nothing about pip so forgive me if the patch is wrong and
please just assume it is just a way to demonstrate the problem.

Fixes: python#75229
Fixes: https://bugs.python.org/issue31046
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Zackery Spytz <zspytz@gmail.com>
References: python#17634
Signed-off-by: Matěj Cepl <mcepl@cepl.eu>
mcepl pushed a commit to openSUSE-Python/cpython that referenced this pull request Jun 13, 2025
When cross-compiling, the local Python interpreter that is used
to run `ensurepip` may not have the same value of `sys.prefix` as
the value of the 'prefix' variable that is set in the Makefile.

With the following values used to install Python locally for
a later copy to the files hierarchy owned by the 'termux'
application on an Android device:

    DESTDIR=/tmp/android
    prefix=/data/data/com.termux/files/usr/local

'make install' causes ensurepip to install pip in
$(DESTDIR)/usr/local instead of the expected $(DESTDIR)/$(prefix)
where is installed the standard library.

The attached patch fixes the problem. The patch was implemented
assuming that pip uses distutils for the installation (note that
setup.py also uses the --prefix option in the Makefile), but I
know nothing about pip so forgive me if the patch is wrong and
please just assume it is just a way to demonstrate the problem.

Fixes: python#75229
Fixes: https://bugs.python.org/issue31046
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Zackery Spytz <zspytz@gmail.com>
References: python#17634
Signed-off-by: Matěj Cepl <mcepl@cepl.eu>
mcepl added a commit to openSUSE-Python/cpython that referenced this pull request Jun 13, 2025
When using `python -m ensurepip` with the `--root` option for staged
installations, the generated pip script contained an incorrect shebang
that pointed into the staging directory. This made the installation
unusable once the staging directory was removed.

This commit fixes the issue by using the internal pip `--executable`
option to force the shebang to point to the correct, final
interpreter path.

It also corrects related pathing issues:

- Removes the check that incorrectly disallowed using --root and
  --prefix together.
- Defaults the installation prefix to `/` when --root is used alone,
  ensuring installation occurs at the base of the staging directory.

References: python#17634 (comment)
Signed-off-by: Matěj Cepl <mcepl@cepl.eu>
@mcepl
Copy link
Contributor

mcepl commented Jun 13, 2025

Replaced by #135488, let’s make this ship sail!

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

Successfully merging this pull request may close these issues.