Skip to content

Skip executable checks for remote symlink inputs - #30316

Closed
tamird wants to merge 1 commit into
bazelbuild:masterfrom
tamird:tamird/fix-remote-executable-symlink
Closed

tamird wants to merge 1 commit into
bazelbuild:masterfrom
tamird:tamird/fix-remote-executable-symlink

Conversation

@tamird

@tamird tamird commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Executable symlink actions stat their input to verify permissions. A
remote-only derived output whose local parent directory was removed
cannot pass that check, even though it is made executable on download.

Skip the check only for unmaterialized derived outputs. Recreate the
missing input parent for action filesystems so post-action output
validation can resolve the new symlink without materializing its target.
Keep the executable checks for materialized outputs and remote
repository sources. Cover the shutdown and bazel-bin-deletion sequence
with a remote-execution integration test.

Fixes #26877.

@tamird
tamird marked this pull request as ready for review July 17, 2026 01:14
@tamird
tamird requested a review from a team as a code owner July 17, 2026 01:14
@tamird
tamird requested review from mai93 and removed request for a team July 17, 2026 01:14
@github-actions github-actions Bot added team-Configurability platforms, toolchains, cquery, select(), config transitions awaiting-review PR is awaiting review from an assigned reviewer labels Jul 17, 2026
@fmeum

fmeum commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

This would benefit from an integration test that reproduces the specific situation reported in the issue, not just unit tests.

@fmeum fmeum added team-Remote-Exec Issues and PRs for the Execution (Remote) team and removed team-Configurability platforms, toolchains, cquery, select(), config transitions labels Jul 17, 2026
@fmeum
fmeum requested review from a team and removed request for mai93 July 17, 2026 07:05
@tamird
tamird force-pushed the tamird/fix-remote-executable-symlink branch from 8db6909 to 364fda1 Compare July 17, 2026 12:13
@tamird

tamird commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Added the integration regression in 364fda1. It populates //:b, shuts down, deletes bazel-bin, then runs the cache-hit //:c through subdir/a. The regression exposed the missing ActionFS parent during output validation, so the fix now recreates that parent without materializing the target. The focused integration and unit tests pass.

@fmeum
fmeum requested a review from tjgq July 17, 2026 13:37
@github-actions github-actions Bot added the community-reviewed Reviewed by a trusted community contributor label Jul 17, 2026
@tamird
tamird force-pushed the tamird/fix-remote-executable-symlink branch from 364fda1 to 101dc0b Compare July 29, 2026 11:34
@tamird
tamird force-pushed the tamird/fix-remote-executable-symlink branch from 101dc0b to 34a50b7 Compare July 29, 2026 19:16
@tamird
tamird force-pushed the tamird/fix-remote-executable-symlink branch from 34a50b7 to 4c7d6b3 Compare August 21, 2026 15:10
@tamird

tamird commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@tjgq could you review the remote symlink-input fix and its unit/integration regressions (SymlinkAction.java plus tests, +108/−1)? fmeum approved this head on July 17. @bazelbuild/triage, this is still awaiting maintainer review.

— Codex, on Tamir’s behalf.

@tjgq

tjgq commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

I acknowledge the issue, but dislike the proposed fix. I need more time to think about alternatives.

Executable symlink actions stat their input to verify permissions. A
remote-only derived output whose local parent directory was removed
cannot pass that check, even though it is made executable on download.

Skip the check only for unmaterialized derived outputs. Recreate the
missing input parent for action filesystems so post-action output
validation can resolve the new symlink without materializing its target.
Keep the executable checks for materialized outputs and remote
repository sources. Cover the shutdown and bazel-bin-deletion sequence
with a remote-execution integration test.

Fixes bazelbuild#26877.
@tamird
tamird force-pushed the tamird/fix-remote-executable-symlink branch from 4c7d6b3 to 7e99522 Compare September 24, 2026 18:58
@tamird

tamird commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #30926 and its prerequisites.

@tamird tamird closed this Sep 24, 2026
@tamird
tamird deleted the tamird/fix-remote-executable-symlink branch September 24, 2026 22:42
@github-actions github-actions Bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed Reviewed by a trusted community contributor team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Symlink failure if bazel-bin cleaned between bazel runs, RBE enabled

3 participants