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 some CMake guards when building the sysroot #462

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

alexcrichton
Copy link
Collaborator

  • Require that the compiler is Clang, for example gcc and msvc cannot compile to WebAssembly.
  • Require that the Clang version is above the minimum threshold. Unsure what the minimum threshold is at this time so I've set it to 18.0.0

* Require that the compiler is Clang, for example gcc and msvc cannot
  compile to WebAssembly.
* Require that the Clang version is above the minimum threshold. Unsure
  what the minimum threshold is at this time so I've set it to 18.0.0
Copy link
Collaborator

@abrown abrown left a comment

Choose a reason for hiding this comment

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

Why set the minimum? I.e., if it's too high then old-toolchain users run into issues; if it's too low then what's the point?

@alexcrichton
Copy link
Collaborator Author

It's mostly the first part, "users run into issues," where an error message saying "your toolchain is too old" is often more debuggable than some obscure "whatever error this old compiler happened to emit" error message. In my experience in Rust most folks outside of maintainers don't know how to diagnose such errors since the version isn't the obvious culprit

Copy link
Collaborator

@abrown abrown left a comment

Choose a reason for hiding this comment

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

Makes sense; let's merge it. I would propose we also keep open the possibility of lowering the minimum in the future: if any users of older toolchains feel strongly about this, maybe we allow them to downgrade this?

@abrown abrown merged commit b0075f9 into WebAssembly:main Jul 31, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants