feat: Allow queries, not just target patterns (#15)

* feat: Allow queries in addition to target patterns

Co-authored-by: Martin Kosiba <[email protected]>
Co-authored-by: Romain Chossart <[email protected]>

README.md

@@ -14,8 +14,8 @@ Where <before-revision> may be any commit revision - full commit hashes, short c
     	Bazel binary (basename on $PATH, or absolute or relative path) to run (default "bazel")
   -ignore-file value
     	Files to ignore for git operations, relative to the working-directory. These files shan't affect the Bazel graph.
-  -target-pattern string
-    	Target pattern to diff. (default "//...")
+  -targets bazel query
+      Targets to consider. Accepts any valid bazel query expression (see https://bazel.build/reference/query). (default "//...")
   -verbose
     	Whether to explain (messily) why each target is getting run
   -working-directory string
@@ -39,8 +39,8 @@ Optional flags:
     	Files to ignore for git operations, relative to the working-directory. These files shan't affect the Bazel graph.
   -manual-test-mode string
     	How to handle affected tests tagged manual. Possible values: run|skip (default "skip")
-  -target-pattern string
-    	Target pattern to consider. (default "//...")
+  -targets bazel query
+      Targets to consider. Accepts any valid bazel query expression (see https://bazel.build/reference/query). (default "//...")
   -working-directory string
     	Working directory to query (default ".")
 ```

WORKSPACE

@@ -2,9 +2,6 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
 http_archive(
     name = "bazel_gazelle",
-    patch_args = ["-p1"],
-    # Pull in https://github.com/bazelbuild/bazel-gazelle/pull/1227
-    patches = ["@//:third_party/patches/bazel_gazelle/1227-label-pattern-matching.patch"],
     sha256 = "73bac497740e1a9583354ca36332cd48fd60803b1d0ed927ea3fcb3488565182",
     strip_prefix = "bazel-gazelle-a98769c9a26e312ac40d1f5b14f574fdcd4aa21a",
     urls = [

cli/BUILD.bazel

@@ -11,6 +11,5 @@ go_library(
     deps = [
         "//common",
         "//pkg",
-        "@bazel_gazelle//label:go_default_library",
     ],
 )

cli/flags.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/bazel-contrib/target-determinator/common"
 	"github.com/bazel-contrib/target-determinator/pkg"
-	gazelle_label "github.com/bazelbuild/bazel-gazelle/label"
 )
 
 type IgnoreFileFlag []common.RelPath
@@ -71,7 +70,7 @@ type CommonFlags struct {
 	EnforceCleanRepo     EnforceCleanFlag
 	DeleteCachedWorktree bool
 	IgnoredFiles         *IgnoreFileFlag
-	TargetPatternFlag    *string
+	TargetsFlag          *string
 }
 
 func StrPtr() *string {
@@ -86,7 +85,7 @@ func RegisterCommonFlags() *CommonFlags {
 		EnforceCleanRepo:     AllowIgnored,
 		DeleteCachedWorktree: false,
 		IgnoredFiles:         &IgnoreFileFlag{},
-		TargetPatternFlag:    StrPtr(),
+		TargetsFlag:          StrPtr(),
 	}
 	flag.StringVar(commonFlags.WorkingDirectory, "working-directory", ".", "Working directory to query.")
 	flag.StringVar(commonFlags.BazelPath, "bazel", "bazel",
@@ -98,14 +97,15 @@ func RegisterCommonFlags() *CommonFlags {
 		"Delete created worktrees after use when created. Keeping them can make subsequent invocations faster.")
 	flag.Var(commonFlags.IgnoredFiles, "ignore-file",
 		"Files to ignore for git operations, relative to the working-directory. These files shan't affect the Bazel graph.")
-	flag.StringVar(commonFlags.TargetPatternFlag, "target-pattern", "//...", "Target pattern to diff.")
+	flag.StringVar(commonFlags.TargetsFlag, "targets", "//...",
+		"Targets to consider. Accepts any valid `bazel query` expression (see https://bazel.build/reference/query).")
 	return &commonFlags
 }
 
 type CommonConfig struct {
 	Context        *pkg.Context
 	RevisionBefore pkg.LabelledGitRev
-	TargetPattern  gazelle_label.Pattern
+	Targets        pkg.TargetsList
 }
 
 // ValidateCommonFlags ensures that the argument follow the right format
@@ -158,9 +158,9 @@ func ResolveCommonConfig(commonFlags *CommonFlags, beforeRevStr string) (*Common
 		return nil, fmt.Errorf("failed to resolve the \"before\" git revision: %w", err)
 	}
 
-	targetPattern, err := gazelle_label.ParsePattern(*commonFlags.TargetPatternFlag)
+	targetsList, err := pkg.ParseTargetsList(*commonFlags.TargetsFlag)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse target pattern: %w", err)
+		return nil, fmt.Errorf("failed to parse targets: %w", err)
 	}
 
 	// Additional checks
@@ -182,6 +182,6 @@ func ResolveCommonConfig(commonFlags *CommonFlags, beforeRevStr string) (*Common
 	return &CommonConfig{
 		Context:        context,
 		RevisionBefore: beforeRev,
-		TargetPattern:  targetPattern,
+		Targets:        targetsList,
 	}, nil
 }

driver/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 load("//rules:multi_platform_go_binary.bzl", "multi_platform_go_binary")
 
 go_library(

driver/driver.go

@@ -33,7 +33,7 @@ type driverFlags struct {
 type config struct {
 	Context        *pkg.Context
 	RevisionBefore pkg.LabelledGitRev
-	TargetPattern  gazelle_label.Pattern
+	Targets        pkg.TargetsList
 	// One of "run" or "skip".
 	ManualTestMode string
 }
@@ -77,7 +77,7 @@ func main() {
 
 	if err := pkg.WalkAffectedTargets(config.Context,
 		config.RevisionBefore,
-		config.TargetPattern,
+		config.Targets,
 		false,
 		callback); err != nil {
 		log.Fatal(err)
@@ -161,7 +161,7 @@ func resolveConfig(flags driverFlags) (*config, error) {
 	return &config{
 		Context:        commonArgs.Context,
 		RevisionBefore: commonArgs.RevisionBefore,
-		TargetPattern:  commonArgs.TargetPattern,
+		Targets:        commonArgs.Targets,
 		ManualTestMode: flags.manualTestMode,
 	}, nil
 }

pkg/BUILD.bazel

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "hash_cache.go",
         "target_determinator.go",
+        "targets_list.go",
         "walker.go",
     ],
     importpath = "github.com/bazel-contrib/target-determinator/pkg",

pkg/target_determinator.go

@@ -111,31 +111,31 @@ type Context struct {
 }
 
 // FullyProcess returns the before and after metadata maps, with fully filled caches.
-func FullyProcess(context *Context, revBefore LabelledGitRev, revAfter LabelledGitRev, pattern label.Pattern) (*QueryResults, *QueryResults, error) {
+func FullyProcess(context *Context, revBefore LabelledGitRev, revAfter LabelledGitRev, targets TargetsList) (*QueryResults, *QueryResults, error) {
 	log.Printf("Processing %s", revBefore)
-	queryInfoBefore, err := fullyProcessRevision(context, revBefore, pattern)
+	queryInfoBefore, err := fullyProcessRevision(context, revBefore, targets)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	// At this point, we assume that the working directory is back to its pristine state.
 	log.Printf("Processing %s", revAfter)
-	queryInfoAfter, err := fullyProcessRevision(context, revAfter, pattern)
+	queryInfoAfter, err := fullyProcessRevision(context, revAfter, targets)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	return queryInfoBefore, queryInfoAfter, nil
 }
 
-func fullyProcessRevision(context *Context, rev LabelledGitRev, pattern label.Pattern) (queryInfo *QueryResults, err error) {
+func fullyProcessRevision(context *Context, rev LabelledGitRev, targets TargetsList) (queryInfo *QueryResults, err error) {
 	defer func() {
 		innerErr := gitCheckout(context.WorkspacePath, context.OriginalRevision)
 		if innerErr != nil && err == nil {
 			err = fmt.Errorf("failed to check out original commit during cleanup: %v", err)
 		}
 	}()
-	queryInfo, loadMetadataCleanup, err := LoadIncompleteMetadata(context, rev, pattern)
+	queryInfo, loadMetadataCleanup, err := LoadIncompleteMetadata(context, rev, targets)
 	defer loadMetadataCleanup()
 	if err != nil {
 		return nil, fmt.Errorf("failed to load metadata at %s: %w", rev, err)
@@ -149,14 +149,13 @@ func fullyProcessRevision(context *Context, rev LabelledGitRev, pattern label.Pa
 }
 
 // LoadIncompleteMetadata loads the metadata about, but not hashes of, targets into a QueryResults.
-// The (transitive) dependencies of the passed pattern will be loaded. For all targets, pass the
-// pattern `//...`.
+// The (transitive) dependencies of the passed targets will be loaded. For all targets, use `//...`.
 //
 // It may change the git revision of the workspace to rev, in which case it is the caller's
 // responsibility to check out the original commit.
 //
 // It returns a non-nil callback to clean up the worktree if it was created.
-func LoadIncompleteMetadata(context *Context, rev LabelledGitRev, pattern label.Pattern) (*QueryResults, func(), error) {
+func LoadIncompleteMetadata(context *Context, rev LabelledGitRev, targets TargetsList) (*QueryResults, func(), error) {
 	// Create a temporary context to allow the workspace path to point to a git worktree if necessary.
 	context = &Context{
 		WorkspacePath:        context.WorkspacePath,
@@ -195,7 +194,7 @@ func LoadIncompleteMetadata(context *Context, rev LabelledGitRev, pattern label.
 		return nil, cleanupFunc, err
 	}
 
-	queryInfo, err := doQueryDeps(context, pattern)
+	queryInfo, err := doQueryDeps(context, targets)
 	if err != nil {
 		return nil, cleanupFunc, fmt.Errorf("failed to query at %s in %v: %w", rev, context.WorkspacePath, err)
 	}
@@ -557,13 +556,13 @@ func bazelInfo(bazelPath string, workspacePath string, key string) (string, erro
 	return strings.TrimRight(stdoutBuf.String(), "\n"), nil
 }
 
-func doQueryDeps(context *Context, pattern label.Pattern) (*QueryResults, error) {
+func doQueryDeps(context *Context, targets TargetsList) (*QueryResults, error) {
 	bazelRelease, err := BazelRelease(context.BazelPath, context.WorkspacePath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to resolve the bazel release: %w", err)
 	}
 
-	depsPattern := fmt.Sprintf("deps(%s)", pattern.String())
+	depsPattern := fmt.Sprintf("deps(%s)", targets.String())
 	transitiveResult, err := runToCqueryResult(context, depsPattern)
 	if err != nil {
 		return nil, fmt.Errorf("failed to cquery %v: %w", depsPattern, err)
@@ -574,7 +573,7 @@ func doQueryDeps(context *Context, pattern label.Pattern) (*QueryResults, error)
 		return nil, fmt.Errorf("failed to parse cquery result: %w", err)
 	}
 
-	matchingTargetResults, err := runToCqueryResult(context, pattern.String())
+	matchingTargetResults, err := runToCqueryResult(context, targets.String())
 	if err != nil {
 		return nil, fmt.Errorf("failed to run top-level cquery: %w", err)
 	}

pkg/targets_list.go

@@ -0,0 +1,14 @@
+package pkg
+
+type TargetsList struct {
+	targets string
+}
+
+func ParseTargetsList(targets string) (TargetsList, error) {
+	// TODO: validate against syntax in https://bazel.build/reference/query
+	return TargetsList{targets: targets}, nil
+}
+
+func (tl *TargetsList) String() string {
+	return tl.targets
+}

pkg/walker.go

@@ -15,15 +15,15 @@ type WalkCallback func(label.Label, []Difference, *analysis.ConfiguredTarget)
 // callback once for each target which has changed.
 // Explanation of the differences may be expensive in both time and memory to compute, so if
 // includeDifferences is set to false, the []Difference parameter to the callback will always be nil.
-func WalkAffectedTargets(context *Context, revBefore LabelledGitRev, pattern label.Pattern, includeDifferences bool, callback WalkCallback) error {
+func WalkAffectedTargets(context *Context, revBefore LabelledGitRev, targets TargetsList, includeDifferences bool, callback WalkCallback) error {
 	// The revAfter revision represents the current state of the working directory, which may contain local changes.
 	// It is distinct from context.OriginalRevision, which represents the original commit that we want to reset to before exiting.
 	revAfter, err := NewLabelledGitRev(context.WorkspacePath, "", "after")
 	if err != nil {
 		return fmt.Errorf("could not create \"after\" revision: %w", err)
 	}
 
-	beforeMetadata, afterMetadata, err := FullyProcess(context, revBefore, revAfter, pattern)
+	beforeMetadata, afterMetadata, err := FullyProcess(context, revBefore, revAfter, targets)
 	if err != nil {
 		return fmt.Errorf("failed to process change: %w", err)
 	}

target-determinator/target-determinator.go

@@ -30,7 +30,7 @@ type targetDeterminatorFlags struct {
 type config struct {
 	Context        *pkg.Context
 	RevisionBefore pkg.LabelledGitRev
-	TargetPattern  gazelle_label.Pattern
+	Targets        pkg.TargetsList
 	Verbose        bool
 }
 
@@ -79,7 +79,7 @@ func main() {
 
 	if err := pkg.WalkAffectedTargets(config.Context,
 		config.RevisionBefore,
-		config.TargetPattern,
+		config.Targets,
 		config.Verbose,
 		callback); err != nil {
 		// Print something on stdout that will make bazel fail when passed as a target.
@@ -112,7 +112,7 @@ func resolveConfig(flags targetDeterminatorFlags) (*config, error) {
 	return &config{
 		Context:        commonArgs.Context,
 		RevisionBefore: commonArgs.RevisionBefore,
-		TargetPattern:  commonArgs.TargetPattern,
+		Targets:        commonArgs.Targets,
 		Verbose:        flags.verbose,
 	}, nil
 }

tests/integration/java/com/github/bazel_contrib/target_determinator/integration/TargetDeterminatorSpecificFlagsTest.java

@@ -138,16 +138,16 @@ public void testWorktreeCreation() throws Exception {
 
   }
 
-  private Set<Label> getTargets(String commitBefore, String targetPattern) throws Exception {
-    return getTargets(commitBefore, targetPattern, false, true);
+  private Set<Label> getTargets(String commitBefore, String targets) throws Exception {
+    return getTargets(commitBefore, targets, false, true);
   }
 
-  private Set<Label> getTargets(String commitBefore, String targetPattern, boolean enforceClean, boolean deleteCachedWorktree)
+  private Set<Label> getTargets(String commitBefore, String targets, boolean enforceClean, boolean deleteCachedWorktree)
       throws Exception {
     final List<String> args = Stream.of("--working-directory",
         testDir.toString(),
         "--bazel", "bazelisk",
-        "--target-pattern", targetPattern
+        "--targets", targets 
         ).collect(Collectors.toList());
     if (enforceClean) {
       args.add("--enforce-clean=enforce-clean");