Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@zarvox
Copy link
Contributor

@zarvox zarvox commented Mar 25, 2020

This is a second attempt at #16690, which was reverted because it broke the Dart JIT runner. The primary differences are that this time around, we correctly handle absolute vs relative paths, depending on whether library loading bottoms out in fdio_open_fd or fdio_open_fd_at. I've added additional assertions.

On Fuchsia, we can now get executable VMOs from trusted backing
filesystems. This allows us to remove the use of replace_as_executable
in favor of opening files with fdio_open_fd_at with the
OPEN_RIGHT_EXECUTABLE flag and getting VMOs by calling
fdio_get_vmo_exec.

By moving the responsibility for executability into the filesystem, we
are able to remove deprecated-ambient-replace-as-executable from
component manifests for non-JIT runners (the JIT runners still call
replace_as_executable in Dart's allocator). It wasn't abundantly clear
whether .cmx files for tests were used purely in AOT runtime
environments or also saw JIT usage, so I left those as-is.

Testing: I verified locally that the flutter product runner works on
Astro, and also successfully ran the Dart JIT example test (which was
the thing blocking the google3 roll with the previous attempt at this
patchset). If there's any additional pre-flight checks you'd like me to do, let me know!

On Fuchsia, we can now get executable VMOs from trusted backing
filesystems.  This allows us to remove the use of replace_as_executable
in favor of opening files with `fdio_open_fd_at` with the
`OPEN_RIGHT_EXECUTABLE` flag and getting VMOs by calling
`fdio_get_vmo_exec`.

By moving the responsibility for executability into the filesystem, we
are able to remove `deprecated-ambient-replace-as-executable` from
component manifests for non-JIT runners (the JIT runners still call
replace_as_executable in Dart's allocator).  It wasn't abundantly clear
whether .cmx files for tests were used purely in AOT runtime
environments or also saw JIT usage, so I left those as-is.

For context: this is a second attempt at flutter#16690, which was reverted
because it broke the Dart JIT runner.  The primary difference is that
this time around, we correctly handle absolute vs relative paths,
depending on whether library loading bottoms out in `fdio_open_fd` or
`fdio_open_fd_at`.  I've added additional assertions to help ensure any
new usages use the correct shape of path.

Testing: I verified locally that the flutter product runner works on
Astro, and also successfully ran the Dart JIT example test (which was
the thing blocking the google3 roll with the previous attempt at this
patchset).
@auto-assign auto-assign bot requested a review from gw280 March 25, 2020 07:37
@zarvox zarvox merged commit fb84107 into flutter:master Mar 25, 2020
@zarvox zarvox deleted the zarvox-reduce-ambient-exec branch March 25, 2020 20:17
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants