Skip to content

Commit

Permalink
Handle excluded artifacts defined in non-root packages (#190)
Browse files Browse the repository at this point in the history
This handles different sub-packages having different sets of excluded
artifacts, also fixes a crash when such a case occurs.
  • Loading branch information
Ignas authored Jul 24, 2023
1 parent 59d4e1a commit ea89e21
Show file tree
Hide file tree
Showing 16 changed files with 553 additions and 26 deletions.
1 change: 0 additions & 1 deletion java/gazelle/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ func (jc *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
if jc.lang.mavenResolver == nil {
resolver, err := maven.NewResolver(
cfg.MavenInstallFile(),
cfg.ExcludedArtifacts(),
jc.lang.logger,
)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions java/gazelle/javaconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ type Configs map[string]*Config
// NewChild creates a new child Config. It inherits desired values from the
// current Config and sets itself as the parent to the child.
func (c *Config) NewChild() *Config {
clonedExcludedArtifacts := make(map[string]struct{})
for key, value := range c.excludedArtifacts {
clonedExcludedArtifacts[key] = value
}
return &Config{
parent: c,
extensionEnabled: c.extensionEnabled,
Expand All @@ -62,6 +66,7 @@ func (c *Config) NewChild() *Config {
testMode: c.testMode,
customTestFileSuffixes: c.customTestFileSuffixes,
annotationToAttribute: c.annotationToAttribute,
excludedArtifacts: clonedExcludedArtifacts,
}
}

Expand Down
25 changes: 15 additions & 10 deletions java/gazelle/private/maven/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

type Resolver interface {
Resolve(pkg types.PackageName) (label.Label, error)
Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error)
}

// resolver finds Maven provided packages by reading the maven_install.json
Expand All @@ -22,7 +22,7 @@ type resolver struct {
logger zerolog.Logger
}

func NewResolver(installFile string, excludedArtifacts map[string]struct{}, logger zerolog.Logger) (Resolver, error) {
func NewResolver(installFile string, logger zerolog.Logger) (Resolver, error) {
r := resolver{
data: multiset.NewStringMultiSet(),
logger: logger.With().Str("_c", "maven-resolver").Logger(),
Expand All @@ -43,10 +43,7 @@ func NewResolver(installFile string, excludedArtifacts map[string]struct{}, logg
if err != nil {
return nil, fmt.Errorf("failed to parse coordinate %v: %w", coords, err)
}
l := label.New("maven", "", bazel.CleanupLabel(coords.ArtifactString()))
if _, found := excludedArtifacts[l.String()]; found {
continue
}
l := LabelFromArtifact(coords.ArtifactString())
for _, pkg := range c.ListDependencyPackages(depName) {
r.data.Add(pkg, l.String())
}
Expand All @@ -55,27 +52,35 @@ func NewResolver(installFile string, excludedArtifacts map[string]struct{}, logg
return &r, nil
}

func (r *resolver) Resolve(pkg types.PackageName) (label.Label, error) {
func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) {
v, found := r.data.Get(pkg.Name)
if !found {
return label.NoLabel, fmt.Errorf("package not found: %s", pkg)
}

switch len(v) {
var filtered []string
for k := range v {
if _, excluded := excludedArtifacts[k]; excluded {
continue
}
filtered = append(filtered, k)
}

switch len(filtered) {
case 0:
return label.NoLabel, errors.New("no external imports")

case 1:
var ret string
for r := range v {
for _, r := range filtered {
ret = r
break
}
return label.Parse(ret)

default:
r.logger.Error().Msg("Append one of the following to BUILD.bazel:")
for k := range v {
for _, k := range filtered {
r.logger.Error().Msgf("# gazelle:resolve java %s %s", pkg, k)
}

Expand Down
14 changes: 7 additions & 7 deletions java/gazelle/private/maven/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,26 @@ func TestResolver(t *testing.T) {

m := make(map[string]struct{})
m["@maven//:com_google_j2objc_j2objc_annotations"] = struct{}{}
r, err := NewResolver("testdata/guava_maven_install.json", m, logger)
r, err := NewResolver("testdata/guava_maven_install.json", logger)
if err != nil {
t.Fatal(err)
}

assertResolves(t, r, "com.google.common.collect", "@maven//:com_google_guava_guava")
assertResolves(t, r, "javax.annotation", "@maven//:com_google_code_findbugs_jsr305")
got, err := r.Resolve(types.NewPackageName("unknown.package"))
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)
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"))
got, err = r.Resolve(types.NewPackageName("com.google.j2objc.annotations"), m)
if err == nil {
t.Errorf("Want error finding label for excluded artifact, got %v", got)
}

}

func assertResolves(t *testing.T, r Resolver, pkg, wantLabelStr string) {
got, err := r.Resolve(types.NewPackageName(pkg))
func assertResolves(t *testing.T, r Resolver, excludePackages map[string]struct{}, pkg, wantLabelStr string) {
got, err := r.Resolve(types.NewPackageName(pkg), excludePackages)
if err != nil {
t.Errorf("Error finding label for %v: %v", pkg, err)
}
Expand Down
17 changes: 11 additions & 6 deletions java/gazelle/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sort"

"github.com/bazel-contrib/rules_jvm/java/gazelle/javaconfig"
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/java"
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
Expand Down Expand Up @@ -81,13 +82,17 @@ func (Resolver) Embeds(r *rule.Rule, from label.Label) []label.Label {
func (jr Resolver) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, r *rule.Rule, imports interface{}, from label.Label) {
resolveInput := imports.(types.ResolveInput)

packageConfig := c.Exts[languageName].(javaconfig.Configs)[from.Pkg]
if packageConfig == nil {
jr.lang.logger.Fatal().Msg("failed retrieving package config")
}
isTestRule := isTestRule(r.Kind())

jr.populateAttr(c, r, "deps", resolveInput.ImportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames)
jr.populateAttr(c, r, "exports", resolveInput.ExportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames)
jr.populateAttr(c, packageConfig, r, "deps", resolveInput.ImportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames)
jr.populateAttr(c, packageConfig, r, "exports", resolveInput.ExportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames)
}

func (jr Resolver) populateAttr(c *config.Config, r *rule.Rule, attrName string, requiredPackageNames *sorted_set.SortedSet[types.PackageName], ix *resolve.RuleIndex, isTestRule bool, from label.Label, ownPackageNames *sorted_set.SortedSet[types.PackageName]) {
func (jr Resolver) populateAttr(c *config.Config, pc *javaconfig.Config, r *rule.Rule, attrName string, requiredPackageNames *sorted_set.SortedSet[types.PackageName], ix *resolve.RuleIndex, isTestRule bool, from label.Label, ownPackageNames *sorted_set.SortedSet[types.PackageName]) {
labels := sorted_set.NewSortedSetFn[label.Label]([]label.Label{}, labelLess)

for _, implicitDep := range r.AttrStrings(attrName) {
Expand All @@ -99,7 +104,7 @@ func (jr Resolver) populateAttr(c *config.Config, r *rule.Rule, attrName string,
}

for _, imp := range requiredPackageNames.SortedSlice() {
dep, err := jr.resolveSinglePackage(c, imp, ix, from, isTestRule, ownPackageNames)
dep, err := jr.resolveSinglePackage(c, pc, imp, ix, from, isTestRule, ownPackageNames)
if err != nil {
jr.lang.logger.Error().Str("import", dep.String()).Err(err).Msg("error converting import")
panic(fmt.Sprintf("error converting import: %s", err))
Expand Down Expand Up @@ -151,7 +156,7 @@ func simplifyLabel(repoName string, l label.Label, from label.Label) label.Label
return l
}

func (jr *Resolver) resolveSinglePackage(c *config.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName]) (out label.Label, err error) {
func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName]) (out label.Label, err error) {
cacheKey := types.NewResolvableJavaPackage(imp, false, false)
importSpec := resolve.ImportSpec{Lang: languageName, Imp: cacheKey.String()}
if ol, found := resolve.FindRuleWithOverride(c, importSpec, languageName); found {
Expand Down Expand Up @@ -192,7 +197,7 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, imp types.PackageName
return label.NoLabel, nil
}

if l, err := jr.lang.mavenResolver.Resolve(imp); err == nil {
if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts()); err == nil {
return l, nil
}

Expand Down
4 changes: 2 additions & 2 deletions java/gazelle/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func testConfig(t *testing.T, args ...string) (*config.Config, []language.Langua

type testResolver struct{}

func (*testResolver) Resolve(pkg types.PackageName) (label.Label, error) {
func (*testResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) {
return label.NoLabel, errors.New("not implemented")
}

Expand Down Expand Up @@ -280,7 +280,7 @@ func NewTestMavenResolver() *TestMavenResolver {
}
}

func (r *TestMavenResolver) Resolve(pkg types.PackageName) (label.Label, error) {
func (r *TestMavenResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}) (label.Label, error) {
l, found := r.data[pkg]
if !found {
return label.NoLabel, fmt.Errorf("unexpected import: %s", pkg)
Expand Down
Empty file.
Empty file.
27 changes: 27 additions & 0 deletions java/gazelle/testdata/java_exclude_artifact/WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
name = "rules_jvm_external",
sha256 = "23fe83890a77ac1a3ee143e2306ec12da4a845285b14ea13cb0df1b1e23658fe",
strip_prefix = "rules_jvm_external-4.3",
urls = ["https://github.com/bazelbuild/rules_jvm_external/archive/refs/tags/4.3.tar.gz"],
)

load("@rules_jvm_external//:defs.bzl", "maven_install")

maven_install(
artifacts = [
"com.google.guava:guava:30.0-jre",
"junit:junit:4.13.1",
"org.hamcrest:hamcrest-all:1.3",
],
fetch_sources = True,
maven_install_json = "//:maven_install.json",
repositories = [
"https://repo1.maven.org/maven2",
],
)

load("@maven//:defs.bzl", "pinned_maven_install")

pinned_maven_install()
Loading

0 comments on commit ea89e21

Please sign in to comment.