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

Bazel produces an over-aggressive instrumentation_filter #11962

Closed
linzhp opened this issue Aug 18, 2020 · 0 comments
Closed

Bazel produces an over-aggressive instrumentation_filter #11962

linzhp opened this issue Aug 18, 2020 · 0 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Server Issues for serverside rules included with Bazel type: bug

Comments

@linzhp
Copy link
Contributor

linzhp commented Aug 18, 2020

Description of the problem / feature request:

If one of the targets passed to bazel coverage is one level below the root of the repo (e.g., //foo in bazel coverage //foo:go_default_test //bar/baz:go_default_test), Bazel would infer the --instrumentation_filter to be //. This is undesirable, because it will instrument the test coverage for the whole transitive dependency of both //foo:go_default_test and //bar:go_default_test, including third-party libraries.

When we run bazel coverage //foo:go_default_test //bar/baz:go_default_test, we are only interested in the test coverage of those two packages, so only those two packages need to be instrumented. When we run bazel test //foo:go_default_test //bar/baz:go_default_test and then bazel coverage //foo:go_default_test //bar/baz:go_default_test, we would expect only //foo:go_default_test and //bar/baz:go_default_test are rebuilt because of the extra instrumentation code. Their dependencies should not need rebuild. Due to the over-instrumentation, the whole transitive graph is rebuilt.

Even for targets deep below, the behavior is not desirable either. For example, bazel coverage //foo/bar:go_default_test //foo/baz:go_default_test would have --instrumentation_filter=//foo[:/] by default. As a result all transitive dependencies under //foo/ will have to rebuild between bazel coverage and bazel test, even when only //foo/bar and //foo/baz need to be instrumented for test coverage.

I think Bazel should either turn off such optimization entirely, or only optimize for certain wildcard patterns such as //foo/..., such bazel test and bazel coverage can share cache except the tested targets.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

bazel coverage --announce_rc //foo:go_default_test //bar/baz:go_default_test

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 3.4.1-homebrew

Have you found anything relevant by searching the web?

optimizeFilterSet transformed packageFilters from {"foo", "bar/baz"} to {""}, which becomes //[/:] later. Then this line further transformed it into //.

@linzhp linzhp changed the title Bazel should only optimize instrumentation_filter for wildcard patterns Bazel produces an over-aggressive instrumentation_filter Aug 18, 2020
@gregestren gregestren added team-Rules-Server Issues for serverside rules included with Bazel type: bug untriaged labels Aug 18, 2020
@c-mita c-mita added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Server Issues for serverside rules included with Bazel type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants