bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 276 forks source link

Issues when upgrading to Bazel 5 #1362

Open zachgrayio opened 2 years ago

zachgrayio commented 2 years ago

Seeing some strange behavior when upgrading to Bazel 5 for a mid-sized Scala project I'm looking at.

The first issue we worked through was missing classes, and appeared after moving from bazel 4.2 and bazel 5; we resolved this by adding the missing dependency directly to the failing scala_library in question, so it's a trivially easy fix, but I'm not sure exactly why the behavior changed just in bumping the bazel version :thinking: but I'm a little rusty on how the dependency mode options in the scala toolchain interact with Bazel, could be there were changes there I guess?

Second issue that I find even more perplexing (redacted with foo, this is a real project):

ERROR: file 'foo/it.runfiles/MANIFEST' is generated by these conflicting actions:
Label: //foo:it
RuleClass: scala_test rule
Configuration: fb3f630c9664150b7a5df27d2d6586a50d90442e5149d0267894e093602b1d77, a5e5a51a25dac85941e5a079e772850bacd3ab2905204bf78bdc62c76d905fec
Mnemonic: SymlinkTree
Action key: 3f5eff7b31775ca3d96a53c213cba4dcb22aea3b09155835e560871a66c79c31, adf6238ad9f28306822b15250551f05b4ccf4d21e476410e8f59a5627aca3ca4
Progress message: Creating runfiles tree bazel-out/k8-fastbuild/bin/foo/it.runfiles
PrimaryInput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]foo/it.runfiles_manifest
PrimaryOutput: File:[[<execution_root>]bazel-out/k8-fastbuild/bin]foo/it.runfiles/MANIFEST
Owner information: ConfiguredTargetKey{label=//foo:it, config=BuildConfigurationValue.Key[fb3f630c9664150b7a5df27d2d6586a50d90442e5149d0267894e093602b1d77]}, ConfiguredTargetKey{label=//foo:it, config=BuildConfigurationValue.Key[a5e5a51a25dac85941e5a079e772850bacd3ab2905204bf78bdc62c76d905fec]}
MandatoryInputs: are equal
Outputs: are equal
....

:point_up: dozens of these kinds of things. Diffing the respective configs, they're identical other than the following which is present in only one of the configs, and maybe the culprit:

diff <(bazel config 96f337ca1992d337c541dd7d730d4727cf7fe642e811ddb77c51a8069b4f9e34) <(bazel config af0d32aeca223632866b193bd65f1c19bae8377f6d9f0ca2a508907a49227b6d)
INFO: Displaying config with id af0d32aeca223632866b193bd65f1c19bae8377f6d9f0ca2a508907a49227b6d
INFO: Displaying config with id 96f337ca1992d337c541dd7d730d4727cf7fe642e811ddb77c51a8069b4f9e34
1,2c1,2
< BuildConfiguration 96f337ca1992d337c541dd7d730d4727cf7fe642e811ddb77c51a8069b4f9e34:
< Skyframe Key: BuildConfigurationValue.Key[96f337ca1992d337c541dd7d730d4727cf7fe642e811ddb77c51a8069b4f9e34]
---
> BuildConfiguration af0d32aeca223632866b193bd65f1c19bae8377f6d9f0ca2a508907a49227b6d:
> Skyframe Key: BuildConfigurationValue.Key[af0d32aeca223632866b193bd65f1c19bae8377f6d9f0ca2a508907a49227b6d]
77a78,97
> }
> FragmentOptions com.google.devtools.build.lib.analysis.test.TestConfiguration$TestOptions {
>   cache_test_results: AUTO
>   coverage_report_generator: @bazel_tools//tools/test:coverage_report_generator
>   coverage_support: @bazel_tools//tools/test:coverage_support
>   experimental_cancel_concurrent_tests: false
>   experimental_fetch_all_coverage_outputs: false
>   experimental_persistent_test_runner: false
>   experimental_retain_test_configuration_across_testonly: false
>   experimental_split_coverage_postprocessing: false
>   incompatible_exclusive_test_sandboxed: false
>   runs_per_test: [(?:(?>.*)) Options: [1]]
>   runs_per_test_detects_flakes: false
>   test_arg: []
>   test_filter: null
>   test_result_expiration: -1
>   test_runner_fail_fast: false
>   test_sharding_strategy: EXPLICIT
>   test_timeout: {short=PT1M, moderate=PT5M, long=PT15M, eternal=PT1H}
>   trim_test_configuration: true

I'm not sure exactly what to make of it thus far :thinking:

zachgrayio commented 2 years ago

A minor update:

The badly behaving targets here turned out to be integration tests which depended on other integration test targets, in an attempt to make use of shared utility code which should have been factored out to common libraries. Interestingly this seemingly worked in Bazel 4 and earlier, but fails in 5 with the colliding actions.

The simple answer here is that the offending project code should be fixed, so maybe this is a non-issue, but that said, I wonder if it makes sense to look into and/or document the change in behavior at all?

liucijus commented 2 years ago

@zachgrayio in Bazel 5, --trim_test_configuration was changed to true, see if setting it to false helps with your issue

zachgrayio commented 2 years ago

@zachgrayio in Bazel 5, --trim_test_configuration was changed to true, see if setting it to false helps with your issue

thats gotta be it, yeah. was lookin for something like this in the patch notes but failed at grep i guess :laughing:

liucijus commented 2 years ago

If it helped, it probably means that there's bug Rules Scala or Bazel 5

BillyAutrey commented 2 years ago

My team actually hit this exact issue as well. Adding that flag allowed us to proceed with further testing on Bazel 5.

zachgrayio commented 2 years ago

So following up here, yes, that flag alone is enough to avoid the issue here in this build.

That said I'm not convinced this is evidence a bug per se; as I said, the root of the issue is that I discovered tests depending on other tests in this project, and this is what's driving the colliding actions. I assume that after bazel 5 flipped this flag, one of the redundant configs is getting trimmed enough that it ends up conflicting rather than being identical, like it was under bazel 4.

The takeaway here for me is that maybe this shouldn't have ever worked in the first place and the behavior now is probably more sensible.

FWIW, fixing this correctly really wasn't that hard, and for others running into this: if you're hitting this issue it's likely you need to take another look at your build graph and fix the root cause.

lolski commented 2 years ago

Is Bazel 5 officially supported? It's not yet mentioned in the compatibility table.

BillyAutrey commented 1 year ago

That said I'm not convinced this is evidence a bug per se; as I said, the root of the issue is that I discovered tests depending on other tests in this project

This isn't actually the root cause, after I dug into this a bit. I think the root cause is non-tests depending on test libraries. I.e., if you had some targets like this, it would fail:

scala_library(
    name = "mytarget",
    deps = [ ":tests" ],
)

scala_test(
    name = "tests",
)

https://bazel.build/reference/command-line-reference#flag--trim_test_configuration

When this flag is active, tests cannot be built as dependencies of non-test rules