Merge pull request #72 from Tinder/remove_modified_filepaths

tinder-maxwellelliott · web-flow · commit 447405132bf8 · 2021-06-24T16:19:21.000-07:00
Remove modified filepaths and avoid query options
diff --git a/README.md b/README.md
@@ -60,27 +60,22 @@ Open `bazel-diff-example.sh` to see how this is implemented. This is purely an e
 
 `bazel-diff` works as follows
 
-* The `modified-filepaths` command is used to fetch the modified filepaths between two Git revisions, we write this list of modified filepaths to a file.
+* The previous revision is checked out, then we run `generate-hashes`. This gives us the hashmap representation for the entire Bazel graph, then we write this JSON to a file.
 
-* The previous revision is checked out, then we run `generate-hashes`. This gives us the hashmap representation for the entire Bazel graph, then we write this JSON to a file. We do not pass `modified-filepaths` here since it is the starting revision.
+* Next we checkout the initial revision, then we run `generate-hashes` and write that JSON to a file. Now we have our final hashmap representation for the Bazel graph.
 
-* Next we checkout the initial revision, then we run `generate-hashes` with the file output of `modified-filepaths` and write that JSON to a file. Now we have our final hashmap representation for the Bazel graph.
-
-* We run `bazel-diff` on the starting and final JSON hash filepaths to get our impacted set of targets. This impacted set of targets is written to a file. You can also pass the `-t` flag to only return tests
+* We run `bazel-diff` on the starting and final JSON hash filepaths to get our impacted set of targets. This impacted set of targets is written to a file.
 
 ## CLI Interface
 
 `bazel-diff` Command
 
 ```terminal
-Usage: bazel-diff [-hV] [-aq=<avoidQuery>] -b=<bazelPath>
-                  [-co=<bazelCommandOptions>] [-fh=<finalHashesJSONPath>]
-                  [-o=<outputPath>] [-sh=<startingHashesJSONPath>]
-                  [-so=<bazelStartupOptions>] -w=<workspacePath> [COMMAND]
+Usage: bazel-diff [-hV] -b=<bazelPath> [-co=<bazelCommandOptions>]
+                  [-fh=<finalHashesJSONPath>] [-o=<outputPath>]
+                  [-sh=<startingHashesJSONPath>] [-so=<bazelStartupOptions>]
+                  -w=<workspacePath> [COMMAND]
 Writes to a file the impacted targets between two Bazel graph JSON files
-      -aq, --avoid-query=<avoidQuery>
-                  A Bazel query string, any targets that pass this query will
-                    be removed from the returned set of targets
   -b, --bazelPath=<bazelPath>
                   Path to Bazel binary
       -co, --bazelCommandOptions=<bazelCommandOptions>
@@ -102,71 +97,39 @@ Writes to a file the impacted targets between two Bazel graph JSON files
   -V, --version   Print version information and exit.
   -w, --workspacePath=<workspacePath>
                   Path to Bazel workspace directory.
-```
-
-`modified-filepaths` Command
-
-```terminal
-Usage: bazel-diff modified-filepaths [-hV] -b=<bazelPath>
-                                     [-co=<bazelCommandOptions>]
-                                     [-so=<bazelStartupOptions>]
-                                     -w=<workspacePath> <startingGitRevision>
-                                     <endingGitRevision> <outputPath>
-Writes to the file the modified filepaths between two revisions.
-      <startingGitRevision> The starting Git revision, e.g. "HEAD^"
-      <endingGitRevision>   The final Git revision, e.g. "HEAD"
-      <outputPath>          Path that the list of modified files will be
-                              written to
-  -b, --bazelPath=<bazelPath>
-                            Path to Bazel binary
-      -co, --bazelCommandOptions=<bazelCommandOptions>
-                            Additional space separated Bazel command options
-                              used when invoking Bazel
-  -h, --help                Show this help message and exit.
-      -so, --bazelStartupOptions=<bazelStartupOptions>
-                            Additional space separated Bazel client startup
-                              options used when invoking Bazel
-  -V, --version             Print version information and exit.
-  -w, --workspacePath=<workspacePath>
-                            Path to Bazel workspace directory.
+Commands:
+  generate-hashes  Writes to a file the SHA256 hashes for each Bazel Target in
+                     the provided workspace.
 ```
 
 `generate-hashes` Command
 
 ```terminal
-Usage: bazel-diff generate-hashes [-m=<modifiedFilepaths> | -a] [-hV]
-                                  -b=<bazelPath> [-co=<bazelCommandOptions>]
+Usage: bazel-diff generate-hashes [-hV] -b=<bazelPath>
+                                  [-co=<bazelCommandOptions>]
                                   [-s=<seedFilepaths>]
                                   [-so=<bazelStartupOptions>]
                                   -w=<workspacePath> <outputPath>
 Writes to a file the SHA256 hashes for each Bazel Target in the provided
 workspace.
-      <outputPath>        The filepath to write the resulting JSON of
-                            dictionary target => SHA-256 values
-  -a, --all-sourcefiles   Experimental: Hash all sourcefile targets (instead of
-                            relying on --modifiedFilepaths), Warning:
-                            Performance may degrade from reading all source
-                            files
+      <outputPath>   The filepath to write the resulting JSON of dictionary
+                       target => SHA-256 values
   -b, --bazelPath=<bazelPath>
-                          Path to Bazel binary
+                     Path to Bazel binary
       -co, --bazelCommandOptions=<bazelCommandOptions>
-                          Additional space separated Bazel command options used
-                            when invoking Bazel
-  -h, --help              Show this help message and exit.
-  -m, --modifiedFilepaths=<modifiedFilepaths>
-                          The path to a file containing the list of modified
-                            filepaths in the workspace, you can use the
-                            'modified-filepaths' command to get this list
+                     Additional space separated Bazel command options used when
+                       invoking Bazel
+  -h, --help         Show this help message and exit.
   -s, --seed-filepaths=<seedFilepaths>
-                          A text file containing a newline separated list of
-                            filepaths, each of these filepaths will be read and
-                            used as a seed for all targets.
+                     A text file containing a newline separated list of
+                       filepaths, each of these filepaths will be read and used
+                       as a seed for all targets.
       -so, --bazelStartupOptions=<bazelStartupOptions>
-                          Additional space separated Bazel client startup
-                            options used when invoking Bazel
-  -V, --version           Print version information and exit.
+                     Additional space separated Bazel client startup options
+                       used when invoking Bazel
+  -V, --version      Print version information and exit.
   -w, --workspacePath=<workspacePath>
-                          Path to Bazel workspace directory.
+                     Path to Bazel workspace directory.
 ```
 
 ## Installing
@@ -205,7 +168,6 @@ now run the tool with
 bazel run //:bazel-diff
 ```
 
-
 > Note, in releases prior to 2.0.0 the value for the `main_class` attribute is just `BazelDiff`
 
 ### Run Via JAR Release
diff --git a/bazel-diff-example.sh b/bazel-diff-example.sh
@@ -25,27 +25,18 @@ $bazel_path run :bazel-diff $shared_flags --script_path="$bazel_diff"
 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 -a
+$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $starting_hashes_json
 
 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 $final_hashes_json -a
+$bazel_diff generate-hashes -w $workspace_path -b $bazel_path $final_hashes_json
 
 echo "Determining Impacted Targets"
-$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 -a --avoid-query "//... except tests(//...)"
+$bazel_diff -sh $starting_hashes_json -fh $final_hashes_json -w $workspace_path -b $bazel_path -o $impacted_targets_path
 
 IFS=$'\n' read -d '' -r -a impacted_targets < $impacted_targets_path
 formatted_impacted_targets=$(IFS=$'\n'; echo "${impacted_targets[*]}")
 echo "Impacted Targets between $previous_revision and $final_revision:"
 echo $formatted_impacted_targets
 echo ""
-
-IFS=$'\n' read -d '' -r -a impacted_test_targets < $impacted_test_targets_path
-formatted_impacted_test_targets=$(IFS=$'\n'; echo "${impacted_test_targets[*]}")
-echo "Impacted Test Targets between $previous_revision and $final_revision:"
-echo $formatted_impacted_test_targets
-echo ""
diff --git 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=last_downstream_green
+export USE_BAZEL_VERSION=4.1.0
 
 containsElement () {
   local e match="$1"
@@ -23,28 +23,19 @@ containsElement () {
 
 $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
+$bazel_path run :bazel-diff $shared_flags -- generate-hashes -w $workspace_path -b $bazel_path $final_hashes_json
 
-awk '{gsub(/:StringGenerator.java": \"\w+\"/,"modifiedhash");print}' $final_hashes_json > /dev/null
+final_tmp=$(mktemp)
+awk '{gsub(/32c874ad651f0dd234d712b7f1dce6b54aa36cf16380da763a8eb7bb7c1f9514/,"modifiedhash");print}' $final_hashes_json > $final_tmp
+cp $final_tmp $final_hashes_json
 
-$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', //...)"
+$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
 
 IFS=$'\n' read -d '' -r -a impacted_targets < $impacted_targets_path
-target1="//test/java/com/integration:bazel-diff-integration-test-lib"
-target2="//src/main/java/com/integration:bazel-diff-integration-lib"
-target3="//test/java/com/integration:bazel-diff-integration-tests"
-target4="//src/main/java/com/integration/submodule:Submodule"
-if containsElement $target1 "${impacted_targets[@]}" && \
-    containsElement $target2 "${impacted_targets[@]}" && \
-    containsElement $target3 "${impacted_targets[@]}"
+target1="//src/main/java/com/integration:StringGenerator.java"
+if containsElement $target1 "${impacted_targets[@]}"
 then
-    if containsElement $target4 "${impacted_targets[@]}"
-    then
-        echo "FAILURE Incorrect impacted targets: ${impacted_targets[@]}"
-        exit 1
-    else
-        echo "SUCCESS: Correct impacted targets: ${impacted_targets[@]}"
-    fi
+    echo "SUCCESS: Correct impacted targets: ${impacted_targets[@]}"
 else
     echo "FAILURE: Incorrect impacted targets: ${impacted_targets[@]}"
     exit 1
diff --git a/src/main/java/com/bazel_diff/BazelClient.java b/src/main/java/com/bazel_diff/BazelClient.java
@@ -20,7 +20,6 @@
 
 interface BazelClient {
     List<BazelTarget> queryAllTargets() throws IOException;
-    Set<String> queryForImpactedTargets(Set<String> impactedTargets, String avoidQuery, String universeQuery) throws IOException;
     Set<BazelSourceFileTarget> convertFilepathsToSourceTargets(Set<Path> filepaths) throws IOException, NoSuchAlgorithmException;
     Set<BazelSourceFileTarget> queryAllSourcefileTargets() throws IOException, NoSuchAlgorithmException;
 }
@@ -46,25 +45,6 @@ public List<BazelTarget> queryAllTargets() throws IOException {
         return targets.stream().map( target -> new BazelTargetImpl(target)).collect(Collectors.toList());
     }
 
-    @Override
-    public Set<String> queryForImpactedTargets(Set<String> impactedTargets, String avoidQuery, String universeQuery) throws IOException {
-        Set<String> impactedTargetNames = new HashSet<>();
-        String targetQuery = impactedTargets.stream()
-                                            .map(target -> String.format("'%s'", target))
-                                            .collect(Collectors.joining(" + "));
-        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);
-        }
-        List<Build.Target> targets = performBazelQuery(query);
-        for (Build.Target target : targets) {
-            if (target.hasRule()) {
-                impactedTargetNames.add(target.getRule().getName());
-            }
-        }
-        return impactedTargetNames;
-    }
-
     @Override
     public Set<BazelSourceFileTarget> convertFilepathsToSourceTargets(Set<Path> filepaths) throws IOException, NoSuchAlgorithmException {
         Set<BazelSourceFileTarget> sourceTargets = new HashSet<>();
diff --git a/src/main/java/com/bazel_diff/TargetHashingClient.java b/src/main/java/com/bazel_diff/TargetHashingClient.java
@@ -10,9 +10,8 @@
 import com.google.common.primitives.Bytes;
 
 interface TargetHashingClient {
-    Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths, Set<Path> seedFilepaths) throws IOException, NoSuchAlgorithmException;
     Map<String, String> hashAllBazelTargetsAndSourcefiles(Set<Path> seedFilepaths) throws IOException, NoSuchAlgorithmException;
-    Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes, String avoidQuery, String universeQuery, Boolean hashAllTargets) throws IOException;
+    Set<String> getImpactedTargets(Map<String, String> startHashes, Map<String, String> endHashes) throws IOException;
 }
 
 class TargetHashingClientImpl implements TargetHashingClient {
@@ -24,12 +23,6 @@ class TargetHashingClientImpl implements TargetHashingClient {
         this.files = files;
     }
 
-    @Override
-    public Map<String, String> hashAllBazelTargets(Set<Path> modifiedFilepaths, Set<Path> seedFilepaths) throws IOException, NoSuchAlgorithmException {
-        Set<BazelSourceFileTarget> bazelSourcefileTargets = bazelClient.convertFilepathsToSourceTargets(modifiedFilepaths);
-        return hashAllTargets(createSeedForFilepaths(seedFilepaths), bazelSourcefileTargets);
-    }
-
     @Override
     public Map<String, String> hashAllBazelTargetsAndSourcefiles(Set<Path> seedFilepaths) throws IOException, NoSuchAlgorithmException {
         Set<BazelSourceFileTarget> bazelSourcefileTargets = bazelClient.queryAllSourcefileTargets();
@@ -39,10 +32,7 @@ public Map<String, String> hashAllBazelTargetsAndSourcefiles(Set<Path> seedFilep
     @Override
     public Set<String> getImpactedTargets(
         Map<String, String> startHashes,
-        Map<String, String> endHashes,
-        String avoidQuery,
-        String universeQuery,
-        Boolean hashAllTargets)
+        Map<String, String> endHashes)
     throws IOException {
         Set<String> impactedTargets = new HashSet<>();
         for (Map.Entry<String,String> entry : endHashes.entrySet()) {
@@ -51,10 +41,7 @@ public Set<String> getImpactedTargets(
                 impactedTargets.add(entry.getKey());
             }
         }
-        if (hashAllTargets != null && hashAllTargets && avoidQuery == null) {
-            return impactedTargets;
-        }
-        return bazelClient.queryForImpactedTargets(impactedTargets, avoidQuery, universeQuery);
+        return impactedTargets;
     }
 
     private byte[] createDigestForTarget(
diff --git a/src/main/java/com/bazel_diff/main.java b/src/main/java/com/bazel_diff/main.java
diff --git a/test/java/com/bazel_diff/TargetHashingClientImplTests.java b/test/java/com/bazel_diff/TargetHashingClientImplTests.java
