Skip to content

change approach for vendoring parsed requirements#679

Merged
alexeagle merged 1 commit intomainfrom
i677
Apr 22, 2022
Merged

change approach for vendoring parsed requirements#679
alexeagle merged 1 commit intomainfrom
i677

Conversation

@alexeagle
Copy link
Contributor

@alexeagle alexeagle commented Apr 1, 2022

Instead of printing something different in the pip_parse tooling, just suggest that users continue
to run the repository rule, but check the output into their repo, then load from that instead
of from the generated repository.

This is simpler than trying to work out what correct arguments to pass to the tool when running it
outside of the pip_parse starlark context.

Fixes #677.

all_whl_requirements = ["@pip_parsed_certifi//:whl", "@pip_parsed_charset_normalizer//:whl", "@pip_parsed_idna//:whl", "@pip_parsed_requests//:whl", "@pip_parsed_urllib3//:whl"]

_packages = [('pip_parsed_certifi', 'certifi==2021.10.8 --hash=sha256:78884e7c1d4b00ce3cea67b44566851c4343c120abd683433ce934a68ea58872 --hash=sha256:d62a0163eb4c2344ac042ab2bdf75399a71a2d8c7d47eac2e2ee91b9d6339569'), ('pip_parsed_charset_normalizer', 'charset-normalizer==2.0.12 --hash=sha256:2857e29ff0d34db842cd7ca3230549d1a697f96ee6d3fb071cfa6c7393832597 --hash=sha256:6881edbebdb17b39b4eaaa821b438bf6eddffb4468cf344f09f89def34a8b1df'), ('pip_parsed_idna', 'idna==3.3 --hash=sha256:84d9dd047ffa80596e0f246e2eab0b391788b0503584e8945f2368256d2735ff --hash=sha256:9d643ff0a55b762d5cdb124b8eaa99c66322e2157b69160bc32796e824360e6d'), ('pip_parsed_requests', 'requests==2.27.1 --hash=sha256:68d7c56fd5a8999887728ef304a6d12edc7be74f1cfa47714fc8b414525c9a61 --hash=sha256:f22fa1e554c9ddfd16e6e41ac79759e17be9e492b3587efa038054674760e72d'), ('pip_parsed_urllib3', 'urllib3==1.26.9 --hash=sha256:44ece4d53fb1706f667c9bd1c648f5469a2ec925fcf3a776667042d645472c14 --hash=sha256:aabaf16477806a5e1dd19aa41f8c2b7950dd3c746362d7e3223dbe6de6ac448e')]
_config = {'python_interpreter': 'python3', 'python_interpreter_target': '@python39_aarch64-apple-darwin//:bin/python3', 'quiet': True, 'timeout': 600, 'repo': 'pip_parsed', 'isolated': True, 'extra_pip_args': [], 'pip_data_exclude': [], 'enable_implicit_namespace_pkgs': False, 'environment': {}, 'repo_prefix': 'pip_parsed_'}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugh, this line gets a resolved interpreter which is platform-dependent and can't be checked in.
@python39//:python3 would work fine here, as it's an alias to the platform interpreter, however we use rctx.rpath on the label provided and that doesn't work with alias labels.
Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay we can just "fix it in post" - when we copy this file, replace it back with the symbolic reference

@alexeagle
Copy link
Contributor Author

This is an annoying change because I can't find a way to exclude the generated .bzl file from buildifier, so to get a green CI I had to make formatting tweaks to fix what we generate, and then more tweaks on the result in the genrule, sigh

Copy link
Contributor

@hrfuller hrfuller left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

Instead of printing something different in the pip_parse tooling, just suggest that users continue
to run the repository rule, but check the output into their repo, then load from that instead
of from the generated repository.

This is simpler than trying to work out what correct arguments to pass to the tool when running it
outside of the pip_parse starlark context.
@rahul-theorem
Copy link

rahul-theorem commented Apr 22, 2022

@alexeagle would you be willing to cut a hotfix release with this change?

@alexeagle
Copy link
Contributor Author

I plan to cut a regular release tomorrow.

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.

Regression caused by #655

4 participants