Skip to content

Commit

Permalink
gazelle: Fail when errors occur (#202)
Browse files Browse the repository at this point in the history
Intead of introducing a fail-fast, leverage the lifecycle to fail the
extension when errors that would produce invalid build files occur.
  • Loading branch information
stevebarrau authored Sep 4, 2023
1 parent 15f21cc commit 8ff3022
Show file tree
Hide file tree
Showing 21 changed files with 356 additions and 10 deletions.
16 changes: 15 additions & 1 deletion java/gazelle/lang.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gazelle

import (
"context"
"os"

"github.com/bazel-contrib/rules_jvm/java/gazelle/private/java"
Expand All @@ -17,6 +18,7 @@ import (
// javaLang is a language.Language implementation for Java.
type javaLang struct {
config.Configurer
language.BaseLifecycleManager
resolve.Resolver

parser *javaparser.Runner
Expand All @@ -27,6 +29,12 @@ type javaLang struct {
// javaPackageCache is used for module granularity support
// Key is the path to the java package from the Bazel workspace root.
javaPackageCache map[string]*java.Package

// hasHadErrors triggers the extension to fail at destroy time.
//
// this is used to return != 0 when some errors during the generation were
// raised that will create invalid build files.
hasHadErrors bool
}

func NewLanguage() language.Language {
Expand All @@ -38,7 +46,7 @@ func NewLanguage() language.Language {
Caller().
Logger().
Level(goLevel)
logger.Print("creating java language")
logger.Debug().Msg("creating java language")

l := javaLang{
logger: logger,
Expand Down Expand Up @@ -138,3 +146,9 @@ func (l javaLang) Fix(c *config.Config, f *rule.File) {}
func (l javaLang) DoneGeneratingRules() {
l.parser.ServerManager().Shutdown()
}

func (l javaLang) AfterResolvingDeps(_ context.Context) {
if l.hasHadErrors {
l.logger.Fatal().Msg("the java extension encontered errors that will create invalid build files")
}
}
32 changes: 24 additions & 8 deletions java/gazelle/private/maven/resolver.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package maven

import (
"errors"
"fmt"

"github.com/bazel-contrib/rules_jvm/java/gazelle/private/bazel"
Expand All @@ -11,6 +10,25 @@ import (
"github.com/rs/zerolog"
)

// NoExternalImportsError represents the error when no external imports are found.
type NoExternalImportsError struct {
PackageName string
}

func (e *NoExternalImportsError) Error() string {
return fmt.Sprintf("no external imports found for %s", e.PackageName)
}

// MultipleExternalImportsError represents the error when multiple possible external imports are found.
type MultipleExternalImportsError struct {
PackageName string
PossiblePackages []string
}

func (e *MultipleExternalImportsError) Error() string {
return fmt.Sprintf("multiple external imports found for %s; %v", e.PackageName, e.PossiblePackages)
}

type Resolver interface {
Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error)
}
Expand Down Expand Up @@ -54,7 +72,7 @@ func NewResolver(installFile string, logger zerolog.Logger) (Resolver, 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)
return label.NoLabel, &NoExternalImportsError{PackageName: pkg.Name}
}

var filtered []string
Expand All @@ -67,7 +85,7 @@ func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]s

switch len(filtered) {
case 0:
return label.NoLabel, errors.New("no external imports")
return label.NoLabel, &NoExternalImportsError{PackageName: pkg.Name}

case 1:
var ret string
Expand All @@ -78,12 +96,10 @@ func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]s
return label.Parse(ret)

default:
r.logger.Error().Msg("Append one of the following to BUILD.bazel:")
for _, k := range filtered {
r.logger.Error().Msgf("# gazelle:resolve java %s %s", pkg.Name, k)
return label.NoLabel, &MultipleExternalImportsError{
PackageName: pkg.Name,
PossiblePackages: filtered,
}

return label.NoLabel, errors.New("many possible imports")
}
}

Expand Down
19 changes: 18 additions & 1 deletion java/gazelle/resolve.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package gazelle

import (
"errors"
"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/maven"
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
"github.com/bazelbuild/bazel-gazelle/config"
Expand Down Expand Up @@ -197,7 +199,22 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config
return label.NoLabel, nil
}

if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts(), pc.MavenRepositoryName()); err == nil {
if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts(), pc.MavenRepositoryName()); err != nil {
var noExternal *maven.NoExternalImportsError
var multipleExternal *maven.MultipleExternalImportsError

if errors.As(err, &noExternal) {
// do not fail, the package might be provided elsewhere
} else if errors.As(err, &multipleExternal) {
jr.lang.logger.Error().Msg("Append one of the following to BUILD.bazel:")
for _, possible := range multipleExternal.PossiblePackages {
jr.lang.logger.Error().Msgf("# gazelle:resolve java %s %s", imp.Name, possible)
}
jr.lang.hasHadErrors = true
} else {
jr.lang.logger.Fatal().Err(err).Msg("maven resolver error")
}
} else {
return l, nil
}

Expand Down
1 change: 1 addition & 0 deletions java/gazelle/testdata/maven/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ http_archive(
load("@rules_jvm_external//:defs.bzl", "maven_install")

maven_install(
name = "vendor_java",
artifacts = [
"junit:junit:4.13.1",
"com.google.guava:guava:30.0-jre",
Expand Down
5 changes: 5 additions & 0 deletions java/gazelle/testdata/maven_with_collision/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Maven with collision

Make sure the java extension does fail on colliding packages in maven.

Note that the maven_install.json file is manually crafted/invalid in order to simulate a collision on the `com.google.common.primitives` import.
27 changes: 27 additions & 0 deletions java/gazelle/testdata/maven_with_collision/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 = [
"junit:junit:4.13.1",
"com.google.guava:guava:30.0-jre",
],
fetch_sources = True,
maven_install_json = "//:maven_install.json",
repositories = [
"http://uk.maven.org/maven2",
"https://jcenter.bintray.com/",
],
)

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

pinned_maven_install()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
5 changes: 5 additions & 0 deletions java/gazelle/testdata/maven_with_collision/expectedStderr.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
12:00AM ERR java/gazelle/resolve.go:XXX > Append one of the following to BUILD.bazel:
12:00AM ERR java/gazelle/resolve.go:XXX > # gazelle:resolve java com.google.common.primitives @maven//:com_google_guava_guava
12:00AM ERR java/gazelle/resolve.go:XXX > # gazelle:resolve java com.google.common.primitives @maven//:com_google_guava_guava_sources
12:00AM WRN java/gazelle/resolve.go:XXX > Unable to find package for import in any dependency from rule=//src/main/java/com/example/myproject package=com.google.common.primitives
12:00AM FTL java/gazelle/lang.go:XXX > the java extension encontered errors that will create invalid build files
82 changes: 82 additions & 0 deletions java/gazelle/testdata/maven_with_collision/maven_install.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{
"dependency_tree": {
"dependencies": [
{
"coord": "com.google.guava:guava:30.0-jre",
"dependencies": [
"com.google.code.findbugs:jsr305:3.0.2",
"com.google.errorprone:error_prone_annotations:2.3.4",
"com.google.guava:failureaccess:1.0.1",
"com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava",
"com.google.j2objc:j2objc-annotations:1.3",
"org.checkerframework:checker-qual:3.5.0"
],
"directDependencies": [
"com.google.code.findbugs:jsr305:3.0.2",
"com.google.errorprone:error_prone_annotations:2.3.4",
"com.google.guava:failureaccess:1.0.1",
"com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava",
"com.google.j2objc:j2objc-annotations:1.3",
"org.checkerframework:checker-qual:3.5.0"
],
"file": "v1/https/jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre.jar",
"mirror_urls": [
"http://uk.maven.org/maven2/com/google/guava/guava/30.0-jre/guava-30.0-jre.jar",
"https://jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre.jar"
],
"packages": [
"com.google.common.annotations",
"com.google.common.base",
"com.google.common.base.internal",
"com.google.common.cache",
"com.google.common.collect",
"com.google.common.escape",
"com.google.common.eventbus",
"com.google.common.graph",
"com.google.common.hash",
"com.google.common.html",
"com.google.common.io",
"com.google.common.math",
"com.google.common.net",
"com.google.common.primitives",
"com.google.common.reflect",
"com.google.common.util.concurrent",
"com.google.common.xml",
"com.google.thirdparty.publicsuffix"
],
"sha256": "56b292df9ec29d102820c1fd7dd581cd749d5c416c7b3aeac008dbda3b984cc2",
"url": "https://jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre.jar"
},
{
"coord": "com.google.guava:guava:jar:sources:30.0-jre",
"dependencies": [
"com.google.code.findbugs:jsr305:jar:sources:3.0.2",
"com.google.errorprone:error_prone_annotations:jar:sources:2.3.4",
"com.google.guava:failureaccess:jar:sources:1.0.1",
"com.google.guava:listenablefuture:jar:sources:9999.0-empty-to-avoid-conflict-with-guava",
"com.google.j2objc:j2objc-annotations:jar:sources:1.3",
"org.checkerframework:checker-qual:jar:sources:3.5.0"
],
"directDependencies": [
"com.google.code.findbugs:jsr305:jar:sources:3.0.2",
"com.google.errorprone:error_prone_annotations:jar:sources:2.3.4",
"com.google.guava:failureaccess:jar:sources:1.0.1",
"com.google.guava:listenablefuture:jar:sources:9999.0-empty-to-avoid-conflict-with-guava",
"com.google.j2objc:j2objc-annotations:jar:sources:1.3",
"org.checkerframework:checker-qual:jar:sources:3.5.0"
],
"file": "v1/https/jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre-sources.jar",
"mirror_urls": [
"http://uk.maven.org/maven2/com/google/guava/guava/30.0-jre/guava-30.0-jre-sources.jar",
"https://jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre-sources.jar"
],
"packages": [
"com.google.common.primitives"
],
"sha256": "daa8a245663f9027ae4b84239147d3439221839155a4d93cbab280c3e657a73d",
"url": "https://jcenter.bintray.com/com/google/guava/guava/30.0-jre/guava-30.0-jre-sources.jar"
}
],
"version": "0.1.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.example.myproject;

import com.google.common.primitives.Ints;

/** This application compares two numbers, using the Ints.compare method from Guava. */
public class App {

public static int compare(int a, int b) {
return Ints.compare(a, b);
}

public static void main(String... args) throws Exception {
App app = new App();
System.out.println("Success: " + app.compare(2, 1));
}
}
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:resolve java com.google.common.primitives @maven//:com_google_guava_guava
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:resolve java com.google.common.primitives @maven//:com_google_guava_guava
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Maven with collision and resolve

Make sure the java extension does not fail on colliding packages in maven if the correct resolve directives are present.

Note that the maven_install.json file is manually crafted/invalid in order to simulate a collision on the `com.google.common.primitives` import.
27 changes: 27 additions & 0 deletions java/gazelle/testdata/maven_with_collision_and_resolve/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 = [
"junit:junit:4.13.1",
"com.google.guava:guava:30.0-jre",
],
fetch_sources = True,
maven_install_json = "//:maven_install.json",
repositories = [
"http://uk.maven.org/maven2",
"https://jcenter.bintray.com/",
],
)

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

pinned_maven_install()
Empty file.
Loading

0 comments on commit 8ff3022

Please sign in to comment.