Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 15, 2025

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

@fmeum fmeum requested a review from Copilot April 15, 2025 19:55
Copy link

Copilot AI left a 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

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 16, 2025

Stacked on #25859

@fmeum fmeum requested a review from tjgq April 16, 2025 09:29
@fmeum fmeum marked this pull request as ready for review April 16, 2025 09:29
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 16, 2025
@iancha1992 iancha1992 added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Apr 16, 2025
Copy link
Contributor

@tjgq tjgq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, mind rebasing?

@fmeum fmeum force-pushed the 25834-fix-dangling-symlink branch from 3e2fb54 to 8c1c32f Compare May 9, 2025 14:11
@fmeum fmeum requested a review from tjgq May 9, 2025 14:11
@fmeum
Copy link
Collaborator Author

fmeum commented May 9, 2025

@tjgq I also added the missing BUILD target here so that the new test actually runs.

@fmeum fmeum force-pushed the 25834-fix-dangling-symlink branch from 8c1c32f to 11d791b Compare May 14, 2025 07:22
fmeum added 2 commits May 14, 2025 09:26
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
@fmeum fmeum force-pushed the 25834-fix-dangling-symlink branch from 11d791b to 2418d60 Compare May 14, 2025 07:27
@fmeum
Copy link
Collaborator Author

fmeum commented May 14, 2025

@tjgq I fixed the conflict, this is ready for another review.

@meteorcloudy
Copy link
Member

Can you please rebase again, the test failures should be fixed at HEAD.

@fmeum
Copy link
Collaborator Author

fmeum commented May 16, 2025

@meteorcloudy done!

@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 19, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented May 19, 2025

@bazel-io fork 8.3.0

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 20, 2025
fmeum added a commit to fmeum/bazel that referenced this pull request May 20, 2025
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
@fmeum fmeum deleted the 25834-fix-dangling-symlink branch May 28, 2025 13:00
fmeum added a commit to fmeum/bazel that referenced this pull request Oct 25, 2025
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)
dd-mergequeue bot pushed a commit to DataDog/datadog-agent that referenced this pull request Jan 15, 2026
### 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Core Skyframe, bazel query, BEP, options parsing, bazelrc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants