--experimental_remote_download_regex doesn't work with a cache in 6.0.0rc4

[BalestraPatrick]

Description of the bug:

The flag --experimental_remote_download_regex which is used by some IDE integrations doesn't work with --disk_cache and --remote_download_toplevel in 6.0.0rc2. I was able to verify that the same behavior works great in 5.3.0, but suddenly broke sometime before 6.0.0rc2. I couldn't bisect exactly due to some breakages between Bazel and rules_apple, but I don't think there are many changes that could impact it.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

This is a bit tricky to reproduce, but it goes as following:

• Use the sample project at https://github.com/BalestraPatrick/rules_xcodeproj/tree/repro-disk-cache-bug.
• cd examples/integration and generate the project with bazelisk run //:xcodeproj-sim_arm64.
• Run a build with bazelisk run //:xcodeproj-sim_arm64 --cpu=ios_sim_arm64 -- --generator_output_groups=all_targets 'build --remote_download_toplevel --cpu=ios_sim_arm64 --apple_platform_type=ios --disk_cache=~/.test-cache' (Xcode 14.0 might be required).
• Open iOSApp.swift and make some modifications (simply modify the existing Swift.print statement). Run a no-op build as a test. This is because the errors in importing the indexstore (which is what we download through --remote_download_regex) will show up only on the following build. Everything should work fine and no errors should be seen because the example project is using Bazel 5.3.0. You can verify that the indexstore has contents in something like /Users/you/src/rules_xcodeproj/examples/integration/bazel-output-base/rules_xcodeproj/build_output_base/execroot/__main__/bazel-out/ios-sim_arm64-min15.0-applebin_ios-ios_sim_arm64-dbg-ST-2ccbe0f541dd/bin/iOSApp/Source/iOSApp.library.indexstore/.
• Change .bazelversion to 6.0.0rc2 (or likely earlier revisions will reproduce the issue too).
• Repeat the earlier steps to trigger an incremental build. The indexstore directory will contain the right files as expected. Then do a no-op build which will be fetched from the disk cache. If you now list the contents of the same directory as before, you'll see that the indexstore directory is empty. Instead, it should contain actual files like the build with 5.3.0 or any other build that doesn't hit the disk cache.

My assumption is that the values specified in --remote_download_regex (here) are not respected when an action is fetched from the disk cache. When the action is executed locally or fetched from a remote cache, everything seems to work as expected.

Which operating system are you running Bazel on?

macOS 13.0

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

6.0.0rc2

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[meteorcloudy]

@bazel-io fork 6.0.0

[meteorcloudy]

I assume this is also reproducible with 6.0.0rc4?

[BalestraPatrick]

@meteorcloudy Looks like it from my tests.

[meteorcloudy]

@BalestraPatrick Thanks, @coeuvre is on it now

[coeuvre]

I can reproduce this error with 6.0.0rc2 but cannot with 6.0.0rc4 nor HEAD. Maybe there are some changes in between that already fix this issue.

@BalestraPatrick Can you double check whether it is still the case for 6.0.0rc4?

[coeuvre]

6.0.0rc3 doesn't have this issue neither. it's likely fixed by 8818a57.

[meteorcloudy]

6.0.0rc3 doesn't have this issue neither. it's likely fixed by 8818a57.

@BalestraPatrick Can you help confirm this? Maybe we'll be able to release 6.0 on time after all?

[brentleyjones]

Seems like this example is fixed by rc4, but his internal project is still broken on rc4. We are trying to adjust the repro to account for the remaining failure.

[BalestraPatrick]

I tried running bazelisk clean --expunge and removing the disk cache manually but the problem persisted. What fixed it when upgrading from rc2 to rc4 was completely disabling the disk cache, running a build without it, and then re-adding the disk cache did not every reproduce the problem even in our internal project. I'm not sure if this is helpful.

[coeuvre]

I tried running bazelisk clean --expunge and removing the disk cache manually but the problem persisted.

Which version did you try with?

IIUC, the issue was when we do an no-op incremental build, the content inside indexstore are deleted. If we do a normal incremental build i.e. immediately after edit the source file, there are content inside indexstore even with 6.0.0rc2.

[coeuvre]

I tried running bazelisk clean --expunge and removing the disk cache manually but the problem persisted.

If you run the command inside examples/integration, it only clean the workspace for the runner script, not the one for the actual iOS target.

[coeuvre]

What fixed it when upgrading from rc2 to rc4 was completely disabling the disk cache, running a build without it, and then re-adding the disk cache did not every reproduce the problem even in our internal project.

The reason a following build didn't download the content from disk cache is because of action cache, the generating action didn't get chance to run, so the outputs are not downloaded.

[brentleyjones]

He did the clean targeting the nested output base as well, not just the runner. The new bug he is reporting is that even after an expunge of that output base the indexstore directories were coming up empty.

[brentleyjones]

Either way I think this exact bug is fixed. There might be another bug related to incremental state.

[BalestraPatrick]

@coeuvre Yeah, I was testing with bazelisk run //:xcodeproj -- "clean --expunge" to clean the output base that the runner script uses. So from what I understand, the problem should be fixed. I will let you know if I'm able to reproduce the issue in rc4 or later, but feel free not to consider this a release blocker now. Thanks for the help!

[coeuvre]

Thanks for confirming the fix. Please create another issue to track the related bug. Closing this now.

[meteorcloudy]

@BalestraPatrick Then we'll move forward with the 6.0 release, and if we identify the other potential incremental bug, we can fix it in later minor/patch releases.

[brentleyjones]

@meteorcloudy Patrick told me he repro'd this again with rc4. We are trying to get an example repro together.

[brentleyjones]

I repro'ed as well. Getting some steps together. Can we re-open the issue?

[brentleyjones]

I'm still trying to distill this down to something specific, but it's not limited to the disk cache, I've repro'ed with a remote cache as well.

[brentleyjones]

Repro steps:

1. Check out the repro/regex_download_bug branch at https://github.com/buildbuddy-io/rules_xcodeproj
2. cd examples/integration
3. Close Xcode if open
4. rm -rf ~/.test-bazel-disk-cache/
5. rm -rf bazel-output-base
6. bazelisk run //:xcodeproj-sim_arm64
7. xed .
8. Build iOSApp in Xcode
9. Edit iOSApp/Source/iOSApp.swift, changing the number in the string to 2
10. Build iOSApp in Xcode
11. Edit iOSApp/Source/iOSApp.swift, changing the number in the string to 1 (so we get a cache hit)
12. Build iOSApp. See missing .indexstore error(s)

The above uses Xcode for steps 6-12, since it's not dependent on Starlark Hash paths, but they can be replaced with this instead:

6. bazelisk run //:xcodeproj-sim_arm64 -- --generator_output_groups=all_targets 'build'
7. Check to confirm that bazel-output-base/rules_xcodeproj/build_output_base/execroot/__main__/bazel-out/ios-sim_arm64-min15.0-applebin_ios-ios_sim_arm64-dbg-ST-9f9dcf014ff4/bin/iOSApp/Source/iOSApp.library.indexstore/v5 (or similar path) exists
8. Edit iOSApp/Source/iOSApp.swift, changing the number in the string to 2
9. Check to confirm that bazel-output-base/rules_xcodeproj/build_output_base/execroot/__main__/bazel-out/ios-sim_arm64-min15.0-applebin_ios-ios_sim_arm64-dbg-ST-9f9dcf014ff4/bin/iOSApp/Source/iOSApp.library.indexstore/v5 (or similar path) exists
10. Edit iOSApp/Source/iOSApp.swift, changing the number in the string to 1 (so we get a cache hit)
11. bazelisk run //:xcodeproj-sim_arm64 -- --generator_output_groups=all_targets 'build'
12. Confirm that bazel-output-base/rules_xcodeproj/build_output_base/execroot/__main__/bazel-out/ios-sim_arm64-min15.0-applebin_ios-ios_sim_arm64-dbg-ST-9f9dcf014ff4/bin/iOSApp/Source/iOSApp.library.indexstore/v5 (or similar path) does not exist

This repro uses the disk cache for ease of testing, but the bug applies to a remote cache as well.

[coeuvre]

Thanks for the repro, i am looking into this issue.

[coeuvre]

I found the root cause: we didn't match regex with the files inside tree artifact.

Detailed explaination:

1. iOSApp.library.indexstore is a tree artifact.
2. After we edited iOSApp/Source/iOSApp.swift. In the following build, Bazel will rerun the action that generate iOSApp.library.indexstore.
3. Before rerun, Bazel deletes iOSApp.library.indexstore.
4. If we couldn't hit the disk/remote cache, the action is executed locally. iOSApp.library.indexstore is correctly generated.
5. If we can hit the cache, files inside iOSApp.library.indexstore are not downloaded because the regex is .*\.indexstore/.* and we only match it with iOSApp.library.indexstore not the files inside it.

Working on the fix.