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.

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

Don't store build check rmeta #1200

Merged
merged 2 commits into from
Oct 29, 2024
Merged

Don't store build check rmeta #1200

merged 2 commits into from
Oct 29, 2024

Conversation

nmattia
Copy link
Contributor

@nmattia nmattia commented Oct 28, 2024

Fixes #1199

This ensures the .rmeta files generated by can_compile are not written to the build output. Those metadata files can create determinism and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to /dev/null.

@nmattia
Copy link
Contributor Author

nmattia commented Oct 28, 2024

Note: I'm not sure whether /dev/null will work on "exotic" platforms like Windows!

@nmattia
Don't store build check rmeta
330bcc0 
This ensures the `.rmeta` files generated by `can_compile` are not
written to the build output. Those metadata files can create determinism
and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to
`/dev/null`.
@nmattia
Copy link
Contributor Author

nmattia commented Oct 28, 2024

Note 2: as far as I can tell the metadata is not being read anymore since #730

@sunfishcode
Copy link
Member

Thanks for the PR! This looks reasonable to me, though I also don't know how this works on Windows. That said, the Windows CI appears to have passed. Is Windows is recognizing /dev/null for compatibility?

@nmattia
Copy link
Contributor Author

nmattia commented Oct 28, 2024

Is Windows is recognizing /dev/null for compatibility?

If the CI runs under WSL then most likely yes. Also I see the build script uses various sorts of pipes for stdin, so I guess using /dev/null should be OK?

@sunfishcode
Copy link
Member

The Windows CI doesn't run in WSL. Windows has its own version of pipes and Rust's std::process::Command knows how to use it.

What if we instead used .arg("-o").arg("-") and .stdout(Stdio::null())? That seems like it should work and avoid any portability question.

@nmattia
Copy link
Contributor Author

nmattia commented Oct 28, 2024

What if we instead used .arg("-o").arg("-") and .stdout(Stdio::null())? That seems like it should work and avoid any portability question.

That sounds good! I'll push a fix tomorrow when I have some time. Thanks for the quick feedback!

@nmattia
Copy link
Contributor Author

nmattia commented Oct 29, 2024

@sunfishcode PTAL!

@sunfishcode sunfishcode merged commit cf68714 into bytecodealliance:main Oct 29, 2024
45 checks passed
@sunfishcode
Copy link
Member

Thanks!

@nmattia nmattia deleted the nm-rmeta-devnull branch October 29, 2024 19:51
@nmattia nmattia mentioned this pull request Oct 31, 2024
Closed
sunfishcode pushed a commit that referenced this pull request Nov 4, 2024
@nmattia @sunfishcode
Don't store build check rmeta (#1200)
ce48183 
This ensures the `.rmeta` files generated by `can_compile` are not
written to the build output. Those metadata files can create determinism
and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to
`/dev/null`.
sunfishcode pushed a commit that referenced this pull request Nov 4, 2024
@nmattia @sunfishcode
Don't store build check rmeta (#1200)
9fd6e48 
This ensures the `.rmeta` files generated by `can_compile` are not
written to the build output. Those metadata files can create determinism
and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to
`/dev/null`.
@sunfishcode
Copy link
Member

This is now released in rustix 0.38.39.

sunfishcode added a commit to bytecodealliance/cap-std that referenced this pull request Nov 22, 2024
@sunfishcode
Don't store build check rmeta
0e55fc8 
This ensures the `.rmeta` files generated by `can_compile` are not
written to the build output. Those metadata files can create determinism
and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to
`/dev/null`.

This is a port of bytecodealliance/rustix#1200 to cap-std.
@sunfishcode sunfishcode mentioned this pull request Nov 22, 2024
Merged
sunfishcode added a commit to bytecodealliance/cap-std that referenced this pull request Nov 22, 2024
@sunfishcode
Don't store build check rmeta (#375)
cc7d2b9 
* Don't store build check rmeta

This ensures the `.rmeta` files generated by `can_compile` are not
written to the build output. Those metadata files can create determinism
and cache invalidation issues in some build systems.

Instead the output -- since it is never actually read -- is written to
`/dev/null`.

This is a port of bytecodealliance/rustix#1200 to cap-std.

* Update CI for wasm32-wasip1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rust metadata generated from build script creates determinism issues
2 participants
@nmattia @sunfishcode