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

fix(api,c-api,vm) Move .cargo/config into crates #2475

Closed
wants to merge 2 commits into from

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Jul 16, 2021

Description

Rework of #1939.

We need to pass the -Wl,-E arguments to the linker when compiling wasmer, wasmer_c_api and wasmer_vm. It was done generally with a .cargo/config.toml file but it was local to our project, i.e. it was not used when Wasmer was a dependency of another project.

So we move the content of .cargo/config.toml to the respective Cargo.toml files.

Fixes #1939.
Fixes #1928.
Fixes #2107.

Review

  • Add a short description of the change to the CHANGELOG.md file not necessary I guess.

@Hywan Hywan added bug Something isn't working 📦 lib-c-api About wasmer-c-api 📦 lib-api About wasmer 📦 lib-vm About wasmer-vm labels Jul 16, 2021
@Hywan Hywan self-assigned this Jul 16, 2021
@Hywan
Copy link
Contributor Author

Hywan commented Jul 16, 2021

bors try

bors bot added a commit that referenced this pull request Jul 16, 2021
@Hywan Hywan mentioned this pull request Jul 16, 2021
1 task
@bors
Copy link
Contributor

bors bot commented Jul 16, 2021

try

Build failed:

@syrusakbary
Copy link
Member

syrusakbary commented Jul 16, 2021

I had no idea this was possible.
I think we might need to add to the wasmer_vm as well (not sure how to fix it)

@Hywan
Copy link
Contributor Author

Hywan commented Jul 19, 2021

@Hywan
Copy link
Contributor Author

Hywan commented Jul 19, 2021

I believe we must write build.rs scripts that emit a https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname.

@Hywan
Copy link
Contributor Author

Hywan commented Jul 19, 2021

Er, we can't configure RUSTFLAGS with a build.rs script, and my previous idea of using cargo-rustc-link-lib is wrong.

@Hywan Hywan changed the title fix(api,c-api,vm) Move .cargo/config to Cargo.tomls fix(api,c-api,vm) Move .cargo/config into crates Jul 19, 2021
@Hywan
Copy link
Contributor Author

Hywan commented Jul 19, 2021

Reference issue, rust-lang/cargo#9426.

@Hywan
Copy link
Contributor Author

Hywan commented Jul 20, 2021

What we need exactly is rust-lang/cargo#9557: The rustc-link-arg option.

I'm going to try with rustc-cdylib-link-arg which is stable now, but is going to be renamed rustc-link-arg-cdylib in the future if I've understood everything correctly.

@Hywan
Copy link
Contributor Author

Hywan commented Jul 20, 2021

bors try

bors bot added a commit that referenced this pull request Jul 20, 2021
@bors
Copy link
Contributor

bors bot commented Jul 20, 2021

try

Build failed:

@Hywan
Copy link
Contributor Author

Hywan commented Jul 20, 2021

bors try

bors bot added a commit that referenced this pull request Jul 20, 2021
@bors
Copy link
Contributor

bors bot commented Jul 20, 2021

try

Build failed:

@Hywan
Copy link
Contributor Author

Hywan commented Jul 20, 2021

Nah, we need to wait :-).

@syrusakbary
Copy link
Member

Should we close this PR @Hywan ?

@Hywan
Copy link
Contributor Author

Hywan commented Jul 23, 2021

We can keep it open and wait on Rust.

@wchaudry wchaudry closed this Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-api About wasmer 📦 lib-c-api About wasmer-c-api 📦 lib-vm About wasmer-vm
Projects
None yet
3 participants