bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Improve error for mismatched local_jdk version in tests

Open keith opened this issue 1 year ago • 1 comments

Description of the bug:

When working on bazel itself, some shell tests like //src/test/tools/bzlmod:verify_default_lock_file depend on the local_jdk through this script:

https://github.com/bazelbuild/bazel/blob/31fae9e8e6687cbaf0dfe55a466696210c80be96/src/test/shell/testenv.sh.tmpl#L95

In the case your installed JDK mismatches what bazel expects, the tests fail with this error:

Executing tests from //src/test/tools/bzlmod:verify_default_lock_file
-----------------------------------------------------------------------------
usage: dirname string [...]
usage: dirname string [...]

Since you end up trying to run dirname on an empty string. I discovered this was the JDK version mismatch with this log from --toolchain_resolution_debug='.*':

ToolchainResolution:   Rejected toolchain @@rules_java~7.4.0~toolchains~local_jdk//:jdk; mismatching config settings: local_jdk_name_version_setting

Which category does this issue belong to?

No response

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

On macOS install azul 21 so that this is your only java install:

% /usr/libexec/java_home -V
Matching Java Virtual Machines (1):
    21.0.2 (arm64) "Azul Systems, Inc." - "Zulu 21.32.17" /Library/Java/JavaVirtualMachines/zulu-21.jdk/Contents/Home
/Library/Java/JavaVirtualMachines/zulu-21.jdk/Contents/Home

Run: bazel test src/test/tools/bzlmod:verify_default_lock_file --test_output=all

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

7.0.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

You can work around this by passing --java_runtime_version=21, based on the docs for that flag the default is supposed to be local_jdk, which I would assume means it should respect this version without me having to pass that flag. At first I assumed that was because this setup:

https://github.com/bazelbuild/bazel/blob/31fae9e8e6687cbaf0dfe55a466696210c80be96/tools/jdk/BUILD.tools#L178-L181

Doesn't include 21 but changing that locally doesn't seem to affect this behavior.

keith avatar Feb 16 '24 22:02 keith

I submitted https://github.com/bazelbuild/bazel/pull/21392 which improves the error but thinking about this more I guess really it seems like this issue should be eliminated a different way

keith avatar Feb 16 '24 22:02 keith

#21392 looks fine to me as an improvement, but $(rlocation local_jdk/bin/java))) being empty is more concerning. I'm going to take a stab at trying to repro.

hvadehra avatar Feb 21 '24 16:02 hvadehra

Let me know if my steps don't work and I can try to debug further

keith avatar Feb 21 '24 18:02 keith

I tried your steps in a pristine container and everything works fine.

Given that --java_runtime_version=21 makes things work for you, does the issue also go away if you explicitly set --java_runtime_version=local_jdk?

My best guess is the runtime version is getting set to something that is neither 21 nor local_jdk. Is it possible you have this set in ~/.bazelrc or somewhere else (you could try building with --announce_rc to check)?

hvadehra avatar Feb 22 '24 11:02 hvadehra

My best guess is the runtime version is getting set to something that is neither 21 nor local_jdk. Is it possible you have this set in ~/.bazelrc or somewhere else (you could try building with --announce_rc to check)?

🤦🏻 you're totally right, I had an old user.bazelrc that was passing --java_runtime_version=remotejdk_11, sorry about that! I'm totally not used to having an ignored rc file like that. I guess there's a minor question of if that should still work for these tests, but that doesn't seem nearly as important

keith avatar Feb 22 '24 17:02 keith

Thanks for confirming.

I've submitted 77c2791212ae972661231ae696d5e02a669839f0 to at least localize the reliance on local_jdk to the tests that actually make use of it. I think your change could be still be valuable, so it would be great if you could rebase on master and incorporate it into the new function.

hvadehra avatar Feb 23 '24 13:02 hvadehra

thanks, updated my PR. I think this is fine to close afterwards

keith avatar Feb 23 '24 19:02 keith