From 20243af927a7023e6639e9feb5b3d41c400b26be Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 19 Jun 2023 23:28:46 +0100 Subject: [PATCH] Handle aliases to source files (#67) Previously an alias to a source file ended up not hashing the source file because the file didn't exist in the same configuration as the depender. --- pkg/hash_cache.go | 21 +++++++++++++++---- .../integration/Tests.java | 10 +++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/pkg/hash_cache.go b/pkg/hash_cache.go index 9fac8824..e5756427 100644 --- a/pkg/hash_cache.go +++ b/pkg/hash_cache.go @@ -515,11 +515,24 @@ func hashRule(thc *TargetHashCache, rule *build.Rule, configuration *analysis.Co return nil, fmt.Errorf("failed to parse ruleInput label %s: %w", ruleInputLabelString, err) } var depConfigurations []Configuration + // Aliases don't transition, and we've seen aliases expanding across configurations cause dependency cycles for nogo targets. if rule.GetRuleClass() == "alias" { - // Aliases don't transition, and we've seen aliases expanding across configurations cause dependency cycles for nogo targets. - // Narrow just to the current configuration in this case. - depConfiguration := NormalizeConfiguration(configuration.GetChecksum()) - depConfigurations = []Configuration{depConfiguration} + knownDepConfigurations := thc.context[ruleInputLabel] + isSourceFile := true + for _, ct := range knownDepConfigurations { + if ct.GetTarget().GetType() != build.Target_SOURCE_FILE { + isSourceFile = false + } + } + + if isSourceFile { + // If it's a source file, it doesn't exist in the current configuration, but does exist in the empty configuration. + // Accordingly, we need to explicitly transition to the empty configuration. + depConfigurations = []Configuration{NormalizeConfiguration("")} + } else { + // If it's not a source file, narrow just to the current configuration - we know there was no transition, so we must be in the same configuration. + depConfigurations = []Configuration{NormalizeConfiguration(configuration.GetChecksum())} + } } else { depConfigurations = thc.KnownConfigurations(ruleInputLabel).SortedSlice() } 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 c244662b..329ba706 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 @@ -540,6 +540,12 @@ public void aliasTargetIsDetectedIfActualTargetChanged() throws Exception { Set.of("//java/example:ExampleTest", "//java/example:example_test")); } + @Test + public void aliasTargetIsDetectedIfActualFileChanged() throws Exception { + doTest(Commits.ALIAS_ADD_TARGET_TO_FILE, Commits.ALIAS_CHANGE_TARGET_THROUGH_ALIAS_TO_FILE, + Set.of("//java/example:ExampleTest", "//java/example:ExampleTestSource")); + } + @Test public void aliasTargetIsDetectedIfActualLabelChanged() throws Exception { doTest(Commits.ALIAS_ADD_TARGET, Commits.ALIAS_CHANGE_ACTUAL, @@ -692,4 +698,8 @@ class Commits { public static final String ALIAS_CHANGE_ACTUAL = "ec19104291a3ea7cb61fb54ecdeaee73127bf284"; public static final String ALIAS_CHANGE_TARGET_THROUGH_ALIAS = "3d9788053a2eed1a2f3beb05070872526ebdf76e"; + + public static final String ALIAS_ADD_TARGET_TO_FILE = "7be0f96f26742daec661c2ebfbb08b88b6355a3b"; + + public static final String ALIAS_CHANGE_TARGET_THROUGH_ALIAS_TO_FILE = "68fbd3661f3626e2df6a55b079444adbf76b5a3b"; }