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

Explore using cargo-hakari to avoid duplicate builds between miri test and miri run #3372

Closed
RalfJung opened this issue Mar 10, 2024 · 6 comments
Labels
A-dev Area: working on Miri as a developer C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@RalfJung
Copy link
Member

./miri test and ./miri run currently each build their own copy of the Miri binary. The reasons for that are described here. The tool https://crates.io/crates/cargo-hakari is intended to help with cases like that when they arise in a workspace; maybe they also help with the situation of multiple crates within a single package.

@RalfJung
Copy link
Member Author

One challenge might be that the crate can't be in a workspace -- this entire repository is embedded in the rustc repo as a subtree, and rustc makes Miri a workspace member, so Miri's Cargo.toml can't itself be declared a workspace root.

@sunshowers is that a problem for hakari, or does it also work for non-workspace crates?

@sunshowers
Copy link
Contributor

sunshowers commented Mar 10, 2024

Hmmm, interesting. I've never tried it on single-crate projects, but I wonder if there's a way to make it work with some creativity. For example you could have an outer Cargo.toml that does declare a workspace, and import miri as a subdirectory into the Rust tree (so the Rust tree never sees miri as a workspace, just as a crate).

@RalfJung
Copy link
Member Author

The subtree approach we are working doesn't let us do that I think. This repo here is basically just a projection of the src/tools/miri subdir in the rustc repo -- the projection is deterministic so each time we extract we get matching git history. But I don't think we can have stuff "around" the projection. Everything in this repo must also exist in the rust repo.

@RalfJung RalfJung added C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement A-dev Area: working on Miri as a developer labels May 3, 2024
@RalfJung
Copy link
Member Author

Just using hakari on Miri fails:

Error: 
   0: for path /home/r/src/rust/miri/Cargo.toml, [workspace] section not found

I was hoping it could just consider this to be a single-crate workspace, but unfortunately that's not how it works.

However, even if I make Miri into a workspace, I get a rather inscrutable error message:

The application panicked (crashed).
Message:  valid idx
Location: /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hakari-0.17.4/src/cli_ops/workspace_ops.rs:186

@RalfJung
Copy link
Member Author

I managed to get it to work by tweaking the workspace settings:

[workspace]
members = ["miri"]
exclude = ["cargo-miri"]

And that seems to work indeed. :) No more rebuilds in ./miri run. 🎉

However we can't set this up for CI because we can't have this [workspace] section permanently in our Cargo.toml.

@RalfJung
Copy link
Member Author

In the end I went with #3881 -- entirely avoiding the use of cargo run, and therefore the need for a workspace-hack crate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dev Area: working on Miri as a developer C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

No branches or pull requests

2 participants