Some packages seem to be recompiled unnecessarily for coverage

[linzhp]

bazel coverage shouldn't need to recompile packages that don't need coverage instrumentation. For some packages, this is not the case. The recompilation also changes the __.PKGDEF file

What version of rules_go are you using?

0.24.4

What version of gazelle are you using?

0.22.0

What version of Bazel are you using?

3.6.0

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Darwin and Linux, amd64

What did you do?

$ git clone git@github.com:linzhp/bazel_examples.git
$ cd bazel_examples
$ bazel build //sqlite:go_default_library
$ bazel coverage --profile=/tmp/profile.json sqlite:go_default_test
$ bazel analyze-profile /tmp/profile.json

What did you expect to see?

No recompilation needed for external/com_github_mattn_go_sqlite3/go-sqlite3.a, so the action GoCompilePkg external/com_github_mattn_go_sqlite3/go-sqlite3.a should a take very short time.

What did you see instead?

=== PHASE SUMMARY INFORMATION ===

Total launch phase time         0.031 s    0.39%
Total init phase time           0.049 s    0.63%
Total target pattern evaluation phase time    0.001 s    0.02%
Total interleaved loading-and-analysis phase time    0.408 s    5.19%
Total preparation phase time    0.006 s    0.09%
Total execution phase time      7.373 s   93.66%
Total finish phase time         0.001 s    0.01%
------------------------------------------------
Total run time                  7.871 s  100.00%

Critical path (7.350 s):
       Time Percentage   Description
    0.91 ms    0.01%   action 'GoToolchainBinaryCompile external/go_sdk/builder.a [for host]'
    0.94 ms    0.01%   action 'GoToolchainBinary external/go_sdk/builder [for host]'
    2.54 ms    0.03%   action 'GoCompilePkg external/org_golang_x_tools/internal/lsp/fuzzy/go_tool_library.a'
    1.42 ms    0.02%   action 'GoCompilePkg external/org_golang_x_tools/internal/analysisinternal/go_tool_library.a'
    1.14 ms    0.02%   action 'GoCompilePkg external/org_golang_x_tools/go/analysis/go_tool_library.a'
    1.16 ms    0.02%   action 'GoCompilePkg external/org_golang_x_tools/go/analysis/passes/inspect/go_tool_library.a'
    1.64 ms    0.02%   action 'GoCompilePkg external/org_golang_x_tools/go/analysis/passes/composite/go_tool_library.a'
    1.30 ms    0.02%   action 'GoCompilePkg default_nogo~nogo.a'
    1.25 ms    0.02%   action 'GoLink default_nogo_/default_nogo'
    7.333 s   99.77%   action 'GoCompilePkg external/com_github_mattn_go_sqlite3/go-sqlite3.a'
    0.53 ms    0.01%   action 'GoCompilePkg sqlite/go_default_test.internal.a'
    0.49 ms    0.01%   action 'GoCompilePkg sqlite/go_default_test_test.external.a'
    0.46 ms    0.01%   action 'GoCompilePkg sqlite/go_default_test~testmain.a'
    0.39 ms    0.01%   action 'GoLink sqlite/go_default_test_/go_default_test'
    0.04 ms    0.00%   runfiles for //sqlite go_default_test
    2.55 ms    0.03%   action 'Testing //sqlite go_default_test'

[jayconrod]

I think by default, bazel coverage instruments all targets that tests depend on but not the tests on themselves. That's different from go test, which only instruments the libraries under test but not their dependencies. The --instrumentation_filter flag lets you control which targets are instrumented.

__.PKGDEF is probably changing because the instrumentation adds a new dependency of @io_bazel_rules_go//go/tools/coverdata in each target.

[linzhp]

I think by default, bazel coverage instruments all targets that tests depend on but not the tests on themselves.

Due to bazelbuild/bazel#11971, the default --instrumentation_filter is more limited after Bazel 3.6:

bazel coverage --profile=/tmp/profile.json sqlite:go_default_test
INFO: Using default value for --instrumentation_filter: "^//sqlite[/:]".
INFO: Override the above default with --instrumentation_filter
INFO: Build options --collect_code_coverage, --instrumentation_filter, and --test_timeout have changed, discarding analysis cache.
INFO: Analyzed target //sqlite:go_default_test (0 packages loaded, 7499 targets configured).
INFO: Found 1 test target...
Target //sqlite:go_default_test up-to-date:
  bazel-bin/sqlite/go_default_test_/go_default_test
INFO: Elapsed time: 5.810s, Critical Path: 5.48s
INFO: 2 processes: 1 internal, 1 darwin-sandbox.
INFO: Build completed successfully, 2 total actions
//sqlite:go_default_test                                        (cached) PASSED in 2.8s
  /private/var/tmp/_bazel_zplin/1b23f78923ad6c0028e8aab1a0a6d5fb/execroot/__main__/bazel-out/darwin-fastbuild/testlogs/sqlite/go_default_test/coverage.dat

Executed 0 out of 1 test: 1 test passes.
INFO: Build completed successfully, 2 total actions

In this case, @com_github_mattn_go_sqlite3//:go-sqlite3 does not match "^//sqlite[/:]", but still gets recompiled.

[jayconrod]

Hmm, I'm not sure what would cause that then.

rules_go uses ctx.coverage_instrumented to decide whether or not to instrument a target. That basically just says whether the target matched --instrumentation_filter. It would be good to confirm what that method returns for that target. If it returns True, it may be a Bazel bug.

Assuming it returns False, something else is causing the action to miss the cache. bazel aquery should turn up some details about what's different in the two configurations.

[linzhp]

ctx.coverage_instrumented is False for @com_github_mattn_go_sqlite3//:go-sqlite3 when the instrumentation filter doesn't match it. However bazel aquery only returns the configuration for bazel build, not bazel coverage. I can't run bazel aquery --collect_code_coverage because it would return the configuration with all packages instrumented.