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

Decide how to handle a symlink inside a remotely produced tree artifact #16361

Closed
tjgq opened this issue Sep 30, 2022 · 1 comment
Closed

Decide how to handle a symlink inside a remotely produced tree artifact #16361

tjgq opened this issue Sep 30, 2022 · 1 comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug

Comments

@tjgq
Copy link
Contributor

tjgq commented Sep 30, 2022

Tree artifacts are defined as a collection of regular files. However, a remotely executed action may produce a tree artifact containing a symlink (as reported by the remote execution protocol). The current behavior is to reject such a symlink if it's absolute, and to transparently dereference it to a regular file in the context of the local filesystem if it's relative. This is arguably a bad idea since it's non-hermetic.

Note that transparently dereferencing symlinks isn't necessarily a problem by itself (we also do it for locally executed actions); the fact that action execution and dereferencing occur on different machines is. But sadly there's no mechanism in protocol through which we can request the dereferencing (although, notably, the Google implementation of RBE will always transparently dereference symlinks in action outputs, contrary to the spec).

Note that a similar issue exists for symlinks outside of tree artifacts, but additional considerations apply, since we want (and do allow) actions to produce symlinks that are never dereferenced by Bazel (i.e., declare_symlink).

@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug untriaged labels Oct 1, 2022
@tjgq tjgq added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Oct 18, 2022
copybara-service bot pushed a commit to bazelbuild/rules_apple that referenced this issue Jan 20, 2023
This change adds support to import macOS versioned frameworks
for both framework and XCFramework importing rules:

  - apple_dynamic_framework_import
  - apple_dynamic_xcframework_import

This change fixes an issue with imported macOS frameworks causing build
failures at the signing step for `macos_application` targets due to an
incorrect framework bundle format since Bazel will resolve symlinks for
versioned frameworks (standalone or bundled inside an XCFramework).

`codesign` will fail with the following error:

  "bundle format is ambiguous (could be app or framework)"

Since macOS frameworks include different framework versions under a
`Versions` directory and symlinks for the effective version this change
updates imported_dynamic_framework_processor to handle re-do symbolic
links from imported macOS frameworks, as well code signing each
framework version per Apple documentation.

Support to bundle macOS versioned frameworks with tree artifact outputs
is not supported because Bazel does not re-create symbolic links from
remote executed actions yet (bazelbuild/bazel#16361).

PiperOrigin-RevId: 503486495
copybara-service bot pushed a commit that referenced this issue May 2, 2023
…ee artifact.

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) entails additional complexity and is of dubious value. So let's instead ban it through an incompatible change.

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.

PiperOrigin-RevId: 528731896
Change-Id: Ia48d2ec2fa6d01ce227315c85bbcf5c3f07bb29a
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
…ee artifact.

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) entails additional complexity and is of dubious value. So let's instead ban it through an incompatible change.

Note that this is distinct from bazelbuild#16361, which calls for the ability to include *unresolved* symlinks in a tree artifact on an opt-in basis.

PiperOrigin-RevId: 528731896
Change-Id: Ia48d2ec2fa6d01ce227315c85bbcf5c3f07bb29a
keith pushed a commit to bazelbuild/rules_apple that referenced this issue Aug 18, 2023
This change adds support to import macOS versioned frameworks
for both framework and XCFramework importing rules:

  - apple_dynamic_framework_import
  - apple_dynamic_xcframework_import

This change fixes an issue with imported macOS frameworks causing build
failures at the signing step for `macos_application` targets due to an
incorrect framework bundle format since Bazel will resolve symlinks for
versioned frameworks (standalone or bundled inside an XCFramework).

`codesign` will fail with the following error:

  "bundle format is ambiguous (could be app or framework)"

Since macOS frameworks include different framework versions under a
`Versions` directory and symlinks for the effective version this change
updates imported_dynamic_framework_processor to handle re-do symbolic
links from imported macOS frameworks, as well code signing each
framework version per Apple documentation.

Support to bundle macOS versioned frameworks with tree artifact outputs
is not supported because Bazel does not re-create symbolic links from
remote executed actions yet (bazelbuild/bazel#16361).

PiperOrigin-RevId: 503486495
(cherry picked from commit a82a299)
keith added a commit to bazelbuild/rules_apple that referenced this issue Aug 18, 2023
This change adds support to import macOS versioned frameworks
for both framework and XCFramework importing rules:

  - apple_dynamic_framework_import
  - apple_dynamic_xcframework_import

This change fixes an issue with imported macOS frameworks causing build
failures at the signing step for `macos_application` targets due to an
incorrect framework bundle format since Bazel will resolve symlinks for
versioned frameworks (standalone or bundled inside an XCFramework).

`codesign` will fail with the following error:

  "bundle format is ambiguous (could be app or framework)"

Since macOS frameworks include different framework versions under a
`Versions` directory and symlinks for the effective version this change
updates imported_dynamic_framework_processor to handle re-do symbolic
links from imported macOS frameworks, as well code signing each
framework version per Apple documentation.

Support to bundle macOS versioned frameworks with tree artifact outputs
is not supported because Bazel does not re-create symbolic links from
remote executed actions yet
(bazelbuild/bazel#16361).

PiperOrigin-RevId: 503486495
(cherry picked from commit a82a299)

Co-authored-by: Mauricio Garcia <mauriciogs@google.com>
@tjgq
Copy link
Contributor Author

tjgq commented Dec 4, 2023

I'm going to close this, since recent changes provide a resolution: we will handle symlinks inside a remotely produced tree artifact in the exact same manner as inside a locally produced one (see a03ec7f for details). Although they do introduce a risk of non-hermeticity, this burden is already borne by rule authors, and symlinks are already allowed by file outputs; so there's little value in making local and remote execution divergent.

#15454 still stands as a feature request, but would almost certainly be introduced as a flag, instead of a reinterpretation of preexisting tree artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug
Projects
None yet
Development

No branches or pull requests

2 participants