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

Fix warnings "target is both a rule and a file" #827

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mhucka
Copy link
Member

@mhucka mhucka commented Dec 9, 2024

Bazel 6.5.0 complains about

(04:17:26) WARNING: ..../external/local_config_tf/BUILD:8216:8:
target 'libtensorflow_framework.so' is both a rule and a file;
please choose another name for the rule
(04:17:26) WARNING: ..../external/local_config_tf/BUILD:8226:8:
target 'test_log_pb2.py' is both a rule and a file;
please choose another name for the rule

(Note: I elided the long pathnames in the messages above by replacing them with '....'.) The warnings come from definitions in third_party/tf/tf_configure.bzl where the names of a couple of rules are libtensorflow_framework.so and test_log_pb2.py. Changing the names to libtensorflow_framework_so and test_log_pb2_py resolves the problem. Other names are possible, of course, but those seem like good choices in order to keep their meanings clear.

Bazel 6.5.0 complains about

```
(04:17:26) WARNING: ..../external/local_config_tf/BUILD:8216:8:
target 'libtensorflow_framework.so' is both a rule and a file;
please choose another name for the rule
(04:17:26) WARNING: ..../external/local_config_tf/BUILD:8226:8:
target 'test_log_pb2.py' is both a rule and a file;
please choose another name for the rule
```

(Note: I elided the long pathnames in the messages above by replacing
them with '....'.) The warnings come from definitions in
`third_party/tf/tf_configure.bzl` where the names of a couple of rules
are `libtensorflow_framework.so` and `test_log_pb2.py`. Changing the
names to `libtensorflow_framework_so` and `test_log_pb2_py` resolves
the problem. Other names are possible, of course, but those seem
like good choices in order to keep their meanings clear.
@mhucka mhucka marked this pull request as ready for review December 9, 2024 05:47
Copy link
Collaborator

@MichaelBroughton MichaelBroughton left a comment

Choose a reason for hiding this comment

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

I think having the name libtensorflow_framework.so is helpful for static objects that get rolled into the wheel file. I'm not 100% confident that this name change can be done, but lets try it and see if wheel releases break or not :).

@mhucka
Copy link
Member Author

mhucka commented Dec 10, 2024

I didn't make this very clear, but the file names are unchanged (i.e., retain .so extensions). This only changes the name of a couple of rules defined in the Bazel configuration.

FWIW, I did test the result using TFQ's test (via "bazel test //tensorflow_quantum/..."), after building and installing the TFQ wheel built with these changes into the python virtual environment, and the tests passed. I can do other tests if you have suggestions?

@mhucka mhucka added kind/bug-report Something doesn't seem to work area/ci Concerns continuous integration workflows and infrastructure labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Concerns continuous integration workflows and infrastructure kind/bug-report Something doesn't seem to work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants