pip icon indicating copy to clipboard operation
pip copied to clipboard

untar_file: remove common leading directory before unpacking

Open encukou opened this issue 1 year ago • 6 comments

Fixes: #12781

This should fix the issue with unpacking hardlinks in tarballs.

encukou avatar Jun 26 '24 13:06 encukou

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.

encukou avatar Jun 26 '24 16:06 encukou

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.

pfmoore avatar Jun 26 '24 16:06 pfmoore

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

encukou avatar Jun 26 '24 20:06 encukou

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

pfmoore avatar Jun 26 '24 20:06 pfmoore

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.

encukou avatar Jun 27 '24 13:06 encukou

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

pfmoore avatar Jun 28 '24 09:06 pfmoore