From e46bb11bedb2ead45309eae04619caca684f6243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignas=20Mikalaj=C5=ABnas?= Date: Wed, 26 Jul 2023 12:23:05 +0300 Subject: [PATCH] Added java_maven_repository_name directive (#192) Some repositories can run maven_install with a different name or maintain multiple sets of maven dependencies, this change will allow overriding `@maven` with whatever custom name that is needed. --- java/gazelle/configure.go | 4 ++++ java/gazelle/generate.go | 15 ++++++++------- java/gazelle/generate_test.go | 4 ++-- java/gazelle/javaconfig/config.go | 15 +++++++++++++++ java/gazelle/private/maven/resolver.go | 15 +++++++-------- java/gazelle/private/maven/resolver_test.go | 6 +++--- java/gazelle/resolve.go | 2 +- java/gazelle/resolve_test.go | 4 ++-- java/gazelle/testdata/maven/BUILD.in | 1 + java/gazelle/testdata/maven/BUILD.out | 1 + .../src/main/java/com/example/compare/BUILD.out | 2 +- .../src/main/java/com/example/myproject/BUILD.out | 2 +- .../src/test/java/com/example/myproject/BUILD.out | 2 +- 13 files changed, 47 insertions(+), 26 deletions(-) diff --git a/java/gazelle/configure.go b/java/gazelle/configure.go index 9f60b9fc..3d6e321c 100644 --- a/java/gazelle/configure.go +++ b/java/gazelle/configure.go @@ -57,6 +57,7 @@ func (jc *Configurer) KnownDirectives() []string { javaconfig.JavaTestFileSuffixes, javaconfig.JavaTestMode, javaconfig.JavaGenerateProto, + javaconfig.JavaMavenRepositoryName, } } @@ -109,6 +110,9 @@ func (jc *Configurer) Configure(c *config.Config, rel string, f *rule.File) { case javaconfig.JavaTestMode: cfg.SetTestMode(d.Value) + case javaconfig.JavaMavenRepositoryName: + cfg.SetMavenRepositoryName(d.Value) + case javaconfig.JavaGenerateProto: switch d.Value { case "true": diff --git a/java/gazelle/generate.go b/java/gazelle/generate.go index cea42db0..31d6a388 100644 --- a/java/gazelle/generate.go +++ b/java/gazelle/generate.go @@ -216,7 +216,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes case "file": for _, tf := range testJavaFiles.SortedSlice() { extraAttributes := separateTestJavaFiles[tf] - l.generateJavaTest(args.File, args.Rel, tf, isModule, testJavaImportsWithHelpers, extraAttributes, &res) + l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), tf, isModule, testJavaImportsWithHelpers, extraAttributes, &res) } case "suite": @@ -242,6 +242,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes suiteName, srcs, packageNames, + cfg.MavenRepositoryName(), testJavaImportsWithHelpers, cfg.GetCustomJavaTestFileSuffixes(), testHelperJavaFiles.Len() > 0, @@ -254,7 +255,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes sortedSeparateTestJavaFiles.Add(src) } for _, src := range sortedSeparateTestJavaFiles.SortedSlice() { - l.generateJavaTest(args.File, args.Rel, src, isModule, testJavaImportsWithHelpers, separateTestJavaFiles[src], &res) + l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), src, isModule, testJavaImportsWithHelpers, separateTestJavaFiles[src], &res) } } } @@ -459,7 +460,7 @@ func (l javaLang) generateJavaBinary(file *rule.File, m types.ClassName, libName }) } -func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazelWorkspace string, f javaFile, includePackageInName bool, imports *sorted_set.SortedSet[types.PackageName], extraAttributes map[string]bzl.Expr, res *language.GenerateResult) { +func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazelWorkspace string, mavenRepositoryName string, f javaFile, includePackageInName bool, imports *sorted_set.SortedSet[types.PackageName], extraAttributes map[string]bzl.Expr, res *language.GenerateResult) { className := f.ClassName() fullyQualifiedTestClass := className.FullyQualifiedClassName() var testName string @@ -481,7 +482,7 @@ func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazel // up the resolver to do this. We probably should. // In the mean time, hard-code some labels. for _, artifact := range junit5RuntimeDeps { - runtimeDeps.Add(maven.LabelFromArtifact(artifact)) + runtimeDeps.Add(maven.LabelFromArtifact(mavenRepositoryName, artifact)) } } @@ -533,7 +534,7 @@ var junit5RuntimeDeps = []string{ "org.junit.platform:junit-platform-reporting", } -func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []string, packageNames, imports *sorted_set.SortedSet[types.PackageName], customTestSuffixes *[]string, hasHelpers bool, res *language.GenerateResult) { +func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []string, packageNames *sorted_set.SortedSet[types.PackageName], mavenRepositoryName string, imports *sorted_set.SortedSet[types.PackageName], customTestSuffixes *[]string, hasHelpers bool, res *language.GenerateResult) { const ruleKind = "java_test_suite" r := rule.NewRule(ruleKind, name) r.SetAttr("srcs", srcs) @@ -549,10 +550,10 @@ func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []str if importsJunit5(imports) { r.SetAttr("runner", "junit5") for _, artifact := range junit5RuntimeDeps { - runtimeDeps.Add(maven.LabelFromArtifact(artifact)) + runtimeDeps.Add(maven.LabelFromArtifact(mavenRepositoryName, artifact)) } if importsJunit4(imports) { - runtimeDeps.Add(maven.LabelFromArtifact("org.junit.vintage:junit-vintage-engine")) + runtimeDeps.Add(maven.LabelFromArtifact(mavenRepositoryName, "org.junit.vintage:junit-vintage-engine")) } // This should probably register imports here, and then allow the resolver to resolve this to an artifact, // but we don't currently wire up the resolver to do this. diff --git a/java/gazelle/generate_test.go b/java/gazelle/generate_test.go index 3809298a..d14e76d4 100644 --- a/java/gazelle/generate_test.go +++ b/java/gazelle/generate_test.go @@ -135,7 +135,7 @@ func TestSingleJavaTestFile(t *testing.T) { var res language.GenerateResult l := newTestJavaLang(t) - l.generateJavaTest(nil, "", f, tc.includePackageInName, stringsToPackageNames(tc.importedPackages), nil, &res) + l.generateJavaTest(nil, "", "maven", f, tc.includePackageInName, stringsToPackageNames(tc.importedPackages), nil, &res) require.Len(t, res.Gen, 1, "want 1 generated rule") @@ -227,7 +227,7 @@ func TestSuite(t *testing.T) { var res language.GenerateResult l := newTestJavaLang(t) - l.generateJavaTestSuite(nil, "blah", []string{src}, stringsToPackageNames([]string{pkg}), stringsToPackageNames(tc.importedPackages), nil, false, &res) + l.generateJavaTestSuite(nil, "blah", []string{src}, stringsToPackageNames([]string{pkg}), "maven", stringsToPackageNames(tc.importedPackages), nil, false, &res) require.Len(t, res.Gen, 1, "want 1 generated rule") diff --git a/java/gazelle/javaconfig/config.go b/java/gazelle/javaconfig/config.go index 4d4e8ad8..588987ce 100644 --- a/java/gazelle/javaconfig/config.go +++ b/java/gazelle/javaconfig/config.go @@ -42,6 +42,10 @@ const ( // rules when a `proto_library` rule is present. // Can be either "true" or "false". Defaults to "true". JavaGenerateProto = "java_generate_proto" + + // JavaMavenRepositoryName tells the code generator what the repository name that contains all maven dependencies is. + // Defaults to "maven" + JavaMavenRepositoryName = "java_maven_repository_name" ) // Configs is an extension of map[string]*Config. It provides finding methods @@ -67,6 +71,7 @@ func (c *Config) NewChild() *Config { customTestFileSuffixes: c.customTestFileSuffixes, annotationToAttribute: c.annotationToAttribute, excludedArtifacts: clonedExcludedArtifacts, + mavenRepositoryName: c.mavenRepositoryName, } } @@ -94,6 +99,7 @@ type Config struct { customTestFileSuffixes *[]string excludedArtifacts map[string]struct{} annotationToAttribute map[string]map[string]bzl.Expr + mavenRepositoryName string } type LoadInfo struct { @@ -114,6 +120,7 @@ func New(repoRoot string) *Config { customTestFileSuffixes: nil, excludedArtifacts: make(map[string]struct{}), annotationToAttribute: make(map[string]map[string]bzl.Expr), + mavenRepositoryName: "maven", } } @@ -139,6 +146,14 @@ func (c *Config) SetGenerateProto(generate bool) { c.generateProto = generate } +func (c *Config) MavenRepositoryName() string { + return c.mavenRepositoryName +} + +func (c *Config) SetMavenRepositoryName(name string) { + c.mavenRepositoryName = name +} + func (c Config) MavenInstallFile() string { return filepath.Join(c.repoRoot, c.mavenInstallFile) } diff --git a/java/gazelle/private/maven/resolver.go b/java/gazelle/private/maven/resolver.go index a600ea92..54b7e75b 100644 --- a/java/gazelle/private/maven/resolver.go +++ b/java/gazelle/private/maven/resolver.go @@ -12,7 +12,7 @@ import ( ) type Resolver interface { - Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) + Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) } // resolver finds Maven provided packages by reading the maven_install.json @@ -43,16 +43,15 @@ func NewResolver(installFile string, logger zerolog.Logger) (Resolver, error) { if err != nil { return nil, fmt.Errorf("failed to parse coordinate %v: %w", coords, err) } - l := LabelFromArtifact(coords.ArtifactString()) for _, pkg := range c.ListDependencyPackages(depName) { - r.data.Add(pkg, l.String()) + r.data.Add(pkg, coords.ArtifactString()) } } return &r, nil } -func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) { +func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { v, found := r.data.Get(pkg.Name) if !found { return label.NoLabel, fmt.Errorf("package not found: %s", pkg) @@ -60,10 +59,10 @@ func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]s var filtered []string for k := range v { - if _, excluded := excludedArtifacts[k]; excluded { + if _, excluded := excludedArtifacts[LabelFromArtifact(mavenRepositoryName, k).String()]; excluded { continue } - filtered = append(filtered, k) + filtered = append(filtered, LabelFromArtifact(mavenRepositoryName, k).String()) } switch len(filtered) { @@ -88,6 +87,6 @@ func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]s } } -func LabelFromArtifact(artifact string) label.Label { - return label.New("maven", "", bazel.CleanupLabel(artifact)) +func LabelFromArtifact(mavenRepositoryName string, artifact string) label.Label { + return label.New(mavenRepositoryName, "", bazel.CleanupLabel(artifact)) } diff --git a/java/gazelle/private/maven/resolver_test.go b/java/gazelle/private/maven/resolver_test.go index cd02249d..83f99603 100644 --- a/java/gazelle/private/maven/resolver_test.go +++ b/java/gazelle/private/maven/resolver_test.go @@ -25,11 +25,11 @@ func TestResolver(t *testing.T) { assertResolves(t, r, m, "com.google.common.collect", "@maven//:com_google_guava_guava") assertResolves(t, r, m, "javax.annotation", "@maven//:com_google_code_findbugs_jsr305") - got, err := r.Resolve(types.NewPackageName("unknown.package"), m) + got, err := r.Resolve(types.NewPackageName("unknown.package"), m, "maven") if err == nil { t.Errorf("Want error finding label for unknown.package, got %v", got) } - got, err = r.Resolve(types.NewPackageName("com.google.j2objc.annotations"), m) + got, err = r.Resolve(types.NewPackageName("com.google.j2objc.annotations"), m, "maven") if err == nil { t.Errorf("Want error finding label for excluded artifact, got %v", got) } @@ -37,7 +37,7 @@ func TestResolver(t *testing.T) { } func assertResolves(t *testing.T, r Resolver, excludePackages map[string]struct{}, pkg, wantLabelStr string) { - got, err := r.Resolve(types.NewPackageName(pkg), excludePackages) + got, err := r.Resolve(types.NewPackageName(pkg), excludePackages, "maven") if err != nil { t.Errorf("Error finding label for %v: %v", pkg, err) } diff --git a/java/gazelle/resolve.go b/java/gazelle/resolve.go index 4eced15b..534cebae 100644 --- a/java/gazelle/resolve.go +++ b/java/gazelle/resolve.go @@ -197,7 +197,7 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config return label.NoLabel, nil } - if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts()); err == nil { + if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts(), pc.MavenRepositoryName()); err == nil { return l, nil } diff --git a/java/gazelle/resolve_test.go b/java/gazelle/resolve_test.go index a6ae218f..54761fd6 100644 --- a/java/gazelle/resolve_test.go +++ b/java/gazelle/resolve_test.go @@ -240,7 +240,7 @@ func testConfig(t *testing.T, args ...string) (*config.Config, []language.Langua type testResolver struct{} -func (*testResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) { +func (*testResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { return label.NoLabel, errors.New("not implemented") } @@ -280,7 +280,7 @@ func NewTestMavenResolver() *TestMavenResolver { } } -func (r *TestMavenResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) { +func (r *TestMavenResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { l, found := r.data[pkg] if !found { return label.NoLabel, fmt.Errorf("unexpected import: %s", pkg) diff --git a/java/gazelle/testdata/maven/BUILD.in b/java/gazelle/testdata/maven/BUILD.in index e69de29b..6bd050ce 100644 --- a/java/gazelle/testdata/maven/BUILD.in +++ b/java/gazelle/testdata/maven/BUILD.in @@ -0,0 +1 @@ +# gazelle:java_maven_repository_name vendor_java diff --git a/java/gazelle/testdata/maven/BUILD.out b/java/gazelle/testdata/maven/BUILD.out index e69de29b..6bd050ce 100644 --- a/java/gazelle/testdata/maven/BUILD.out +++ b/java/gazelle/testdata/maven/BUILD.out @@ -0,0 +1 @@ +# gazelle:java_maven_repository_name vendor_java diff --git a/java/gazelle/testdata/maven/src/main/java/com/example/compare/BUILD.out b/java/gazelle/testdata/maven/src/main/java/com/example/compare/BUILD.out index df9e88f6..1616114b 100644 --- a/java/gazelle/testdata/maven/src/main/java/com/example/compare/BUILD.out +++ b/java/gazelle/testdata/maven/src/main/java/com/example/compare/BUILD.out @@ -4,7 +4,7 @@ java_library( name = "compare", srcs = ["Compare.java"], visibility = ["//:__subpackages__"], - deps = ["@maven//:com_google_guava_guava"], + deps = ["@vendor_java//:com_google_guava_guava"], ) java_binary( diff --git a/java/gazelle/testdata/maven/src/main/java/com/example/myproject/BUILD.out b/java/gazelle/testdata/maven/src/main/java/com/example/myproject/BUILD.out index 0e5342b5..486fb2f2 100644 --- a/java/gazelle/testdata/maven/src/main/java/com/example/myproject/BUILD.out +++ b/java/gazelle/testdata/maven/src/main/java/com/example/myproject/BUILD.out @@ -4,7 +4,7 @@ java_library( name = "myproject", srcs = ["App.java"], visibility = ["//:__subpackages__"], - deps = ["@maven//:com_google_guava_guava"], + deps = ["@vendor_java//:com_google_guava_guava"], ) java_binary( diff --git a/java/gazelle/testdata/maven/src/test/java/com/example/myproject/BUILD.out b/java/gazelle/testdata/maven/src/test/java/com/example/myproject/BUILD.out index ca27b88b..8ae681f8 100644 --- a/java/gazelle/testdata/maven/src/test/java/com/example/myproject/BUILD.out +++ b/java/gazelle/testdata/maven/src/test/java/com/example/myproject/BUILD.out @@ -5,6 +5,6 @@ java_test_suite( srcs = ["AppTest.java"], deps = [ "//src/main/java/com/example/myproject", - "@maven//:junit_junit", + "@vendor_java//:junit_junit", ], )