Skip to content
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

Avoid erroring for source distributions with symlinks in archive #1944

Merged
merged 5 commits into from
Feb 24, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Feb 23, 2024

Summary

For context, we have three extraction paths:

  • untar (async) - used for any .tar.gz, local or remote.
  • unzip (async) - used to unzip remote wheels, or local or remote source distributions.
  • unzip (sync) - used to untar locally-available wheels into the cache.

We use three different crates for these:

These all seem to have different support for symlinks:

  • tokio-tar tries to create a symlink (which works fine on Unix but errors on Windows, since we typically don't have elevated permissions).
  • async-zip seems to write the target contents directly to the file (which is what we want).
  • zip-rs apparently writes the name of the target to the file (which isn't what we want).

Thankfully, symlinks are not allowed in wheels (pypa/pip#5919, https://discuss.python.org/t/symbolic-links-in-wheels/1945), so we can ignore zip-rs.

For tokio-tar, we now skip (and warn) if we see a symlink on Windows. We could do what pip does, and recursively copy, but it's difficult because we don't have Seek on the file. (Alternatively, we could use hard links and junctions, though those also might need to exist already.) Let's see how far this gets us.

(We also no longer attempt to set permissions on symlinks on Unix, which caused another failure.)

Closes #1858.

@charliermarsh charliermarsh added the bug Something isn't working label Feb 23, 2024
@charliermarsh
Copy link
Member Author

I need to test this with a symlinked directory before merging.


// On Windows, skip symlink entries, as they're not supported. pip recursively copies the
// symlink target instead.
#[cfg(windows)]
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this if cfg!(windows) so the branch gets checked when compiling on unix?

@charliermarsh
Copy link
Member Author

Okay, I mean, weird things happen if you include a symlink directory. On macOS, if you run create a symlink directory in pgpdump-1.5, then run zip -r pgpdump-1.5.zip pgpdump-1.5, then unzip, the symlink directory is just a file (not a symlink, and not a directory). If you compress and uncompress in the Finder UI, the symlink directory is preserved, but async-zip fails to unzip it with feature not supported: 'stream reading entries with data descriptors (planned to be reintroduced)'. Meanwhile, tokio-tar will untar it, but it too gets represented as a file.

@charliermarsh charliermarsh enabled auto-merge (squash) February 24, 2024 03:18
@charliermarsh charliermarsh merged commit a1f5041 into main Feb 24, 2024
7 checks passed
@charliermarsh charliermarsh deleted the charlie/sym branch February 24, 2024 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unpacking source distribution fails when there is a symlink in the archive
2 participants