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.

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

Avoid creating symlinks that point out of output TreeArtifact #313

Merged

Conversation

anguslees
Copy link
Contributor

cp -r copies the symlinks, not the destination of symlinks. This results in
a TreeArtifact with symlinks to content outside the TreeArtifact, which is not
well defined in bazel1. At least bazel 7.1.1's linux-sandbox (bazel
--spawn_strategy=sandboxed on Linux) does not follow the symlinks and destroys
the temporary directory containing the actual files after the action, leaving a
TreeArtifact of dangling symlinks.

cp -rL chases the symlinks and creates a TreeArtifact of regular files. This
has the desired behaviour in all bazel versions/sandboxes.

Footnotes

  1. See discussion in https://github.com/bazelbuild/bazel/issues/15454. It is
    unclear whether we want to resolve these symlinks or leave them dangling, and
    behaviour varies across bazel versions and execution environment.

`cp -r` copies the _symlinks_, not the destination of symlinks.  This results in
a TreeArtifact with symlinks to content _outside_ the TreeArtifact, which is not
well defined in bazel[^1].  At least bazel 7.1.1's linux-sandbox (bazel
--spawn_strategy=sandboxed on Linux) does not follow the symlinks and destroys
the temporary directory containing the actual files after the action, leaving a
TreeArtifact of dangling symlinks.

`cp -rL` chases the symlinks and creates a TreeArtifact of regular files.  This
has the desired behaviour in all bazel versions/sandboxes.

[^1]: See discussion in bazelbuild/bazel#15454.  It is
unclear whether we _want_ to resolve these symlinks or leave them dangling, and
behaviour varies across bazel versions and execution environment.
@aaliddell aaliddell merged commit 851143f into rules-proto-grpc:master May 13, 2024
3 checks passed
@aaliddell
Copy link
Member

Excellent thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants