From a47ada987f53dc621f6a45800d043d09ca27e596 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 17 May 2023 10:26:53 +0200 Subject: [PATCH] Fix handling of aliases (#64) Work around https://github.com/bazelbuild/bazel/issues/17749 and https://github.com/bazelbuild/bazel/issues/18421 --- pkg/target_determinator.go | 60 ++++++++++++++----- .../integration/Tests.java | 18 ++++++ 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/pkg/target_determinator.go b/pkg/target_determinator.go index 7f36f1aa..6cb568d6 100644 --- a/pkg/target_determinator.go +++ b/pkg/target_determinator.go @@ -6,6 +6,7 @@ import ( "crypto/sha1" "encoding/hex" "fmt" + "io" "log" "os" "os/exec" @@ -753,20 +754,51 @@ func runToCqueryResult(context *Context, pattern string, includeTransitions bool func findCompatibleTargets(context *Context, pattern string) (map[label.Label]bool, error) { log.Printf("Finding compatible targets under %s", pattern) - var stdout bytes.Buffer - var stderr bytes.Buffer - returnVal, err := context.BazelCmd.Execute( - BazelCmdConfig{Dir: context.WorkspacePath, Stdout: &stdout, Stderr: &stderr}, - []string{"--output_base", context.BazelOutputBase}, "cquery", pattern, - "--output=starlark", - "--starlark:expr=target.label if \"IncompatiblePlatformProvider\" not in providers(target) else \"\"", - ) - if returnVal != 0 || err != nil { - return nil, fmt.Errorf("failed to run compatibility-filtering cquery on %s: %w. Stderr:\n%v", pattern, err, stderr.String()) + compatibleTargets := make(map[label.Label]bool) + + // Add the `or []` to work around https://github.com/bazelbuild/bazel/issues/17749 which was fixed in 6.2.0. + queryFilter := ` if "IncompatiblePlatformProvider" not in (providers(target) or []) else ""` + + // Separate alias and non-alias targets to work around https://github.com/bazelbuild/bazel/issues/18421 + { + var stdout bytes.Buffer + var stderr bytes.Buffer + returnVal, err := context.BazelCmd.Execute( + BazelCmdConfig{Dir: context.WorkspacePath, Stdout: &stdout, Stderr: &stderr}, + []string{"--output_base", context.BazelOutputBase}, "cquery", fmt.Sprintf("%s - kind(alias, %s)", pattern, pattern), + "--output=starlark", + "--starlark:expr=target.label"+queryFilter, + ) + if returnVal != 0 || err != nil { + return nil, fmt.Errorf("failed to run compatibility-filtering cquery on %s: %w. Stderr:\n%v", pattern, err, stderr.String()) + } + if err := addCompatibleTargetsLines(&stdout, compatibleTargets); err != nil { + return nil, err + } } - compatibleTargets := make(map[label.Label]bool) - scanner := bufio.NewScanner(&stdout) + { + var stdout bytes.Buffer + var stderr bytes.Buffer + returnVal, err := context.BazelCmd.Execute( + BazelCmdConfig{Dir: context.WorkspacePath, Stdout: &stdout, Stderr: &stderr}, + []string{"--output_base", context.BazelOutputBase}, "cquery", fmt.Sprintf("kind(alias, %s)", pattern), + "--output=starlark", + // Example output of `repr(target)` for an alias target: `` + "--starlark:expr=repr(target).split(\" \")[2]"+queryFilter, + ) + if returnVal != 0 || err != nil { + return nil, fmt.Errorf("failed to run alias compatibility-filtering cquery on %s: %w. Stderr:\n%v", pattern, err, stderr.String()) + } + if err := addCompatibleTargetsLines(&stdout, compatibleTargets); err != nil { + return nil, err + } + } + return compatibleTargets, nil +} + +func addCompatibleTargetsLines(r io.Reader, compatibleTargets map[label.Label]bool) error { + scanner := bufio.NewScanner(r) for scanner.Scan() { labelStr := scanner.Text() if labelStr == "" { @@ -774,11 +806,11 @@ func findCompatibleTargets(context *Context, pattern string) (map[label.Label]bo } label, err := ParseCanonicalLabel(labelStr) if err != nil { - return nil, fmt.Errorf("failed to parse label from compatibility-filtering: %q: %w", labelStr, err) + return fmt.Errorf("failed to parse label from compatibility-filtering: %q: %w", labelStr, err) } compatibleTargets[label] = true } - return compatibleTargets, nil + return nil } // MatchingTargets stores the top-level targets within a repository, diff --git a/tests/integration/java/com/github/bazel_contrib/target_determinator/integration/Tests.java b/tests/integration/java/com/github/bazel_contrib/target_determinator/integration/Tests.java index 417069ad..c244662b 100644 --- a/tests/integration/java/com/github/bazel_contrib/target_determinator/integration/Tests.java +++ b/tests/integration/java/com/github/bazel_contrib/target_determinator/integration/Tests.java @@ -534,6 +534,18 @@ public void ignoredPlatformSpecificDepChanged() throws Exception { doTest(Commits.SELECT_TARGET, after, Set.of(changedDepTarget)); } + @Test + public void aliasTargetIsDetectedIfActualTargetChanged() throws Exception { + doTest(Commits.ALIAS_ADD_TARGET, Commits.ALIAS_CHANGE_TARGET_THROUGH_ALIAS, + Set.of("//java/example:ExampleTest", "//java/example:example_test")); + } + + @Test + public void aliasTargetIsDetectedIfActualLabelChanged() throws Exception { + doTest(Commits.ALIAS_ADD_TARGET, Commits.ALIAS_CHANGE_ACTUAL, + Set.of("//java/example:example_test")); + } + public void doTest(String commitBefore, String commitAfter, Set expectedTargets) throws TargetComputationErrorException { doTest(commitBefore, commitAfter, expectedTargets, Set.of()); } @@ -674,4 +686,10 @@ class Commits { public static final String CHANGED_NONLINUX_DEP = "a8c04169ef46095d040048610b24adbea4027f32"; public static final String CHANGED_LINUX_DEP = "c5a9f0ad1fc7fc9c3dd31fd904fcc97a55bd2fce"; + + public static final String ALIAS_ADD_TARGET = "194478bcdfb3f8c40b3dd02d06d2be1cd335b83b"; + + public static final String ALIAS_CHANGE_ACTUAL = "ec19104291a3ea7cb61fb54ecdeaee73127bf284"; + + public static final String ALIAS_CHANGE_TARGET_THROUGH_ALIAS = "3d9788053a2eed1a2f3beb05070872526ebdf76e"; }