-
Notifications
You must be signed in to change notification settings - Fork 548
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
fix: Use Bazel's built-in zstd support in _python_repository_impl. #1227
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Bazel has built-in support for zstd [since 5.1](bazelbuild/bazel#15087). This simplifies the logic and makes python_repository work on systems where building from source fails.
Thanks @f0rmiga! I'll obviously make CI pass before asking to merge. |
Can you please sync to head and resolve the merge conflicts? CI also reports there are failures, something relating to setting file permissions for a directory that doesn't exist? They failed even after retrying, so they're likely legit. |
@jiawen im going to mark this as a draft. When you have this passing CI please take it out of draft. |
My apologies - my day job got in the way. Draft sounds good - looks like I have some fun chmod issues to resolve. |
Argh, this PR uncovered some grungy indygreg packaging details. The existing code works because:
So the existing code happens to work because:
@rickeylev Any suggestions? |
FYI, the |
Perhaps the If a separate command has to be run after download_and_extract to merge the two directories, I'm not sure it'll be a net win. Maybe
I don't follow -- how does this PR fix building from source? This PR basically removes the need to have the zstd and tar tools locally available. What does that have to do with building from source? |
This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. |
This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?" |
Bazel has built-in support for zstd since 5.1. This simplifies the logic and makes python_repository work on systems where building from source fails.