-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Include unresolved symlink target into the source directory hash #25864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Files not reviewed (1)
- src/test/java/com/google/devtools/build/lib/skyframe/BUILD: Language not supported
|
Stacked on #25859 |
tjgq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, mind rebasing?
3e2fb54 to
8c1c32f
Compare
|
@tjgq I also added the missing BUILD target here so that the new test actually runs. |
8c1c32f to
11d791b
Compare
This makes it so that changes to the targets of unresolved symlinks invalidate actions that depend on the source directory containing them. # Conflicts: # src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java
# Conflicts: # src/test/java/com/google/devtools/build/lib/skyframe/BUILD
11d791b to
2418d60
Compare
|
@tjgq I fixed the conflict, this is ready for another review. |
|
Can you please rebase again, the test failures should be fixed at HEAD. |
|
@meteorcloudy done! |
|
@bazel-io fork 8.3.0 |
This makes it so that changes to the targets of unresolved symlinks invalidate actions that depend on the source directory containing them. Work towards bazelbuild#25834 Closes bazelbuild#25864. PiperOrigin-RevId: 760968335 Change-Id: I09c250eb8e7d147006a4e6fdc33a372ce7277bc0
This makes it so that changes to the targets of unresolved symlinks invalidate actions that depend on the source directory containing them. Work towards bazelbuild#25834 Closes bazelbuild#25864. PiperOrigin-RevId: 760968335 Change-Id: I09c250eb8e7d147006a4e6fdc33a372ce7277bc0 (cherry picked from commit a3b5836)
### What does this PR do? Bump `bazel` version from [8.4.2](https://github.com/bazelbuild/bazel/releases/tag/8.4.2) to [8.5.1](https://github.com/bazelbuild/bazel/releases/tag/8.5.1). To make that happen, we also need to: - specify which `bash` to use on Windows when evaluating **repository** rules, for compatibility with bazelbuild/bazel#26927: ``` ERROR: /path/to/external/bazel_tools/tools/test/BUILD:23:10: in sh_binary rule @@bazel_tools//tools/test:collect_coverage: Error in fail: No suitable shell toolchain found: * if you are running Bazel on Windows, set the BAZEL_SH environment variable to the path of bash.exe ``` (actual job output: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/1349246422#L77) - bump `rules_go` from (implicit) [0.57.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.57.0) to (explicit) [0.59.0](https://github.com/bazel-contrib/rules_go/releases/tag/v0.59.0) for bazel-contrib/rules_go/pull/4493 to be compatible with bazelbuild/bazel/pull/27296: ``` Error: 'Facts' value has no field or method 'clear' ``` ### Motivation 1. this upgrade alone brings several improvements and bug fixes, among which: - bazelbuild/bazel#27117 - bazelbuild/bazel#27296 - bazelbuild/bazel#27417, covers: - bazelbuild/bazel#25834 - bazelbuild/bazel#25863 - bazelbuild/bazel#25864 - bazelbuild/bazel#25870 - bazelbuild/bazel#26698 - bazelbuild/bazel#27531 - bazelbuild/bazel#27560 - bazelbuild/bazel#27564 - bazelbuild/bazel#27705 - bazelbuild/bazel#27995 - bazelbuild/bazel#27996 2. `bazel` 9.0 is due soon, so better off favoring incremental bumps. ### Additional Notes Leveraging [the latter](bazelbuild/bazel#27996) might allow us to later reconsider whether we'd like to go back to the `--remote_cache` flag (instead of the `--remote_executor` flag that we had to switch to in #44962). Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
This makes it so that changes to the targets of unresolved symlinks invalidate actions that depend on the source directory containing them.
Work towards #25834