From c2ea71f8cc3f4bcb980184029b4bea69ad47e584 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Tue, 1 Jun 2021 18:27:24 -0700 Subject: [PATCH 1/2] Support Universe Query in Bazel-Diff command Allows users to specify a universe query when running the bazel-diff command. This universe query is used when we execute an rdeps query. This flag is ignored when using the hash all filepaths option --- integration/WORKSPACE | 29 ------------------- integration/integration_test.sh | 11 ++++--- src/main/java/com/bazel_diff/BazelClient.java | 6 ++-- .../com/bazel_diff/TargetHashingClient.java | 5 ++-- src/main/java/com/bazel_diff/main.java | 23 ++++++++++++--- .../TargetHashingClientImplTests.java | 16 +++++----- 6 files changed, 38 insertions(+), 52 deletions(-) diff --git a/integration/WORKSPACE b/integration/WORKSPACE index 95a4a7e..71610bb 100644 --- a/integration/WORKSPACE +++ b/integration/WORKSPACE @@ -19,32 +19,3 @@ maven_install( "https://jcenter.bintray.com/", ] ) - -http_archive( - name = "io_bazel_rules_docker", - sha256 = "1698624e878b0607052ae6131aa216d45ebb63871ec497f26c67455b34119c80", - strip_prefix = "rules_docker-0.15.0", - urls = ["https://github.com/bazelbuild/rules_docker/releases/download/v0.15.0/rules_docker-v0.15.0.tar.gz"], -) - -load( - "@io_bazel_rules_docker//repositories:repositories.bzl", - container_repositories = "repositories", -) -container_repositories() - -load("@io_bazel_rules_docker//repositories:deps.bzl", container_deps = "deps") - -container_deps() - -load( - "@io_bazel_rules_docker//container:container.bzl", - "container_pull", -) - -container_pull( - name = "openjdk_11_slim", - digest = "sha256:28b59dc9a129c75349418e3b75508fd8eef3b33dd7d796079d1d19445d907776", - registry = "index.docker.io", - repository = "library/openjdk", -) diff --git a/integration/integration_test.sh b/integration/integration_test.sh index b647142..a504784 100755 --- a/integration/integration_test.sh +++ b/integration/integration_test.sh @@ -10,10 +10,9 @@ modified_filepaths_output="$workspace_path/modified_filepaths.txt" starting_hashes_json="$output_dir/starting_hashes.json" final_hashes_json="$output_dir/final_hashes.json" impacted_targets_path="$output_dir/impacted_targets.txt" -shared_flags="--config=verbose" -command_options="--incompatible_restrict_string_escapes=false" +shared_flags="" -export USE_BAZEL_VERSION=last_downstream_green +export USE_BAZEL_VERSION=4.0.0 containsElement () { local e match="$1" @@ -22,13 +21,13 @@ containsElement () { return 1 } -$bazel_path run :bazel-diff $shared_flags -- generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json -co $command_options +$bazel_path run :bazel-diff $shared_flags -- generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json -$bazel_path run :bazel-diff $shared_flags -- generate-hashes -w $workspace_path -b $bazel_path -m $modified_filepaths_output $final_hashes_json -co $command_options +$bazel_path run :bazel-diff $shared_flags -- generate-hashes -w $workspace_path -b $bazel_path -m $modified_filepaths_output $final_hashes_json awk '{gsub(/:StringGenerator.java": \"\w+\"/,"modifiedhash");print}' $final_hashes_json > /dev/null -$bazel_path run :bazel-diff $shared_flags -- -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path -aq "attr('tags', 'manual', //...)" -co $command_options +$bazel_path run :bazel-diff $shared_flags -- -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path -aq "attr('tags', 'manual', //...)" IFS=$'\n' read -d '' -r -a impacted_targets < $impacted_targets_path target1="//test/java/com/integration:bazel-diff-integration-test-lib" diff --git a/src/main/java/com/bazel_diff/BazelClient.java b/src/main/java/com/bazel_diff/BazelClient.java index 5f62893..6504445 100644 --- a/src/main/java/com/bazel_diff/BazelClient.java +++ b/src/main/java/com/bazel_diff/BazelClient.java @@ -20,7 +20,7 @@ interface BazelClient { List queryAllTargets() throws IOException; - Set queryForImpactedTargets(Set impactedTargets, String avoidQuery) throws IOException; + Set queryForImpactedTargets(Set impactedTargets, String avoidQuery, String universeQuery) throws IOException; Set convertFilepathsToSourceTargets(Set filepaths) throws IOException, NoSuchAlgorithmException; Set queryAllSourcefileTargets() throws IOException, NoSuchAlgorithmException; } @@ -47,12 +47,12 @@ public List queryAllTargets() throws IOException { } @Override - public Set queryForImpactedTargets(Set impactedTargets, String avoidQuery) throws IOException { + public Set queryForImpactedTargets(Set impactedTargets, String avoidQuery, String universeQuery) throws IOException { Set impactedTargetNames = new HashSet<>(); String targetQuery = impactedTargets.stream() .map(target -> String.format("'%s'", target)) .collect(Collectors.joining(" + ")); - String query = query = String.format("rdeps(//... except '//external:all-targets', %s)", targetQuery); + String query = query = String.format("rdeps(%s except '//external:all-targets', %s)", universeQuery, targetQuery); if (avoidQuery != null) { query = String.format("(%s) except (%s)", query, avoidQuery); } diff --git a/src/main/java/com/bazel_diff/TargetHashingClient.java b/src/main/java/com/bazel_diff/TargetHashingClient.java index 3372ad9..ee4c6ba 100644 --- a/src/main/java/com/bazel_diff/TargetHashingClient.java +++ b/src/main/java/com/bazel_diff/TargetHashingClient.java @@ -12,7 +12,7 @@ interface TargetHashingClient { Map hashAllBazelTargets(Set modifiedFilepaths, Set seedFilepaths) throws IOException, NoSuchAlgorithmException; Map hashAllBazelTargetsAndSourcefiles(Set seedFilepaths) throws IOException, NoSuchAlgorithmException; - Set getImpactedTargets(Map startHashes, Map endHashes, String avoidQuery, Boolean hashAllTargets) throws IOException; + Set getImpactedTargets(Map startHashes, Map endHashes, String avoidQuery, String universeQuery, Boolean hashAllTargets) throws IOException; } class TargetHashingClientImpl implements TargetHashingClient { @@ -41,6 +41,7 @@ public Set getImpactedTargets( Map startHashes, Map endHashes, String avoidQuery, + String universeQuery, Boolean hashAllTargets) throws IOException { Set impactedTargets = new HashSet<>(); @@ -53,7 +54,7 @@ public Set getImpactedTargets( if (hashAllTargets != null && hashAllTargets && avoidQuery == null) { return impactedTargets; } - return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery); + return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery, universeQuery); } private byte[] createDigestForTarget( diff --git a/src/main/java/com/bazel_diff/main.java b/src/main/java/com/bazel_diff/main.java index 5f64acc..3f53b8d 100644 --- a/src/main/java/com/bazel_diff/main.java +++ b/src/main/java/com/bazel_diff/main.java @@ -153,6 +153,15 @@ public Integer call() { versionProvider = VersionProvider.class ) class BazelDiff implements Callable { + @ArgGroup(exclusive = true) + Exclusive exclusive; + static class Exclusive { + @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 = {"-u", "--universeQuery"}, description = "The universe query to use when executing `rdeps()` queries, use this to limit the search scope of impacted targets i.e. ignore external targets and such") + String universeRdepsQuery; + } @Option(names = {"-w", "--workspacePath"}, description = "Path to Bazel workspace directory.", scope = ScopeType.INHERIT, required = true) Path workspacePath; @@ -169,9 +178,6 @@ class BazelDiff implements Callable { @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; @@ -221,7 +227,16 @@ public Integer call() throws IOException { Map gsonHash = new HashMap<>(); Map startingHashes = gson.fromJson(startingFileReader, gsonHash.getClass()); Map finalHashes = gson.fromJson(finalFileReader, gsonHash.getClass()); - Set impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery, hashAllSourcefiles); + Boolean shouldHashAllSourceFiles = false; + String universeQuery = "//..."; + if (exclusive != null) { + if (exclusive.hashAllSourcefiles) { + shouldHashAllSourceFiles = exclusive.hashAllSourcefiles; + } else { + universeQuery = exclusive.universeRdepsQuery; + } + } + Set impactedTargets = hashingClient.getImpactedTargets(startingHashes, finalHashes, avoidQuery, universeQuery, shouldHashAllSourceFiles); try { FileWriter myWriter = new FileWriter(outputPath); myWriter.write(impactedTargets.stream().collect(Collectors.joining(System.lineSeparator()))); diff --git a/test/java/com/bazel_diff/TargetHashingClientImplTests.java b/test/java/com/bazel_diff/TargetHashingClientImplTests.java index 8031f3e..e85ee04 100644 --- a/test/java/com/bazel_diff/TargetHashingClientImplTests.java +++ b/test/java/com/bazel_diff/TargetHashingClientImplTests.java @@ -163,7 +163,7 @@ public void hashAllBazelTargets_sourceTargets_modifiedSources_seedFilepaths() th @Test public void getImpactedTargets() throws IOException { - when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject())).thenReturn( + when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject(), anyObject())).thenReturn( new HashSet<>(Arrays.asList("rule1", "rule3")) ); TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock, filesClientMock); @@ -174,7 +174,7 @@ public void getImpactedTargets() throws IOException { hash2.put("rule1", "differentrule1hash"); hash2.put("rule2", "rule2hash"); hash2.put("rule3", "rule3hash"); - Set impactedTargets = client.getImpactedTargets(hash1, hash2, null, false); + Set impactedTargets = client.getImpactedTargets(hash1, hash2, null, null, false); Set expectedSet = new HashSet<>(); expectedSet.add("rule1"); expectedSet.add("rule3"); @@ -183,7 +183,7 @@ public void getImpactedTargets() throws IOException { @Test public void getImpactedTargets_withAvoidQuery() throws IOException { - when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"))).thenReturn( + when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"), eq("universe_query"))).thenReturn( new HashSet<>(Arrays.asList("rule1")) ); TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock, filesClientMock); @@ -194,7 +194,7 @@ public void getImpactedTargets_withAvoidQuery() throws IOException { hash2.put("rule1", "differentrule1hash"); hash2.put("rule2", "rule2hash"); hash2.put("rule3", "rule3hash"); - Set impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", false); + Set impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", "universe_query", false); Set expectedSet = new HashSet<>(); expectedSet.add("rule1"); assertEquals(expectedSet, impactedTargets); @@ -202,7 +202,7 @@ public void getImpactedTargets_withAvoidQuery() throws IOException { @Test public void getImpactedTargets_withHashAllTargets() throws IOException { - when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject())).thenReturn( + when(bazelClientMock.queryForImpactedTargets(anySet(), anyObject(), anyObject())).thenReturn( new HashSet<>(Arrays.asList("rule1")) ); TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock, filesClientMock); @@ -213,7 +213,7 @@ public void getImpactedTargets_withHashAllTargets() throws IOException { hash2.put("rule1", "differentrule1hash"); hash2.put("rule2", "rule2hash"); hash2.put("rule3", "rule3hash"); - Set impactedTargets = client.getImpactedTargets(hash1, hash2, null, true); + Set impactedTargets = client.getImpactedTargets(hash1, hash2, null, null, true); Set expectedSet = new HashSet<>(); expectedSet.add("rule1"); expectedSet.add("rule3"); @@ -222,7 +222,7 @@ public void getImpactedTargets_withHashAllTargets() throws IOException { @Test public void getImpactedTargets_withHashAllTargets_withAvoidQuery() throws IOException { - when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"))).thenReturn( + when(bazelClientMock.queryForImpactedTargets(anySet(), eq("some_query"), anyObject())).thenReturn( new HashSet<>(Arrays.asList("rule1")) ); TargetHashingClientImpl client = new TargetHashingClientImpl(bazelClientMock, filesClientMock); @@ -233,7 +233,7 @@ public void getImpactedTargets_withHashAllTargets_withAvoidQuery() throws IOExce hash2.put("rule1", "differentrule1hash"); hash2.put("rule2", "rule2hash"); hash2.put("rule3", "rule3hash"); - Set impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", true); + Set impactedTargets = client.getImpactedTargets(hash1, hash2, "some_query", null, true); Set expectedSet = new HashSet<>(); expectedSet.add("rule1"); assertEquals(expectedSet, impactedTargets); From 5985c1648ad2fb84af76ffa092e409fa187db5dd Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Wed, 2 Jun 2021 16:15:52 -0700 Subject: [PATCH 2/2] revert change --- integration/integration_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/integration_test.sh b/integration/integration_test.sh index a504784..c55fa2b 100755 --- a/integration/integration_test.sh +++ b/integration/integration_test.sh @@ -12,7 +12,7 @@ final_hashes_json="$output_dir/final_hashes.json" impacted_targets_path="$output_dir/impacted_targets.txt" shared_flags="" -export USE_BAZEL_VERSION=4.0.0 +export USE_BAZEL_VERSION=last_downstream_green containsElement () { local e match="$1"