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

👻 Add options to customize behavior of unknown Wasm imports #313

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

acfoltzer
Copy link
Contributor

Adds a new shared option to the CLI with three behavior modes:

      --unknown-import-behavior <UNKNOWN_IMPORT_BEHAVIOR>
          Set the behavior for unknown imports.

          Note that if a program only works with a non-default setting for this flag, it is unlikely to be publishable to Fastly.

          [default: link-error]

          Possible values:
          - link-error:   Unknown imports are rejected at link time (default behavior)
          - trap:         Unknown imports trap when called
          - zero-or-null: Unknown imports return zero or a null pointer, depending on the type

This ended up touching more files than I'd expect, because I had to make the integration test harness bubble out link-time errors rather than .expect()ing them, so there are quite a few new ?s across the integration test suite.

@acfoltzer acfoltzer added the feature-ux Concerning ergonomics and ease-of-use label Sep 19, 2023
@acfoltzer acfoltzer self-assigned this Sep 19, 2023
Adds a new shared option to the CLI with three behavior modes:

```
      --unknown-import-behavior <UNKNOWN_IMPORT_BEHAVIOR>
          Set the behavior for unknown imports.

          Note that if a program only works with a non-default setting for this flag, it is unlikely to be publishable to Fastly.

          [default: link-error]

          Possible values:
          - link-error:   Unknown imports are rejected at link time (default behavior)
          - trap:         Unknown imports trap when called
          - zero-or-null: Unknown imports return zero or a null pointer, depending on the type
```

This ended up touching more files than I'd expect, because I had to make the integration test
harness bubble out link-time errors rather than `.expect()`ing them, so there are quite a few new
`?`s across the integration test suite.
@acfoltzer acfoltzer marked this pull request as ready for review September 20, 2023 00:48
@acfoltzer acfoltzer requested review from a team and aturon and removed request for a team September 20, 2023 18:29
@acfoltzer acfoltzer enabled auto-merge (squash) September 20, 2023 18:30
@tedmielczarek-fastly
Copy link

tedmielczarek-fastly commented Sep 21, 2023

This sounds great! Since we're using a privileged hostcall in our internal service, we currently have a cargo viceroy feature to avoid referencing that hostcall for local testing. I think the zero-or-null behavior would work perfectly there and make local testing less annoying.

@acfoltzer acfoltzer requested review from awortman-fastly and removed request for aturon September 22, 2023 18:11
Copy link

@iximeow iximeow left a comment

Choose a reason for hiding this comment

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

.... it occurs to me that this is a fascinating way to discover at runtime if a program's dependencies really do need browser-shaped wasm glue, or if maybe you'd never actually reach any of that at runtime. hmmmm..

very handy!

Copy link

@awortman-fastly awortman-fastly left a comment

Choose a reason for hiding this comment

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

i should approve from the right account though

@acfoltzer acfoltzer merged commit f62eb94 into main Sep 22, 2023
7 checks passed
@acfoltzer acfoltzer deleted the acf/unknown-imports branch September 22, 2023 20:34
cmckendry pushed a commit to 1stdibs/Viceroy that referenced this pull request Feb 8, 2024
Adds a new shared option to the CLI with three behavior modes:

```
      --unknown-import-behavior <UNKNOWN_IMPORT_BEHAVIOR>
          Set the behavior for unknown imports.

          Note that if a program only works with a non-default setting for this flag, it is unlikely to be publishable to Fastly.

          [default: link-error]

          Possible values:
          - link-error:   Unknown imports are rejected at link time (default behavior)
          - trap:         Unknown imports trap when called
          - zero-or-null: Unknown imports return zero or a null pointer, depending on the type
```

This ended up touching more files than I'd expect, because I had to make the integration test
harness bubble out link-time errors rather than `.expect()`ing them, so there are quite a few new
`?`s across the integration test suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ux Concerning ergonomics and ease-of-use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants