Skip to content

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Aug 9, 2024

This was introduced in #8668 but it doesn't appear to be necessary, and its presence breaks wasmtime-cli builds for x86_64-unknown-linux-musl on linux (tested with AWS2023, a redhat derivative) when using the default linker.

CI overrides the linker with a musl-gcc install. I'm not sure why, and at least in my project, we build without using the musl-gcc linker and everything works perfectly, and adopting that strategy broke something in a way I couldn't debug. So, I'd like it for CI to test the musl build with the default linker as well, to catch issues like this one. The CI matrix / docker configuration is pretty complicated and I was gonna consult with @alexcrichton on the best way to add this check.

@pchickey pchickey force-pushed the pch/musl_remove_libm_linkage branch from 0c63753 to 7126c7d Compare August 9, 2024 22:04
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Aug 9, 2024
@pchickey pchickey marked this pull request as ready for review August 10, 2024 18:18
@pchickey pchickey requested a review from a team as a code owner August 10, 2024 18:18
@pchickey pchickey requested review from fitzgen and alexcrichton and removed request for a team and fitzgen August 10, 2024 18:18
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

CI overrides the linker with a musl-gcc install. I'm not sure why, ...

This is because our build containers are all Ubuntu so the default linker uses glibc instead of musl. The musl-gcc package is sort of a few wrapper scripts to tell the linker to link musl instead of glibc. In your distro it might be musl-based so this all happens by default perhaps?

In any case looks good to me, now I'm not sure why I needed this but maybe I got something wrong while developing this and accidentally thought it was required.

@alexcrichton alexcrichton added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit def6f32 Aug 12, 2024
125 checks passed
@alexcrichton alexcrichton deleted the pch/musl_remove_libm_linkage branch August 12, 2024 15:12
pchickey pushed a commit that referenced this pull request Aug 12, 2024
pchickey pushed a commit that referenced this pull request Aug 12, 2024
alexcrichton pushed a commit that referenced this pull request Aug 12, 2024
alexcrichton pushed a commit that referenced this pull request Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants