Impacted targets always empty

[BalestraPatrick]

Hello!

I'm trying out the example script (using the latest version with the fix released in https://github.com/Tinder/bazel-diff/releases/tag/2.1.1), but the /tmp/impacted_targets.txt` file is always empty in my case. My test case contains only two commits. So commit A has some changes and commit B has some other changes in another package.

I then run ./bazel-diff-example.sh ~/src/myrepo bazelisk A-SHA B-SHA .

Here's the result:

• /tmp/modified_filepaths.txt correctly contains the paths of the files changed in commit B (8 files were modified in my example).
• /tmp/starting_hashes.json correctly contains some hashes.
• /tmp/final_hashes.json contains exactly the same hashes as the starting json, this seems wrong to me.
• /tmp/impacted_targets.txt is empty.

I'm not really sure what could be the problem. I tried to run bazel-diff from source and adding some log statements and I see that the impacted targets array is empty, so something must be wrong in the diffing or computation of the hashes. Did you ever see something like this? When I run bazelisk query '//external:all-targets' + '//...:all-targets', I can see the target that I would expected to be run with the changes in commit B.

[tinder-maxwellelliott]

Hello there @BalestraPatrick,

I am having issues replicating this locally, here is the output from me running bazel-diff on my local test environment (1 modified file, 1 modified non-Bazel tracked file, impacted file affects a test)

./bazel-diff-example.sh /Users/maxwellelliott/Tinder/bazel_ios_lab bazelisk "HEAD^" "HEAD"
Starting local Bazel server and connecting to it...
Modified Filepaths:
somefile.txt BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule/Sources/AddqevpgoqrfkiouumzjClass.swift

Generating Hashes for Revision 'HEAD^'
Generating Hashes for Revision 'HEAD'
Determining Impacted Targets
Determining Impacted Test Targets
Impacted Targets between HEAD^ and HEAD:
//BazeliOSLab/BazeliOSLab:MyAppLib //BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModuleTestLib //BazeliOSLab/BazeliOSLab:MyApp //BazeliOSLab/BazeliOSLab:MyApp.swift_runtime_linkopts //BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModule //BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModuleUnitTests.__internal__.__test_bundle //BazeliOSLab/BazeliOSLab:MyApp.__internal__.apple_binary //BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModuleUnitTests.__internal__.__test_bundle.swift_runtime_linkopts //BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModuleUnitTests

Impacted Test Targets between HEAD^ and HEAD:
//BazeliOSLab/Modules/AddqevpgoqrfkiouumzjModule:AddqevpgoqrfkiouumzjModuleUnitTests

Some questions:

1. Is the Git repo currently in a clean state and on the starting revision SHA (i.e. not detached)?
2. What is the output from running the example script with logging enabled? (Uncomment line 21)
3. Are you running this with branch names or raw Git Shas?

[BalestraPatrick]

@tinder-maxwellelliott Yes, I'm in a clean state. For some reason, it started working now after updating to 2.1.1 (I was on 2.1.0). This shouldn't really matter since I don't think there are changes that could affect this behavior between the two versions, but I'll continue to keep an eye on this issue. Which brings me to an issue I discovered which could be the culprit.

In testing the tool a bit more with different change sets, I noticed that for some filenames it doesn't work and debugging this even more sent me down a very dark rabbit hole.

Here's my findings.

• bazel-diff/src/main/java/com/bazel_diff/BazelClient.java

  Line 43 in 75fcaee

  public List<BazelTarget> queryAllTargets() throws IOException {

  in some cases fails. I discovered that for some specific file names that are in my modifiedFilePaths this fails.
• I reproduced this with filenames that contains "weird" characters, such as a +. For example, we have an Objective-C category file which is named "FooBar+NSError.m".
• This causes the bazel query to be in the format of something like (simplified a bit): bazelisk --bazelrc=/dev/null query --order_output=no --keep_going "redacted/redacted/FooBar+NSError.m which fails with something like ERROR: Skipping 'redacted/redacted/FooBar': no such target '//:redacted/redacted/FooBar': target 'redacted/redacted/FooBar' not declared in package '' defined by /Users/patrickb/Desktop/bazel-diff/BUILD.
• It seems like bazel-diff should handle properly these type of filenames, but I'm not even sure if there's a way for bazel to do this since I found this issue: Allow any characters in filenames / labels bazelbuild/bazel#374

What do you suggest doing? I think a good approach would be to surface the errors of the bazel query to the actual tool instead of just skipping the digest computation and ending up skipping the target. This is for safety reasons. If bazel-diff can't computer the digest, it should force the target to be ran (and thus output it in the impacted_targets.txt possibly with a warning) since we can't figure out if we should skip it or not.

[BalestraPatrick]

Looks like quoting the filename in the temp file passed as --query_file should work around my specific problem. Submitted a fix in #43.