Skip to content

Commit

Permalink
Add config hash to cycle reporting, to aid debugging.
Browse files Browse the repository at this point in the history
Closes bazelbuild#13371.

PiperOrigin-RevId: 369440045
  • Loading branch information
katre authored and copybara-github committed Apr 20, 2021
1 parent 9267097 commit 660e8a5
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ public BuildOptions.OptionsDiffForReconstruction getOptionsDiff() {
return optionsDiff;
}

public String checksum() {
return optionsDiff.getChecksum();
}

@Override
public SkyFunctionName functionName() {
return SkyFunctions.BUILD_CONFIGURATION;
Expand Down Expand Up @@ -190,7 +194,7 @@ public String toString() {
// This format is depended on by integration tests.
// TODO(blaze-configurability-team): This should at least include the length of fragments.
// to at least remind devs that this Key has TWO key parts.
return "BuildConfigurationValue.Key[" + optionsDiff.getChecksum() + "]";
return "BuildConfigurationValue.Key[" + checksum() + "]";
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ public final String prettyPrint() {
if (label == null) {
return "null";
}
return label.toString();
return String.format(
"%s (%s)", label, configurationKey == null ? "null" : configurationKey.checksum());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import java.util.Map;
import java.util.regex.Pattern;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -77,13 +78,15 @@ public void testDirectPackageGroupCycle() throws Exception {

@Test
public void testThreeLongPackageGroupCycle() throws Exception {
String expectedEvent =
"cycle in dependency graph:\n"
+ " //cycle:superman\n"
+ ".-> //cycle:rock\n"
+ "| //cycle:paper\n"
+ "| //cycle:scissors\n"
+ "`-- //cycle:rock";
@SuppressWarnings("ConstantPatternCompile")
Pattern expectedEvent =
Pattern.compile(
"cycle in dependency graph:\n"
+ " //cycle:superman \\([a-f0-9]+\\)\n"
+ ".-> //cycle:rock \\(null\\)\n"
+ "| //cycle:paper \\(null\\)\n"
+ "| //cycle:scissors \\(null\\)\n"
+ "`-- //cycle:rock \\(null\\)");
checkError(
"cycle",
"superman",
Expand All @@ -94,33 +97,25 @@ public void testThreeLongPackageGroupCycle() throws Exception {
"package_group(name='scissors', includes=['//cycle:rock'])",
"sh_library(name='superman', visibility=[':rock'])");

Event foundEvent = null;
for (Event event : eventCollector) {
if (event.getMessage().contains(expectedEvent)) {
foundEvent = event;
break;
}
}
assertThat(foundEvent).isNotNull();
Event foundEvent = assertContainsEvent(expectedEvent);
assertThat(foundEvent.getLocation().toString()).isEqualTo("/workspace/cycle/BUILD:3:14");
}

@Test
public void cycleThroughVisibility() throws Exception {
String expectedEvent =
"in filegroup rule //cycle:v: cycle in dependency graph:\n"
+ " //cycle:v\n"
+ " //cycle:t\n"
+ ".-> //cycle:v\n"
+ "| //cycle:t\n"
+ "`-- //cycle:v\n"
+ "The cycle is caused by a visibility edge from //cycle:t to the non-package_group"
+ " target //cycle:v. Note that visibility labels are supposed to be package_group"
+ " targets, which prevents cycles of this form.";
checkError(
"cycle",
"v",
expectedEvent,
Pattern.compile(
"in filegroup rule //cycle:v: cycle in dependency graph:\n"
+ " //cycle:v \\([a-f0-9]+\\)\n"
+ " //cycle:t \\([a-f0-9]+\\)\n"
+ ".-> //cycle:v \\([a-f0-9]+\\)\n"
+ "| //cycle:t \\([a-f0-9]+\\)\n"
+ "`-- //cycle:v \\([a-f0-9]+\\)\n"
+ "The cycle is caused by a visibility edge from //cycle:t to the non-package_group"
+ " target //cycle:v. Note that visibility labels are supposed to be package_group"
+ " targets, which prevents cycles of this form."),
"filegroup(name='t', visibility=[':v'])",
"filegroup(name='v', srcs=[':t'])");
}
Expand Down Expand Up @@ -161,10 +156,11 @@ public void testTwoRuleCycle() throws Exception {
checkError(
"a",
"rule1",
"in cc_library rule //a:rule1: cycle in dependency graph:\n"
+ ".-> //a:rule1\n"
+ "| //b:rule2\n"
+ "`-- //a:rule1",
Pattern.compile(
"in cc_library rule //a:rule1: cycle in dependency graph:\n"
+ ".-> //a:rule1 \\([a-f0-9]+\\)\n"
+ "| //b:rule2 \\([a-f0-9]+\\)\n"
+ "`-- //a:rule1 \\([a-f0-9]+\\)"),
"cc_library(name='rule1',",
" deps=['//b:rule2'])");
}
Expand Down Expand Up @@ -199,8 +195,8 @@ public void testIndirectOneRuleCycle() throws Exception {
" cmd = 'cp $< $@')");
}

private String selfEdgeMsg(String label) {
return label + " [self-edge]";
private Pattern selfEdgeMsg(String label) {
return Pattern.compile(label + " \\([a-f0-9]+|null\\) \\[self-edge\\]");
}

// Regression test for: "IllegalStateException in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ protected void assertContainsSublist(String message, List<String> list, List<Str
}

protected void assertContainsSelfEdgeEvent(String label) {
assertContainsEvent(label + " [self-edge]");
assertContainsEvent(Pattern.compile(label + " \\([a-f0-9]+\\) \\[self-edge\\]"));
}

protected NestedSet<Artifact> collectRunfiles(ConfiguredTarget target) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/integration/aspect_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ EOF
local -r exit_code="$?"
[[ "$exit_code" == 1 ]] || fail "Unexpected exit code: $exit_code"
expect_log "cycle in dependency graph"
expect_log "//test:cycletarget \[self-edge\]"
expect_log "//test:cycletarget \(.*\) \[self-edge\]"
expect_not_log "IllegalStateException"
}

Expand Down

0 comments on commit 660e8a5

Please sign in to comment.