experimental_index_url in dependency tree breaks other pip.parse includes
#2268
Comments
|
So if I understand correctly this breaks when trying to use the I think the only solution, that I haven't gotten to doing yet is to implement a
|
yes
I haven't read extensively about what the purpose of |
|
The flag purpose is to support #260, but there were many things why it was not deemed as done and not enabled by default. The TLDR of what it (the feature) does:
|
With this PR we are introducing a new extension for managing PyPI dependencies and it seems that we would fix bazelbuild#2268 in doing so. The code has been sitting around for long enough so that we have ironed out most of the bigger bugs already and the maintainers have agreed for long enough that pip.parse is not the best named, so I have chosen pypi.install. This should not actively break the code and will make some of the experimental parameters noop. This could yield to weird failures in cases where people are building docker containers, so I am thinking that we should potentially add a helpful error message prompting the users to migrate. However I am not sure how that works out when multiple module versions are at play in the same module graph.
|
I think I gave it enough thought and the best way I could think of to solve this is to move this The list of changes are:
I think the |
This PR introduces a new `parse_modules` function in the `pypi/extension.bzl` code to mimic the structure of the `python` extension and to make it easier to write unit tests against the extension itself. I have also written a few unit tests to verify the generic structure. Work towards #2268.
…azelbuild#2325) With this change we finally generate the same lock file within the legacy code `pip.parse` code path and it allows to slowly transition to using the new code path as much as possible without user doing anything. This moves the selection of the host-compatible lock file from the extension evaluation to the build phase - note, we will generate extra repositories here which might not work on the host platform, however, if the users are consuming the `whl_library` repos through the hub repo only, then everything should work. A known issue is that it may break `bazel query` and in these usecases it is advisable to use `cquery` until we have `sdist` cross-building from source fully working. Summary: - feat: reuse the `render_pkg_aliases` for when filename is not known but platform is known - feat: support generating the extra config settings required - feat: `get_whl_flag_versions` now generates extra args for the rules - feat: make lock file generation the same irrespective of the host platform - test: add an extra test with multiple requirements files - feat: support cross-platform builds using `download_only = True` in legacy setups Note, that users depending on the naming of the whl libraries will need to start using `extra_hub_aliases` attribute instead to keep their setups not relying on this implementation detail. Fixes bazelbuild#2268 Work towards bazelbuild#260 --------- Co-authored-by: Richard Levasseur <[email protected]>
In our project we don't use
experimental_index_url, but through pulling inrules_mypywhich does use it here https://github.com/theoremlp/rules_mypy/blob/c7c113c3608bc7569493c2abbff9aaa18119e145/MODULE.bazel#L29, our project's root pip.parse uses stopped working, because a platform specific variant of a dependency was included in the MODULE.bazel.lock.I would expect
experimental_index_urlto only apply to the pip.parse that sets it, or that it only be settable by the root module. The only workaround I found is to patch that out of rules_mypy, which requires an unreleased version of bazel.Here is a repro project repro.zip
In this project run
bazel fetch ...on a non-linux x86_64 machine to see the breakage. The specific issue is that the requirements.txts that we use vary per platform, because pytorch releases different wheel types on different platforms. On linux x86_64, torch+cpu exists, but on macOS or linux aarch64 it doesn't exist, which leads to this download failure on those platforms:Since in the case of rules_mypy it was only trying to set this for its internal dependency, it was a big surprise that it could break us in this way.
The text was updated successfully, but these errors were encountered: