Skip to content

Rust: Remove -Zlink-native-libs=no and bigint flags#5340

Merged
hoodmane merged 10 commits intopyodide:mainfrom
hoodmane:rust-link-native-libs
Jan 17, 2025
Merged

Rust: Remove -Zlink-native-libs=no and bigint flags#5340
hoodmane merged 10 commits intopyodide:mainfrom
hoodmane:rust-link-native-libs

Conversation

@hoodmane
Copy link
Member

@hoodmane hoodmane commented Jan 15, 2025 •
edited
Loading

We don't need these anymore. link_native_libs=no was fixed by:
rust-lang/libc#4002
and wasm_bigint was fixed by:
rust-lang/rust#131736

@hoodmane
Remove -Zlink-native-libs=no
fe93458 
After updating the rust nightly, we don't need this anymore.
@hoodmane
Update pyodide-build
856129f
@hoodmane hoodmane changed the title Remove -Zlink-native-libs=no Remove -Zlink-native-libs=no and bigint flags Jan 15, 2025
@hoodmane hoodmane changed the title Remove -Zlink-native-libs=no and bigint flags Rust: Remove -Zlink-native-libs=no and bigint flags Jan 15, 2025
ryanking13
ryanking13 reviewed Jan 16, 2025
View reviewed changes
Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks! Probably we can add rust_flags to OVERRIDABLE_BUILD_KEYS, so we can set it in pyproject.toml not in Makefile.envs.

@hoodmane
Copy link
Member Author

hoodmane commented Jan 16, 2025 •
edited
Loading

I do kind of like the way that Makefile.envs can use variables though. For instance in #5320 I moved RUST_TOOLCHAIN back to Makefile.envs so I can substitute it into RUST_EMSCRIPTEN_TARGET_URL:

export RUST_TOOLCHAIN ?= nightly-2025-01-15
export RUST_EMSCRIPTEN_TARGET_URL ?= .../rustc-emscripten-target/${RUST_TOOLCHAIN}.tar.bz2

@hoodmane
Copy link
Member Author

hoodmane commented Jan 16, 2025

I wonder if it would make sense to keep Makefile.envs as the source of truth and then generate a json file from it and use that in other tools. Then we can have a CI job that checks that the generated json file matches the output of Makefile.envs. And that will let us use variables.

@ryanking13
Copy link
Member

ryanking13 commented Jan 16, 2025

I wonder if it would make sense to keep Makefile.envs as the source of truth and then generate a json file from it and use that in other tools. Then we can have a CI job that checks that the generated json file matches the output of Makefile.envs. And that will let us use variables.

pyodide-build can read Makefile.envs and export the values using pyodide config CLI, so I think other tools can use that.

I moved RUST_TOOLCHAIN back to Makefile.envs so I can substitute it into RUST_EMSCRIPTEN_TARGET_URL:

Hmm, maybe in this case, putting the values in Makefile.envs makes sense, as it should affect all packages that build against the new Pyodide versions, which will use wasm-eh.

Okay, then I am +1 with this PR.

@hoodmane
Copy link
Member Author

hoodmane commented Jan 16, 2025

Hitting error: rustup is not installed at '/root/.cargo' again.

@hoodmane hoodmane merged commit 65dd662 into pyodide:main Jan 17, 2025
8 of 9 checks passed
@hoodmane hoodmane deleted the rust-link-native-libs branch January 17, 2025 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryanking13 ryanking13 ryanking13 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hoodmane @ryanking13