Skip to content

Allow compiling uucore with wasm32-unknown-unknown #7840

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Allow compiling uucore with wasm32-unknown-unknown #7840

wants to merge 10 commits into from

Conversation

cptpiepmatz
Copy link

@cptpiepmatz cptpiepmatz commented Apr 25, 2025 •
edited
Loading

Older versions of uucore could be compiled with wasm32-unknown-unknown. But inside uucore::mods::io are file handles used which only exist when you have a filesystem which wasm doesn't have. So I feature gated them behind fs. I cannot really run the tests on my machine as my german operating system doesn't report english error messages.

@cptpiepmatz
Copy link
Author

related: nushell/nushell#15638

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

it would be nice to add a job in the CI to test this to make sure we don't regress in the future ?

cakebaker
cakebaker reviewed Apr 25, 2025
View reviewed changes
src/uu/dd/Cargo.toml Outdated
@@ -21,7 +21,7 @@ path = "src/dd.rs"
clap = { workspace = true }
gcd = { workspace = true }
libc = { workspace = true }
uucore = { workspace = true, features = ["format", "parser", "quoting-style"] }
uucore = { workspace = true, features = ["format", "parser", "quoting-style", "fs"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should make the Style/toml job in the CI pass:

Suggested change
uucore = { workspace = true, features = ["format", "parser", "quoting-style", "fs"] }
uucore = { workspace = true, features = [
"format",
"parser",
"quoting-style",
"fs",
] }

Copy link
Author

Choose a reason for hiding this comment

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

Thx, I just run the formatter for me locally via "Event Better TOML" (a vscode extension) but npx --yes @taplo/cli fmt src\uu\dd\Cargo.toml did not work for me for some reason. I'm on Windows.

@cptpiepmatz
Copy link
Author

it would be nice to add a job in the CI to test this to make sure we don't regress in the future ?

I'm not sure how do this correctly, I tried this 3f4ff9b

@cptpiepmatz
Copy link
Author

@sylvestre @cakebaker how do I properly test this? For wasm32 we want uutils as a libraries mostly, so everything that requires a file system won't ever work. How do I finetune the CI for that?

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@sylvestre
Copy link
Contributor

just build it, no need to run the tests for now

@cptpiepmatz
Copy link
Author

I added a new job to handle building for WASM, including it inside the main build job felt overly complicated

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

Why limiting the wasm CI build to only uucore?

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@cptpiepmatz
Copy link
Author

Why limiting the wasm CI build to only uucore?

I just added everything that currently compiles to WASM to the CI but I don't know if that is so great too. The uucore provides some niceties to be used as a lib, that's why I included it as the only thing. Some commands can never really work on wasm32-unknown-unknown as it doesn't come with a file system. But some commands could work, or at least the function interface.

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

thanks! However, building all coreutils with a simple cargo build --target wasm --features unix is probably good enough :)
no need to test utilities individually
sorry if I wasn't clear :)

@cptpiepmatz
Copy link
Author

The problem I have here is that wasm is not unix, also I don't expect everything to work. So I don't really know what we want to run here. cargo build --target wasm32-unknown-unknown --no-default-features works fine for example, but I want explicitly use the format feature on uucore.

So what should I do? I have no problem doing work on the CI but I don't know what exactly we want here.

@sylvestre
Copy link
Contributor

match what is done elsewhere :)
build all at once (or in the same command steps) and run the tests if possible
here, I would like to avoid having 70 new jobs
thanks

@cptpiepmatz
Copy link
Author

How should I handle the "Package" and "Publish" steps?

  • skip them when target is wasm
  • skip them when a certain flag is set
  • try package the wasm file (for what even?)

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

just ignore these steps :)
thanks

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@cptpiepmatz
Copy link
Author

I think the mac build got stuck somehow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cakebaker cakebaker cakebaker left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cptpiepmatz @sylvestre @cakebaker