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

Use bootstrap_file for archive bootstrap files #248

Merged
merged 1 commit into from
May 9, 2024

Conversation

cjntaylor
Copy link
Contributor

Fixes #247

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Fixes linkedin#247
@cjntaylor
Copy link
Contributor Author

I've updated this PR to reflect changes in the most recent release to use the resource name instead of the path name.

Using path.name as in the original implementation defeats the purpose of using the importlib.resources api. The documentation specifically calls out that the as_file method (and it's predecessors) are allowed to materialize the requested file(s) in a temporary location with arbitrary names. The code only works as written because under normal circumstances, the shiv module is already unpacked and the files are just referred to directly.

The whole point of this API is that you're not supposed to assume the latter behaviour. This breaks down in cases where the module isn't unpacked (shiv itself is in a zip file, or embedded in something like pyoxidizer). A name property is specifically provided that doesn't make this assumption and works in both cases.

Please consider pulling in this change. I'm happy to make any alterations you'd like, or even to iterate with you on other approaches. I'd really like to be able to use shiv without needing to maintain my own fork 😉

@lorencarvalho lorencarvalho merged commit db8e844 into linkedin:main May 9, 2024
1 check passed
@lorencarvalho
Copy link
Contributor

Hi @cjntaylor,
Thanks so much for your contribution and especially for your patience 😓

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

Successfully merging this pull request may close these issues.

Incorrect bootstrap filename
2 participants