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

Don't link with --export-dynamic on wasm32-wasi #81255

Merged
merged 1 commit into from
Feb 1, 2021

Conversation

sunfishcode
Copy link
Member

Remove --export-dynamic from the link arguments on the wasm32-wasi
target, as it emits spurious exports and increases code size.

Leave it in place for wasm32-unknown-unknown and
wasm32-unknown-emscripten. Even though it isn't a great solution
there, users are likely depending on its behavior there.

Remove --export-dynamic from the link arguments on the wasm32-wasi
target, as it emits spurious exports and increases code size.

Leave it in place for wasm32-unknown-unknown and
wasm32-unknown-emscripten. Even though it isn't a great solution
there, users are likely depending on its behavior there.
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2021
@jyn514 jyn514 added A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 22, 2021
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

Implementation looks good to me but can't speak to whether the change is appropriate.

@davidtwco
Copy link
Member

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 22, 2021

📌 Commit 9abcfa5 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2021
@bors
Copy link
Contributor

bors commented Jan 25, 2021

⌛ Testing commit 9abcfa5 with merge e60c535320e85d27f58918a87df082c1c17eceda...

@rust-log-analyzer
Copy link
Collaborator

The job dist-x86_64-linux-alt failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustfix v0.5.1
error[E0283]: type annotations needed
   --> src/cargo/util/config/de.rs:530:63
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, env.as_ref()))
    |                                                           |   |
    |                                                           |   |
    |                                                           |   cannot infer type for type parameter `T` declared on the trait `AsRef`
    |                                                           this method call resolves to `&T`
    |
    = note: cannot satisfy `std::string::String: AsRef<_>`
help: use the fully qualified path for the potential candidates
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<OsStr>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<std::path::Path>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<str>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<[u8]>>::as_ref(env)))

error: aborting due to previous error

For more information about this error, try `rustc --explain E0283`.

@bors
Copy link
Contributor

bors commented Jan 25, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 25, 2021
@alexcrichton
Copy link
Member

@bors: retry

That spurious failure was fixed by #81380

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 27, 2021
… r=alexcrichton

Don't link with --export-dynamic on wasm32-wasi

Remove --export-dynamic from the link arguments on the wasm32-wasi
target, as it emits spurious exports and increases code size.

Leave it in place for wasm32-unknown-unknown and
wasm32-unknown-emscripten. Even though it isn't a great solution
there, users are likely depending on its behavior there.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 27, 2021
… r=alexcrichton

Don't link with --export-dynamic on wasm32-wasi

Remove --export-dynamic from the link arguments on the wasm32-wasi
target, as it emits spurious exports and increases code size.

Leave it in place for wasm32-unknown-unknown and
wasm32-unknown-emscripten. Even though it isn't a great solution
there, users are likely depending on its behavior there.
henryboisdequin added a commit to henryboisdequin/rust that referenced this pull request Jan 29, 2021
… r=alexcrichton

Don't link with --export-dynamic on wasm32-wasi

Remove --export-dynamic from the link arguments on the wasm32-wasi
target, as it emits spurious exports and increases code size.

Leave it in place for wasm32-unknown-unknown and
wasm32-unknown-emscripten. Even though it isn't a great solution
there, users are likely depending on its behavior there.
This was referenced Jan 31, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2021
…as-schievink

Rollup of 11 pull requests

Successful merges:

 - rust-lang#80092 (2229: Fix issues with move closures and mutability)
 - rust-lang#80404 (Remove const_in_array_repeat)
 - rust-lang#81255 (Don't link with --export-dynamic on wasm32-wasi)
 - rust-lang#81480 (Add suggestion for nested fields)
 - rust-lang#81549 (Misc ip documentation fixes)
 - rust-lang#81566 (Add a test for rust-lang#71202)
 - rust-lang#81568 (Fix an old FIXME in redundant paren lint)
 - rust-lang#81571 (Fix typo in E0759)
 - rust-lang#81572 (Edit multiple error code Markdown files)
 - rust-lang#81589 (Fix small typo in string.rs)
 - rust-lang#81590 (Stabilize int_bits_const)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ed56145 into rust-lang:master Feb 1, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 1, 2021
@sunfishcode sunfishcode deleted the wasi-no-export-dynamic branch February 27, 2021 15:34
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this pull request Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this pull request Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ZauberNerd added a commit to ZauberNerd/tinygo that referenced this pull request Mar 18, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
aykevl pushed a commit to tinygo-org/tinygo that referenced this pull request Mar 19, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
deadprogram pushed a commit to tinygo-org/tinygo that referenced this pull request Apr 7, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
deadprogram pushed a commit to tinygo-org/tinygo that referenced this pull request Apr 11, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
ardnew pushed a commit to ardnew/tinygo that referenced this pull request Jun 21, 2022
Exporting symbols seems to embed them in the WASM exports section which
causes wasmtime to fail: bytecodealliance/wasmtime#2587
As a workaround, it is possible to specify the `--allow-unknown-exports`
flag on wasmtime.
But as discussed in the above linked issue, this seems to only be a
workaround. For the Rust compiler the fix was to remove the
`--export-dynamic` linker flag when targeting `wasm32-wasi`:
rust-lang/rust#81255
Which is waht this commit does for Tinygo too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries O-wasm Target: WASM (WebAssembly), http://webassembly.org/ S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants