-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
[OBSOLETE] incompatible_remote_disallow_symlink_in_tree_artifact #18284
Labels
incompatible-change
Incompatible/breaking change
migration-ready
Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green
Comments
Hi @tjgq, I see that this flag is not breaking any downstream projects. Can we close this issue? |
I will send a CL to flip the flag, which will also close this. (In the meantime I've renamed the flag, amending the issue title to reflect it.) |
fweikert
pushed a commit
to fweikert/bazel
that referenced
this issue
May 25, 2023
The incompatible changes pipeline hasn't detected any breakages. Fixes bazelbuild#18284. PiperOrigin-RevId: 531485083 Change-Id: Ic48dd79a88fa24bd01a64bc8db40e74216f78f3c
copybara-service bot
pushed a commit
that referenced
this issue
Dec 4, 2023
… matching a locally produced one. This CL harmonizes the criteria for permitting a symlink for locally and remotely produced tree artifacts, namely: 1. A symlink to an absolute path is allowed. 2. A symlink to a relative path is allowed, as long as the target is inside the tree artifact. 3. The target path must exist (a consequence of tree artifacts transparently dereferencing symlinks; see also #15454). as enforced by TreeArtifactValue#visitTree (see https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java;l=585;drc=de9d1f59915a36229978d46b78a22c9e5389db92). The admonition about symlinks potentially compromising hermeticity is still valid, but there's little gain in making the behavior divergent between local and remote execution. I don't believe we can go in the other direction and extend the restriction to cover local execution, as it would break existing projects. The --incompatible_remote_disallow_symlink_in_tree_artifact flag is deleted. It was added at a time when symlink resolution was extremely unreliable when building without the bytes; the state of symlink handling has improved a lot since then. I'm deleting the flag entirely (as opposed to making it a no-op) because it was only introduced in the Bazel 7 tree and hasn't made it into a stable release yet. Makes #18284 obsolete. PiperOrigin-RevId: 587691220 Change-Id: I470a8fc523bdbb7577ad5a564b6b2c0acab17d7d
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 4, 2023
…rtifact, matching a locally produced one. This CL harmonizes the criteria for permitting a symlink for locally and remotely produced tree artifacts, namely: 1. A symlink to an absolute path is allowed. 2. A symlink to a relative path is allowed, as long as the target is inside the tree artifact. 3. The target path must exist (a consequence of tree artifacts transparently dereferencing symlinks; see also bazelbuild#15454). as enforced by TreeArtifactValue#visitTree (see https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java;l=585;drc=de9d1f59915a36229978d46b78a22c9e5389db92). The admonition about symlinks potentially compromising hermeticity is still valid, but there's little gain in making the behavior divergent between local and remote execution. I don't believe we can go in the other direction and extend the restriction to cover local execution, as it would break existing projects. The --incompatible_remote_disallow_symlink_in_tree_artifact flag is deleted. It was added at a time when symlink resolution was extremely unreliable when building without the bytes; the state of symlink handling has improved a lot since then. I'm deleting the flag entirely (as opposed to making it a no-op) because it was only introduced in the Bazel 7 tree and hasn't made it into a stable release yet. Makes bazelbuild#18284 obsolete. PiperOrigin-RevId: 587691220 Change-Id: I470a8fc523bdbb7577ad5a564b6b2c0acab17d7d
keertk
pushed a commit
that referenced
this issue
Dec 4, 2023
…rtifact, matching a locally produced one. (#20426) This CL harmonizes the criteria for permitting a symlink for locally and remotely produced tree artifacts, namely: 1. A symlink to an absolute path is allowed. 2. A symlink to a relative path is allowed, as long as the target is inside the tree artifact. 3. The target path must exist (a consequence of tree artifacts transparently dereferencing symlinks; see also #15454). as enforced by TreeArtifactValue#visitTree (see https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java;l=585;drc=de9d1f59915a36229978d46b78a22c9e5389db92). The admonition about symlinks potentially compromising hermeticity is still valid, but there's little gain in making the behavior divergent between local and remote execution. I don't believe we can go in the other direction and extend the restriction to cover local execution, as it would break existing projects. The --incompatible_remote_disallow_symlink_in_tree_artifact flag is deleted. It was added at a time when symlink resolution was extremely unreliable when building without the bytes; the state of symlink handling has improved a lot since then. I'm deleting the flag entirely (as opposed to making it a no-op) because it was only introduced in the Bazel 7 tree and hasn't made it into a stable release yet. Makes #18284 obsolete. PiperOrigin-RevId: 587691220 Change-Id: I470a8fc523bdbb7577ad5a564b6b2c0acab17d7d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
incompatible-change
Incompatible/breaking change
migration-ready
Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green
OBSOLETE: We're no longer introducing this flag in Bazel 7. See #20426 for rationale.
--- Original text ---
Flag flip for
--incompatible_remote_disallow_symlink_in_tree_artifact
.Effect: when this flag is turned on, it will be an error for a tree artifact produced by a remote action to contain a symlink. It was already an error for it to contain a symlink pointing to an absolute path; this flag extends the restriction to relative paths. The error message will look like:
How to migrate: have the action produce a regular file instead of a symlink, possibly by copying the target file into place.
Rationale: a tree artifact is currently defined to contain only regular files. If a remote action reports back a symlink, it must be resolved locally, which is generally impossible and potentially non-hermetic. Allowing only the hermetic cases (i.e., where resolution doesn't escape the action's inputs and outputs) would additional complexity and have dubious value.
Note that this is distinct from #16361, which calls for the ability to include unresolved symlinks in a tree artifact on an opt-in basis.
The text was updated successfully, but these errors were encountered: