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

prepare PR that will "dry run" the crater migration #85895

Closed
3 tasks done
nikomatsakis opened this issue Jun 1, 2021 · 6 comments
Closed
3 tasks done

prepare PR that will "dry run" the crater migration #85895

nikomatsakis opened this issue Jun 1, 2021 · 6 comments
Assignees
Labels
A-edition-2021 Area: The 2021 edition T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 1, 2021

As part of the 2021 Edition release, we want to do a "dry run" crater run that attempts to do migrations and then builds and runs tests. This will help us check the lints.

We cannot actually run this setup until the following issues are closed, but we can still do the work to prepare the PR now:

and other items in the "Feature Complete Blockers" column from the Rust 2021 Project Board.

@nikomatsakis nikomatsakis added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. A-edition-2021 Area: The 2021 edition labels Jun 1, 2021
@nikomatsakis
Copy link
Contributor Author

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Jul 15, 2021

Mentoring notes

I'm no expert on the crater code, but here are some notes extracted from the thread above (thanks @pietroalbini).

The goal

We want to modify crater so that, for each 2018 crate, it runs cargo fix --edition to see if it builds.

  • Check that the crate is in the 2018 edition; if not, skip it I think
  • Run cargo fix --edition
  • Run cargo check afterwards, to see that it still builds (maybe? I think that cargo fix may do this itself...)

One thing I'm not entirely sure about: crater normally does comparisons between two toolchains. Presumably the baseline behavior here is just a regular build and not running cargo fix. I'm not sure how that logic is encoded.

Modifying crater

To start, you would add a mode like EditionMigration(Year) to this enum. I think we would like to be able to write the code in such a way that we can test migrating 2015 crates migration to 2018, 2018 crates to 2021 or -- in the future -- 2021 crates to 2024 (or whatever the next edition is) with minimal to no edits.

https://github.com/rust-lang/crater/blob/aa40b144e61f7f1c6ba6a3bfc5e85b52b9e4a177/src/experiments.rs#L29-L36

When you do, the compiler will take you on a little tour of the crater code. The critical bit is here, where we create different kinds of tasks depending on the mode:

https://github.com/rust-lang/crater/blob/ba03080696d339981948fff0b3da3611e45fdbb9/src/runner/graph.rs#L291-L324

We'll have to add a new kind of TaskStep for "cargo fix --edition" and propagate things around:

https://github.com/rust-lang/crater/blob/29fcb562fd88f43c0560320fd262f58add55a1a3/src/runner/tasks.rs#L51-L61

At some point that will require writing a function like this that actually invokes cargo fix:

https://github.com/rust-lang/crater/blob/29fcb562fd88f43c0560320fd262f58add55a1a3/src/runner/test.rs#L321-L343

Testing crater

You can run crater from the CLI, as described here:

https://github.com/rust-lang/crater/blob/master/docs/cli-usage.md

@nikomatsakis
Copy link
Contributor Author

@ehuss pointed out on Zulip that another, perhaps better, approach would be to do a crater run with a modified cargo. This would make it easier to get a "stable" comparison.

The idea would be to modify cargo so that e.g. cargo build or cargo test runs cargo fix --edition.

@pietroalbini
Copy link
Member

Actually, now that I think about it the best way to implement this in Crater would be to add a new "toolchain flag" that runs cargo fix --edition, allowing you to define a toolchain like try#1234567890abcdef+edition-fix. That way you'd run the experiment between try#1234567890abcdef and try#1234567890abcdef+edition-fix, and you could use all the existing modes on top of that (check-only, build-and-test...).

You'd need to add a boolean flag here and implement display and parse for +edition-fix. Once that's done before starting a test you need to add code to check whether the flag is set on the toolchain, and in that case run cargo fix --edition.

@ehuss
Copy link
Contributor

ehuss commented Jul 16, 2021

I have prepared a hacked cargo that should work: ehuss/cargo@e32d9d5

Another snag I ran into was that crater runs with a read-only filesystem. The source needs to be writeable to apply any fixes. The hack copies the source to /tmp to work around that. I'm uncertain if that will cause problems, but I think it should work (at least it seemed fine in my testing).

Getting crater to support this use case would be nice, but I think will require quite a few changes (such as making the toolchain_start run different commands than toolchain_end). And it's not really clear how to integrate the things like modifying Cargo.toml. I think the hack should suffice for now, and maybe for the next edition we can plan for something better. 😉

@m-ou-se
Copy link
Member

m-ou-se commented Aug 12, 2021

The crater run is happening here: #87190

(First the top-1000 crates. Later everything.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2021 Area: The 2021 edition T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants