Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Inject copies of remote files with BwoB on Windows without symlink support #24911

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Inject copies of remote files with BwoB on Windows without symlink support #24911

wants to merge 3 commits into from
+72 −12

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jan 13, 2025 •
edited
Loading

Fixes #21747

@fmeum fmeum requested a review from a team as a code owner January 13, 2025 15:31
@fmeum fmeum marked this pull request as draft January 13, 2025 15:31
@fmeum fmeum removed the request for review from a team January 13, 2025 15:31
@github-actions github-actions bot added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 13, 2025
@fmeum fmeum force-pushed the 21747-windows-symlink-as-copy branch 2 times, most recently from c8dd289 to 767e12e Compare January 13, 2025 21:17
@fmeum fmeum force-pushed the 21747-windows-symlink-as-copy branch from 767e12e to cd957cb Compare January 13, 2025 21:19
@fmeum fmeum changed the title TMP: Reproduce Windows issue Inject copies instead of creating junctions to missing files with BwoB on Windows Jan 13, 2025
@fmeum fmeum changed the title Inject copies instead of creating junctions to missing files with BwoB on Windows Inject copies of remote files with BwoB on Windows without symlinks support Jan 13, 2025
@fmeum fmeum requested a review from tjgq January 13, 2025 21:20
@fmeum fmeum marked this pull request as ready for review January 13, 2025 21:20
@fmeum fmeum requested a review from a team as a code owner January 13, 2025 21:20
@fmeum fmeum requested review from gregestren and removed request for a team and gregestren January 13, 2025 21:20
@github-actions github-actions bot added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Jan 13, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 14, 2025

I didn't manage to reproduce the race in the issue, so this is a speculative fix.

tjgq
tjgq requested changes Jan 14, 2025
View reviewed changes
src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java Outdated Show resolved Hide resolved
src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test case verifying that a non-toplevel output materialized by a symlink is never downloaded?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a test and a few assertions, please let me know if that isn't what you wanted to see.

src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java Outdated Show resolved Hide resolved
@fmeum fmeum changed the title Inject copies of remote files with BwoB on Windows without symlinks support Inject copies of remote files with BwoB on Windows without symlink support Jan 16, 2025
@fmeum fmeum requested a review from tjgq January 16, 2025 10:36
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 16, 2025

@bazel-io fork 7.5.0

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 16, 2025

@bazel-io fork 8.1.0

This was referenced Jan 16, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjgq tjgq Awaiting requested review from tjgq

Assignees
No one assigned
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ctx.actions.symlink + building without the bytes misbehaves on Windows
2 participants
@fmeum @tjgq