Skip to content

Avoid rdeps when using hash all targets #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 19, 2021
Merged

Avoid rdeps when using hash all targets #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29362fb
Avoid rdeps when using hash all targets
tinder-maxwellelliott May 17, 2021
bf6d2b2
fix for potential null pointer
tinder-maxwellelliott May 19, 2021
4e12513
address feedback
tinder-maxwellelliott May 19, 2021
d26cd08
Merge branch 'master' into skip_rdeps_all_targets
tinder-maxwellelliott May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions bazel-diff-example.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ previous_revision=$3
# Final Revision SHA
final_revision=$4

modified_filepaths_output="/tmp/modified_filepaths.txt"
starting_hashes_json="/tmp/starting_hashes.json"
final_hashes_json="/tmp/final_hashes.json"
impacted_targets_path="/tmp/impacted_targets.txt"
Expand All @@ -23,29 +22,21 @@ shared_flags=""

$bazel_path run :bazel-diff $shared_flags --script_path="$bazel_diff"

$bazel_diff modified-filepaths $previous_revision $final_revision -w $workspace_path -b $bazel_path $modified_filepaths_output

IFS=$'\n' read -d '' -r -a modified_filepaths < $modified_filepaths_output
formatted_filepaths=$(IFS=$'\n'; echo "${modified_filepaths[*]}")
echo "Modified Filepaths:"
echo $formatted_filepaths
echo ""

git -C $workspace_path checkout $previous_revision --quiet

echo "Generating Hashes for Revision '$previous_revision'"
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json -a

git -C $workspace_path checkout $final_revision --quiet

echo "Generating Hashes for Revision '$final_revision'"
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path -m $modified_filepaths_output $final_hashes_json
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $final_hashes_json -a

echo "Determining Impacted Targets"
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path -a

echo "Determining Impacted Test Targets"
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_test_targets_path --avoid-query "//... except tests(//...)"
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_test_targets_path -a --avoid-query "//... except tests(//...)"

IFS=$'\n' read -d '' -r -a impacted_targets < $impacted_targets_path
formatted_impacted_targets=$(IFS=$'\n'; echo "${impacted_targets[*]}")
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/bazel_diff/BazelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public Set<String> queryForImpactedTargets(Set<String> impactedTargets, String a
String targetQuery = impactedTargets.stream()
.map(target -> String.format("'%s'", target))
.collect(Collectors.joining(" + "));
String query = String.format("rdeps(//... except '//external:all-targets', %s)", targetQuery);
String query = query = String.format("rdeps(//... except '//external:all-targets', %s)", targetQuery);
if (avoidQuery != null) {
query = String.format("(%s) except (%s)", query, avoidQuery);
}
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/com/bazel_diff/TargetHashingClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
interface TargetHashingClient {
Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths) throws IOException, NoSuchAlgorithmException;
Map<String, String> hashAllBazelTargetsAndSourcefiles() throws IOException, NoSuchAlgorithmException;
Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes, String avoidQuery) throws IOException;
Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes, String avoidQuery, Boolean hashAllTargets) throws IOException;
}

class TargetHashingClientImpl implements TargetHashingClient {
Expand All @@ -36,7 +36,8 @@ public Map<String, String> hashAllBazelTargetsAndSourcefiles() throws IOExceptio
public Set<String> getImpactedTargets(
Map<String, String> startHashes,
Map<String, String> endHashes,
String avoidQuery)
String avoidQuery,
Boolean hashAllTargets)
throws IOException {
Set<String> impactedTargets = new HashSet<>();
for (Map.Entry<String,String> entry : endHashes.entrySet()) {
Expand All @@ -45,6 +46,9 @@ public Set<String> getImpactedTargets(
impactedTargets.add(entry.getKey());
}
}
if (hashAllTargets != null && hashAllTargets && avoidQuery == null) {
return impactedTargets;
}
return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery);
}

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/com/bazel_diff/main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ class BazelDiff implements Callable<Integer> {
@Option(names = {"-o", "--output"}, scope = ScopeType.LOCAL, description = "Filepath to write the impacted Bazel targets to, newline separated")
File outputPath;

@Option(names = {"-a", "--all-sourcefiles"}, description = "Experimental: Hash all sourcefile targets (instead of relying on --modifiedFilepaths), Warning: Performance may degrade from reading all source files")
Boolean hashAllSourcefiles;

@Option(names = {"-aq", "--avoid-query"}, scope = ScopeType.LOCAL, description = "A Bazel query string, any targets that pass this query will be removed from the returned set of targets")
String avoidQuery;

Expand Down Expand Up @@ -207,7 +210,7 @@ public Integer call() throws IOException {
Map<String, String > gsonHash = new HashMap<>();
Map<String, String> startingHashes = gson.fromJson(startingFileReader, gsonHash.getClass());
Map<String, String> finalHashes = gson.fromJson(finalFileReader, gsonHash.getClass());
Set<String> impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery);
Set<String> impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery, hashAllSourcefiles);
try {
FileWriter myWriter = new FileWriter(outputPath);
myWriter.write(impactedTargets.stream().collect(Collectors.joining(System.lineSeparator())));
Expand Down
43 changes: 41 additions & 2 deletions test/java/com/bazel_diff/TargetHashingClientImplTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void getImpactedTargets() throws IOException {
hash2.put("rule1", "differentrule1hash");
hash2.put("rule2", "rule2hash");
hash2.put("rule3", "rule3hash");
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null);
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null, false);
Set<String> expectedSet = new HashSet<>();
expectedSet.add("rule1");
expectedSet.add("rule3");
Expand All @@ -149,7 +149,46 @@ public void getImpactedTargets_withAvoidQuery() throws IOException {
hash2.put("rule1", "differentrule1hash");
hash2.put("rule2", "rule2hash");
hash2.put("rule3", "rule3hash");
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query");
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", false);
Set<String> expectedSet = new HashSet<>();
expectedSet.add("rule1");
assertEquals(expectedSet, impactedTargets);
}

@Test
public void getImpactedTargets_withHashAllTargets() throws IOException {
when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject())).thenReturn(
new HashSet<>(Arrays.asList("rule1"))
);
TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock);
Map<String, String> hash1 = new HashMap<>();
hash1.put("rule1", "rule1hash");
hash1.put("rule2", "rule2hash");
Map<String, String> hash2 = new HashMap<>();
hash2.put("rule1", "differentrule1hash");
hash2.put("rule2", "rule2hash");
hash2.put("rule3", "rule3hash");
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null, true);
Set<String> expectedSet = new HashSet<>();
expectedSet.add("rule1");
expectedSet.add("rule3");
assertEquals(expectedSet, impactedTargets);
}

@Test
public void getImpactedTargets_withHashAllTargets_withAvoidQuery() throws IOException {
when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"))).thenReturn(
new HashSet<>(Arrays.asList("rule1"))
);
TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock);
Map<String, String> hash1 = new HashMap<>();
hash1.put("rule1", "rule1hash");
hash1.put("rule2", "rule2hash");
Map<String, String> hash2 = new HashMap<>();
hash2.put("rule1", "differentrule1hash");
hash2.put("rule2", "rule2hash");
hash2.put("rule3", "rule3hash");
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", true);
Set<String> expectedSet = new HashSet<>();
expectedSet.add("rule1");
assertEquals(expectedSet, impactedTargets);
Expand Down