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

package_annotation additive_build_content doesn't add new target to pip hub repo #2187

Closed
klbrvik opened this issue Sep 5, 2024 · 3 comments · Fixed by #2369
Closed

package_annotation additive_build_content doesn't add new target to pip hub repo #2187

klbrvik opened this issue Sep 5, 2024 · 3 comments · Fixed by #2369

Comments

@klbrvik
Copy link

klbrvik commented Sep 5, 2024

🐞 bug report

Affected Rule

pip_parse rule

Is this a regression?

Unknown, haven't used it before and didn't try older versions.

Description

Trying to add new target in existing pip dependency utilizing package_annotation and additive_build_content. BUILD file content is updated but target is not visible upon querying pip dependency targets.

I'm trying to add libtorch cc_library so my CPP code can depend on it.

🔬 Minimal Reproduction

This forked repo PR which extends existing example pip_repository_annotations contains changes required for reproduction. To reproduce:

  1. clone forked repo, checkout branch new-target-in-pip-dep
  2. go into rules_python/examples/pip_repository_annotations
  3. run bazelisk build //...
  4. query for targets in torch pip dependency with bazelisk query @pip//torch:all, which returns:
@pip//torch:data
@pip//torch:dist_info
@pip//torch:pkg
@pip//torch:torch
@pip//torch:whl

Expectation is to have new target with label @pip//torch:libtorch as defined by cc_library... snippet mentioned above and can be seen in linked PR.

Note: contents of BUILD file are successfully updated, we can confirm be looking at examples/pip_repository_annotations/bazel-pip_repository_annotations/external/pip_torch/BUILD.bazel which contains cc_library... snippet at the end.

🌍 Your Environment

Operating System:

NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"

Output of bazel version:

Bazelisk version: v1.19.0
Build label: 7.3.1
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Aug 19 16:12:50 2024 (1724083970)
Build timestamp: 1724083970
Build timestamp as int: 1724083970

Rules_python version:

Recently forked main branch.

@aignas
Copy link
Collaborator

aignas commented Sep 5, 2024

A workaround is to use the Label @pip_torch//:my_added_target.

@aignas aignas changed the title package_annotation additive_build_content doesn't add new target to pip dependency package_annotation additive_build_content doesn't add new target to pip hub repo Sep 5, 2024
@klbrvik
Copy link
Author

klbrvik commented Sep 9, 2024

@aignas thanks, it works.

When you say it's workaround do you mean it's not the ideal way to do it but currently only possible way and should be fixed in future?

@aignas
Copy link
Collaborator

aignas commented Sep 9, 2024 via email

aignas added a commit to aignas/rules_python that referenced this issue Nov 4, 2024
aignas added a commit to aignas/rules_python that referenced this issue Nov 4, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
…repos (#2369)

Before this change, it was impossible for users to use the targets
created with `additive_build_content` whl annotation unless they relied
on the implementation detail of the naming of the spoke repositories and
had `use_repo` statements in their `MODULE.bazel` files importing the
spoke repos.

With #2325 in the works, users will have to change their `use_repo`
statements, which is going to be disruptive. In order to offer them an
alternative for not relying on the names of the spokes, there has to be
a way to expose the extra targets created and this PR implements a
method. Incidentally, the same would have happened if we wanted to
stabilize the #260 work and mark `experimental_index_url` as
non-experimental anymore.

I was hoping we could autodetect them by parsing the build content
ourselves in the `pip` extension, but it turned out to be extremely
tricky and I figured that it was better to have an API rather than not
have it.

Whilst at it, also relax the naming requirements for the
`whl_modifications` attribute.

Fixes #2187
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 a pull request may close this issue.

2 participants