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

Transitive tags #4264

Closed
abergmeier opened this issue Dec 10, 2017 · 15 comments
Closed

Transitive tags #4264

abergmeier opened this issue Dec 10, 2017 · 15 comments

Comments

@abergmeier
Copy link
Contributor

Please provide the following information. The more we know about your system and use case, the more easily and likely we can help.

Description of the problem / feature request / question:

I wanted to enable bazel test //src/test/... --test_tag_filters=-android (can someone explain why the sdk/ndk are not set up in the workspace in the first place?). Turns out tagging all android-tests is quite the work. The interesting thing is that it might not be necessary if tags would be transitive.
Then we only would need to tag the base libraries and filegroups and viola - all tests would be correctly tagged.

Environment info

  • Bazel version (output of bazel info release): 0.8.1
@aj-michael
Copy link
Contributor

Just curious, which android tests under //src/test are causing you problems? Is it more than just //src/test/shell/bazel/android and //src/test/java/com/google/devtools/build/android?

@aj-michael
Copy link
Contributor

And the answer to “can someone explain why the sdk/ndk are not set up in the workspace in the first place” is that many Bazel developers dont have $ANDROID_HOME and $ANDROID_NDK_HOME set, and without them the repository functions will fail when they are triggered by loading any android target.

@abergmeier
Copy link
Contributor Author

Is it more than just //src/test/shell/bazel/android and //src/test/java/com/google/devtools/build/android?

Quite. Just do a grep -rn android --include='BUILD' src/test/* | cut -d ':' -f1 | sort -u.

@abergmeier
Copy link
Contributor Author

And the answer to [...] is that many ...

Not quite. The other solution would be to have a repository rule, which just downloads a predefined set of sdk/ndk, instead of forcing everyone to do that manually.

@laszlocsomor
Copy link
Contributor

@abergmeier : are you suggesting to add a transitive_test_tags attribute (or similar) to library rules?

If so, I see a couple problems with that: it's not clear how would a rule remove transitive tags from the set it inherited from its upstream rules (dependencies), and how would Bazel handle diamond dependencies when it depends on library rule via different paths and the set of transitive tags are different on the different paths.

How about using Bazel query instead? Get the rdeps of the base library and filter the results for tests?

@abergmeier
Copy link
Contributor Author

@laszlocsomor I think I would rather have transitive_tags as a implicit "property" on test rules (only available via Java!?). I would imaging a set of {tags(test) + tags(deps) + tags(data)}.
And then issue a command: bazel test //src/test/... --test_transitive_tag_filters=-android.
Do you think limiting, which tags bubble up the graph is necessary?
I explicitly do NOT want to use query, since it would turn my example from trivial to complicated.

@laszlocsomor
Copy link
Contributor

@abergmeier : I don't see what you mean by "implicit property" -- a rule attribute, a test tag, Bazel somehow knowing that a particular java_library contains Android code? Can you maybe show an example set of rules?

@abergmeier
Copy link
Contributor Author

@laszlocsomor The thing in my mind is something like a aspect implementation that simply passes on all tags of deps and srcs and data AND which is connected to a new option --test_transitive_tag_filters.

@ulfjack
Copy link
Contributor

ulfjack commented Dec 20, 2017

I'd prefer not to do that - it introduces another way to have spooky action at a distance. What I was talking about with @lberki the other week is to have a mechanism to configure certain subtrees of a repository, e.g., to add a java_plugin to all Java rules under //java/my/weird/package.

@abergmeier
Copy link
Contributor Author

I'd prefer not to do that

Ok. But then the only way of easily disabling testing android rules is too manually tag them. And then to add some way of figuring out, when such a tag is missing.

@aj-michael
Copy link
Contributor

Quite. Just do a grep -rn android --include='BUILD' src/test/* | cut -d ':' -f1 | sort -u.

src/test/java/com/google/devtools/build/android/BUILD
src/test/java/com/google/devtools/build/android/desugar/BUILD
src/test/java/com/google/devtools/build/android/desugar/dependencies/BUILD
src/test/java/com/google/devtools/build/android/desugar/runtime/BUILD
src/test/java/com/google/devtools/build/android/dexer/BUILD
src/test/java/com/google/devtools/build/android/idlclass/BUILD
src/test/java/com/google/devtools/build/android/junctions/BUILD
src/test/java/com/google/devtools/build/android/resources/BUILD
src/test/java/com/google/devtools/build/android/testing/manifestmerge/BUILD
src/test/java/com/google/devtools/build/android/ziputils/BUILD
src/test/java/com/google/devtools/build/lib/BUILD
src/test/java/com/google/devtools/build/lib/rules/android/BUILD
src/test/py/bazel/BUILD
src/test/shell/bazel/android/BUILD
src/test/shell/bazel/BUILD

As I said earlier, only //src/test/shell/bazel/android/... and //src/test/java/com/google/devtools/build/android/... should require an Android SDK. //src/test/java/com/google/devtools/build/lib/rules/android/... does not.

@abergmeier
Copy link
Contributor Author

As I said earlier, only //src/test/shell/bazel/android/... and //src/test/java/com/google/devtools/build/android/... should require an Android SDK. //src/test/java/com/google/devtools/build/lib/rules/android/... does not.

AFAIR you are wrong (reason being implicit dependencies). I will introduce the android tag if someone at Google helps with setting up non-android testing.

@ulfjack
Copy link
Contributor

ulfjack commented Jan 9, 2018

My suggestion is to provide a way to mark - say - the src/test/java/com/google/devtools/build/android subtree as Android. That's technically a manual step, but then it applies to all test targets within that tree.

On the other hand, I'm also in favor of moving all the Android stuff out of Bazel proper (and to rules_android), which would also solve the specific issue here.

@abergmeier
Copy link
Contributor Author

On the other hand, I'm also in favor of moving all the Android stuff out of Bazel proper (and to rules_android), which would also solve the specific issue here.

👍

@jin
Copy link
Member

jin commented Sep 3, 2019

All of the Android stuff are moving out of Bazel core. Tracked in #5354

@jin jin closed this as completed Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants