From b18c44ec3e0f996443db5a319edd49535f89d1c9 Mon Sep 17 00:00:00 2001 From: hvadehra Date: Fri, 18 Aug 2023 22:45:39 +0200 Subject: [PATCH] Update java templates in rbeconfigsgen.go (#999) * Update java templates in rbeconfigsgen.go Use @rules_java for bazel > 7.0.0 * Fix buildifier CI failures --- pkg/rbeconfigsgen/rbeconfigsgen.go | 60 ++++++++++--- pkg/rbeconfigsgen/rbeconfigsgen_test.go | 100 ++++++++++++++++++++++ rules/exec_properties/exec_properties.bzl | 4 +- 3 files changed, 149 insertions(+), 15 deletions(-) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 25c29c106..a5210e9f6 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -99,12 +99,33 @@ java_runtime( srcs = [], java_home = "{{ .JavaHome }}", ) +`)) + + // javaBuildTemplateLt7 is the Java toolchain config BUILD file template for Bazel versions + // >=5.0.0 (tentative?) and < 7.0.0. + javaBuildTemplateLt7 = template.Must(template.New("javaBuild").Parse(buildHeader + ` +load("@bazel_tools//tools/jdk:local_java_repository.bzl", "local_java_runtime") + +package(default_visibility = ["//visibility:public"]) + +alias( + name = "jdk", + actual = "rbe_jdk", +) + +local_java_runtime( + name = "rbe_jdk", + java_home = "{{ .JavaHome }}", + version = "{{ .JavaVersion }}", +) `)) // javaBuildTemplate is the Java toolchain config BUILD file template for Bazel versions - // >=5.0.0 (tentative?). + // >=7.0.0 (including pre-releases). + // The difference between the older template is directly referencing to @rules_java + // instead of the indirection via @bazel_tools javaBuildTemplate = template.Must(template.New("javaBuild").Parse(buildHeader + ` -load("@bazel_tools//tools/jdk:local_java_repository.bzl", "local_java_runtime") +load("@rules_java//toolchains:local_java_repository.bzl", "local_java_runtime") package(default_visibility = ["//visibility:public"]) @@ -562,6 +583,25 @@ func UsesLocalJavaRuntime(bazelVersion string) (bool, error) { return !bv.LessThan(*semver.New("5.0.0")), nil } +func getJavaTemplate(o *Options) (*template.Template, error) { + usesNewJavaRule := o.JavaUseLocalRuntime + if !usesNewJavaRule { + var err error + usesNewJavaRule, err = UsesLocalJavaRuntime(o.BazelVersion) + if (err != nil) { + return nil, fmt.Errorf("unable to determine what Java toolchain rule to use for Bazel %q: %w", o.BazelVersion, err) + } + } + if !usesNewJavaRule { + return legacyJavaBuildTemplate, nil + } + // use latest template if BazelVersion is unspecified + if o.BazelVersion != "" && o.BazelVersion < "7" { + return javaBuildTemplateLt7, nil + } + return javaBuildTemplate, nil +} + // genJavaConfigs returns a BUILD file containing a Java toolchain rule definition that contains // the following attributes determined by probing details about the JDK version installed in the // running toolchain container. @@ -613,17 +653,11 @@ func genJavaConfigs(d *dockerRunner, o *Options) (generatedFile, error) { } log.Printf("Java version: '%s'.", javaVersion) - usesNewJavaRule := o.JavaUseLocalRuntime - if !usesNewJavaRule { - usesNewJavaRule, err = UsesLocalJavaRuntime(o.BazelVersion) - if err != nil { - return generatedFile{}, fmt.Errorf("unable to determine what Java toolchain rule to use for Bazel %q: %w", o.BazelVersion, err) - } - } - t := legacyJavaBuildTemplate - if usesNewJavaRule { - t = javaBuildTemplate - } + t, err := getJavaTemplate(o) + if err != nil { + return generatedFile{}, err + } + buf := bytes.NewBuffer(nil) if err := t.Execute(buf, &javaBuildTemplateParams{ JavaHome: javaHome, diff --git a/pkg/rbeconfigsgen/rbeconfigsgen_test.go b/pkg/rbeconfigsgen/rbeconfigsgen_test.go index 62a6b1119..eac84669d 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen_test.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen_test.go @@ -16,6 +16,7 @@ package rbeconfigsgen import ( "testing" + "text/template" ) func TestGenCppToolchainTarget(t *testing.T) { @@ -84,3 +85,102 @@ func TestGenCppToolchainTarget(t *testing.T) { }) } } + +func TestGetJavaTemplate(t *testing.T) { + tests := []struct { + name string + want *template.Template + opt *Options + }{ + { + name: "bazel 4, choose legacy", + want: legacyJavaBuildTemplate, + opt: &Options{ + BazelVersion: "4.0.0", + }, + }, + { + name: "bazel 5, choose BazelLt7", + want: javaBuildTemplateLt7, + opt: &Options{ + BazelVersion: "5.0.0", + }, + }, + { + name: "bazel 7, choose latest", + want: javaBuildTemplate, + opt: &Options{ + BazelVersion: "7.0.0", + }, + }, + { + name: "bazel 7-pre, choose latest", + want: javaBuildTemplate, + opt: &Options{ + BazelVersion: "7.0.0-pre.20230724.1", + }, + }, + { + name: "useLocalRuntime forced, choose latest", + want: javaBuildTemplate, + opt: &Options{ + JavaUseLocalRuntime: true, + }, + }, + { + name: "useLocalRuntime forced, bazel 4, choose BazelLt7", + want: javaBuildTemplateLt7, + opt: &Options{ + BazelVersion: "4.0.0", + JavaUseLocalRuntime: true, + }, + }, + { + name: "useLocalRuntime forced, bazel 5, choose BazelLt7", + want: javaBuildTemplateLt7, + opt: &Options{ + BazelVersion: "5.0.0", + JavaUseLocalRuntime: true, + }, + }, + { + name: "useLocalRuntime forced, bazel 6, choose BazelLt7", + want: javaBuildTemplateLt7, + opt: &Options{ + BazelVersion: "6.0.0", + JavaUseLocalRuntime: true, + }, + }, + { + name: "useLocalRuntime forced, bazel 7, choose latest", + want: javaBuildTemplate, + opt: &Options{ + BazelVersion: "7.0.0", + JavaUseLocalRuntime: true, + }, + }, + { + name: "useLocalRuntime forced, bazel 7-pre, choose latest", + want: javaBuildTemplate, + opt: &Options{ + BazelVersion: "7.0.0-pre.20200202", + JavaUseLocalRuntime: true, + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + // We skip validation since we don't set all options required for + // regular execution. + got, err := getJavaTemplate(tc.opt); + if err != nil { + t.Fatalf("getJavaTemplate failed: %v, wanted: %v", err, tc.want) + } else if got != tc.want { + t.Fatalf("getJavaTemplate: %v, wanted %v", got, tc.want) + } + }) + } +} diff --git a/rules/exec_properties/exec_properties.bzl b/rules/exec_properties/exec_properties.bzl index 6391302f5..f9de58027 100644 --- a/rules/exec_properties/exec_properties.bzl +++ b/rules/exec_properties/exec_properties.bzl @@ -203,7 +203,7 @@ PARAMS = { ), } -def create_exec_properties_dict(**kwargs): +def create_exec_properties_dict(**_kwargs): fail("create_exec_properties_dict is deprecated. Please use create_rbe_exec_properties_dict instead.") def create_rbe_exec_properties_dict(**kwargs): @@ -242,7 +242,7 @@ def create_rbe_exec_properties_dict(**kwargs): return dict -def merge_dicts(*dict_args): +def merge_dicts(*_dict_args): fail("merge_dicts is deprecated. Please use dicts.add() instead. See https://github.com/bazelbuild/bazel-skylib/blob/master/docs/dicts_doc.md") def _exec_property_sets_repository_impl(repository_ctx):