ali5h / rules_pip

pip package rules for bazel that are fast (incremental fetch), support different python versions and work with all kinds of packages (i.e. packages with namespaces)
MIT License
60 stars 24 forks source link

Py-compile-3.7: change args per issue 811 #71

Closed chickenandpork closed 5 months ago

chickenandpork commented 2 years ago

As discussed in https://github.com/jazzband/pip-tools/issues/811 with a quick change at https://github.com/jazzband/pip-tools/issues/811#issuecomment-1058233070, py-compile-3.7.0 no longer accepts --no-index, fairly suddenly as well.

The --no-emit-index-url is a replacement.

Probably need to discuss how to properly and gracefully maintain compatibility: probably need to make it check the version of the interpreter, or allow user-setting.

ali5h commented 2 years ago

please rebase on master, thanks

chickenandpork commented 2 years ago

rebased, I think that should work...

I had merged this MR against my master branch in my fork; pushing a rebase to the PR brach, the pull request should map out to a delta on top of effectively your master branch.

My concern is that it tracks a change in pip-tools, so it requires that if the user supplies a pip-tools, they have to be the correct version. I guess that's a fairly rare circumstance, so not a significant risk.

Workflow is pending approval so I can't check it myself. Please let me know.

chickenandpork commented 2 years ago

@ali5h rebased

ali5h commented 2 years ago

I don't fully understand your concern. what do you mean user provides the pip-tools?

fahhem commented 2 years ago

piptools (in which pip-compile is from) is vendored into the repo under third_party to solve the chicken-or-egg problem of getting rules_pip's dependencies via pip. As a result, our version of pip-compile will always be 6.4.0: https://github.com/ali5h/rules_pip/tree/master/third_party/py/pip_tools-6.4.0.dist-info

Can you verify that the issue a) is in 6.4.0, b) is solved by your change, and c) there is a test that compiles the requirements.txt?

I haven't 100% checked, but it seems to me that this isn't an issue since the generated requirements.txt isn't checked into user repositories, only kept in cache by bazel so the index URL being in the output doesn't matter. Is that the case?