search

pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.49k stars 3.01k forks source link

untar_file: remove common leading directory before unpacking #12799

Closed encukou closed 2 months ago

encukou commented 3 months ago

Fixes: #12781

This should fix the issue with unpacking hardlinks in tarballs.

encukou commented 3 months ago

It should be possible, but I'm a bit lost in the codebase. What's a best way to add a functional test that builds a sdist and checks that it installs?

In this case, test sdist contain hardlinks and symlinks. Not all OSes allow those on disk, but they should handle a tarball that contains them, so it might need to be built with tarfile rather than something like setuptools.

pfmoore commented 3 months ago

I think you can add a pre-built tarfile to tests/data/packages and reference that via one of our test fixtures (data). Something like

def test_something(script: PipTestEnvironment, data: TestData):
    result = script.pip("install", "--no-index", "--find-links", data.find_links, "my_funky_sdist")

Or yes, you could do it manually by building a sdist in a temporary directory using the tarfile module. It's a bit more obvious what's going on if you do that, but it's more work.

encukou commented 3 months ago

I'll set up a Windows machine tomorrow to debug this.

pfmoore commented 3 months ago

They look unrelated to this change, I've just hit "rerun" in case they are intermittent failures.

encukou commented 3 months ago

Turns out it's because tarfile doesn't rewrite symlink targets on Windows. sdists with symlinks that point to another directory are not (and AFAICS never were) unpacked correctly on Windows with symlinks enabled (i.e. in Developer Mode).

This became a bug in CPython after Windows started supporting symlinks. Fixing it there is not a high priority for me. Using os.path.sep in pip tests should be forward-compatible with a future fix.

pfmoore commented 3 months ago

Looks like there's still a CI failure to fix, and the branch needs to be brought up to date with main.