Skip to content

Commit de74691

Browse files
tinder-maxwellelliotttinder-maxwellelliott
authored
Merge pull request #64 from Tinder/skip_rdeps_all_targets
Avoid rdeps when using hash all targets
2 parents 2728442 + d26cd08 commit de74691

File tree

5 files changed

+56
-19
lines changed

5 files changed

+56
-19
lines changed

‎bazel-diff-example.sh‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ previous_revision=$3
99
# Final Revision SHA
1010
final_revision=$4
1111

12-
modified_filepaths_output="/tmp/modified_filepaths.txt"
1312
starting_hashes_json="/tmp/starting_hashes.json"
1413
final_hashes_json="/tmp/final_hashes.json"
1514
impacted_targets_path="/tmp/impacted_targets.txt"
@@ -23,29 +22,21 @@ shared_flags=""
2322

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

26-
$bazel_diff modified-filepaths $previous_revision $final_revision -w $workspace_path -b $bazel_path $modified_filepaths_output
27-
28-
IFS=$'\n' read -d '' -r -a modified_filepaths < $modified_filepaths_output
29-
formatted_filepaths=$(IFS=$'\n'; echo "${modified_filepaths[*]}")
30-
echo "Modified Filepaths:"
31-
echo $formatted_filepaths
32-
echo ""
33-
3425
git -C $workspace_path checkout $previous_revision --quiet
3526

3627
echo "Generating Hashes for Revision '$previous_revision'"
37-
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json
28+
$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json -a
3829

3930
git -C $workspace_path checkout $final_revision --quiet
4031

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

4435
echo "Determining Impacted Targets"
45-
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path
36+
$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path -a
4637

4738
echo "Determining Impacted Test Targets"
48-
$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(//...)"
39+
$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(//...)"
4940

5041
IFS=$'\n' read -d '' -r -a impacted_targets < $impacted_targets_path
5142
formatted_impacted_targets=$(IFS=$'\n'; echo "${impacted_targets[*]}")

‎src/main/java/com/bazel_diff/BazelClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public Set<String> queryForImpactedTargets(Set<String> impactedTargets, String a
5252
String targetQuery = impactedTargets.stream()
5353
.map(target -> String.format("'%s'", target))
5454
.collect(Collectors.joining(" + "));
55-
String query = String.format("rdeps(//... except '//external:all-targets', %s)", targetQuery);
55+
String query = query = String.format("rdeps(//... except '//external:all-targets', %s)", targetQuery);
5656
if (avoidQuery != null) {
5757
query = String.format("(%s) except (%s)", query, avoidQuery);
5858
}

‎src/main/java/com/bazel_diff/TargetHashingClient.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
interface TargetHashingClient {
1111
Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths) throws IOException, NoSuchAlgorithmException;
1212
Map<String, String> hashAllBazelTargetsAndSourcefiles() throws IOException, NoSuchAlgorithmException;
13-
Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes, String avoidQuery) throws IOException;
13+
Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes, String avoidQuery, Boolean hashAllTargets) throws IOException;
1414
}
1515

1616
class TargetHashingClientImpl implements TargetHashingClient {
@@ -36,7 +36,8 @@ public Map<String, String> hashAllBazelTargetsAndSourcefiles() throws IOExceptio
3636
public Set<String> getImpactedTargets(
3737
Map<String, String> startHashes,
3838
Map<String, String> endHashes,
39-
String avoidQuery)
39+
String avoidQuery,
40+
Boolean hashAllTargets)
4041
throws IOException {
4142
Set<String> impactedTargets = new HashSet<>();
4243
for (Map.Entry<String,String> entry : endHashes.entrySet()) {
@@ -45,6 +46,9 @@ public Set<String> getImpactedTargets(
4546
impactedTargets.add(entry.getKey());
4647
}
4748
}
49+
if (hashAllTargets != null && hashAllTargets && avoidQuery == null) {
50+
return impactedTargets;
51+
}
4852
return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery);
4953
}
5054

‎src/main/java/com/bazel_diff/main.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ class BazelDiff implements Callable<Integer> {
158158
@Option(names = {"-o", "--output"}, scope = ScopeType.LOCAL, description = "Filepath to write the impacted Bazel targets to, newline separated")
159159
File outputPath;
160160

161+
@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")
162+
Boolean hashAllSourcefiles;
163+
161164
@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")
162165
String avoidQuery;
163166

@@ -207,7 +210,7 @@ public Integer call() throws IOException {
207210
Map<String, String > gsonHash = new HashMap<>();
208211
Map<String, String> startingHashes = gson.fromJson(startingFileReader, gsonHash.getClass());
209212
Map<String, String> finalHashes = gson.fromJson(finalFileReader, gsonHash.getClass());
210-
Set<String> impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery);
213+
Set<String> impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery, hashAllSourcefiles);
211214
try {
212215
FileWriter myWriter = new FileWriter(outputPath);
213216
myWriter.write(impactedTargets.stream().collect(Collectors.joining(System.lineSeparator())));

‎test/java/com/bazel_diff/TargetHashingClientImplTests.java‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void getImpactedTargets() throws IOException {
129129
hash2.put("rule1", "differentrule1hash");
130130
hash2.put("rule2", "rule2hash");
131131
hash2.put("rule3", "rule3hash");
132-
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null);
132+
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null, false);
133133
Set<String> expectedSet = new HashSet<>();
134134
expectedSet.add("rule1");
135135
expectedSet.add("rule3");
@@ -149,7 +149,46 @@ public void getImpactedTargets_withAvoidQuery() throws IOException {
149149
hash2.put("rule1", "differentrule1hash");
150150
hash2.put("rule2", "rule2hash");
151151
hash2.put("rule3", "rule3hash");
152-
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query");
152+
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", false);
153+
Set<String> expectedSet = new HashSet<>();
154+
expectedSet.add("rule1");
155+
assertEquals(expectedSet, impactedTargets);
156+
}
157+
158+
@Test
159+
public void getImpactedTargets_withHashAllTargets() throws IOException {
160+
when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject())).thenReturn(
161+
new HashSet<>(Arrays.asList("rule1"))
162+
);
163+
TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock);
164+
Map<String, String> hash1 = new HashMap<>();
165+
hash1.put("rule1", "rule1hash");
166+
hash1.put("rule2", "rule2hash");
167+
Map<String, String> hash2 = new HashMap<>();
168+
hash2.put("rule1", "differentrule1hash");
169+
hash2.put("rule2", "rule2hash");
170+
hash2.put("rule3", "rule3hash");
171+
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, null, true);
172+
Set<String> expectedSet = new HashSet<>();
173+
expectedSet.add("rule1");
174+
expectedSet.add("rule3");
175+
assertEquals(expectedSet, impactedTargets);
176+
}
177+
178+
@Test
179+
public void getImpactedTargets_withHashAllTargets_withAvoidQuery() throws IOException {
180+
when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"))).thenReturn(
181+
new HashSet<>(Arrays.asList("rule1"))
182+
);
183+
TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock);
184+
Map<String, String> hash1 = new HashMap<>();
185+
hash1.put("rule1", "rule1hash");
186+
hash1.put("rule2", "rule2hash");
187+
Map<String, String> hash2 = new HashMap<>();
188+
hash2.put("rule1", "differentrule1hash");
189+
hash2.put("rule2", "rule2hash");
190+
hash2.put("rule3", "rule3hash");
191+
Set<String> impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", true);
153192
Set<String> expectedSet = new HashSet<>();
154193
expectedSet.add("rule1");
155194
assertEquals(expectedSet, impactedTargets);

0 commit comments

Comments
 (0)