Skip to content

Commit

Permalink
Fix handling of aliases (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
illicitonion authored May 17, 2023
1 parent 4a95fe9 commit a47ada9
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
60 changes: 46 additions & 14 deletions pkg/target_determinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"crypto/sha1"
"encoding/hex"
"fmt"
"io"
"log"
"os"
"os/exec"
Expand Down Expand Up @@ -753,32 +754,63 @@ 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: `<alias target //java/example:example_test of //java/example:OtherExampleTest>`
"--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 == "" {
continue
}
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> expectedTargets) throws TargetComputationErrorException {
doTest(commitBefore, commitAfter, expectedTargets, Set.of());
}
Expand Down Expand Up @@ -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";
}

0 comments on commit a47ada9

Please sign in to comment.