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

Add Seek fallback for zip files #2320

Merged
merged 4 commits into from
Mar 10, 2024
Merged

Conversation

charliermarsh
Copy link
Member

Summary

Some zip files can't be streamed; in particular, rs-async-zip doesn't support data descriptors right now (though it may in the future). This PR adds a fallback path for such zips that downloads the entire zip file to disk, then unzips it from disk (which gives us Seek).

Closes #2216.

Test Plan

cargo run pip install --extra-index-url https://buf.build/gen/python hashb_foxglove_protocolbuffers_python==25.3.0.1.20240226043130+465630478360 --force-reinstall -n

@charliermarsh charliermarsh added bug Something isn't working compatibility Compatibility with another interface or tool labels Mar 10, 2024
@charliermarsh charliermarsh force-pushed the charlie/data-descriptors branch 2 times, most recently from 88024f5 to a608a9d Compare March 10, 2024 02:33
@charliermarsh
Copy link
Member Author

charliermarsh commented Mar 10, 2024

Somewhat torn on merging this. It's good behavior but it does add complexity, and rs-async-zip might add data descriptor support soon (TBD) -- see: Majored/rs-async-zip#122.

@charliermarsh charliermarsh force-pushed the charlie/data-descriptors branch 3 times, most recently from ff9a92b to bd727ba Compare March 10, 2024 02:53
@charliermarsh
Copy link
Member Author

Eh, ultimately, this is a good thing to do.

@charliermarsh
Copy link
Member Author

All these Windows tests are randomly overflowing their stacks is confusing to me. This shouldn't have changed the stack at all?

charliermarsh added a commit that referenced this pull request Mar 10, 2024
## Summary

Improves consistency and helps with the kinds of failures seen in
#2320.
@charliermarsh charliermarsh merged commit a267a50 into main Mar 10, 2024
7 checks passed
@charliermarsh charliermarsh deleted the charlie/data-descriptors branch March 10, 2024 15:39
@konstin
Copy link
Member

konstin commented Mar 10, 2024

All these Windows tests are randomly overflowing their stacks is confusing to me. This shouldn't have changed the stack at all?

Every local variable increases the stack frame size 😭

@charliermarsh
Copy link
Member Author

Nooooo

@charliermarsh
Copy link
Member Author

It must be so marginal

@konstin
Copy link
Member

konstin commented Mar 10, 2024

debug is just super inefficient with stack space, this never happens on release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Compatibility with another interface or tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data descriptors aren't supported in zip files
2 participants