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

sync Miri #102464

Closed
wants to merge 1 commit into from
Closed

sync Miri #102464

wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

Created via

(cd src/tools && rm -rf miri && git clone https://github.com/rust-lang/miri --depth 1 && rm -rf miri/.git && git add miri)

r? @oli-obk

@rustbot
Copy link
Collaborator

rustbot commented Sep 29, 2022

The Miri subtree was changed

cc @rust-lang/miri

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Sep 29, 2022

@bors r+ p=1 tool sync

@bors
Copy link
Contributor

bors commented Sep 29, 2022

📌 Commit bae2388116acb768e1de6f5d76fae696c97fe811 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2022
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 29, 2022
@RalfJung
Copy link
Member Author

@bors r=oli-obk
(I also removed installing xargo from bootstrap)

Do we still need p=1? Miri isn't broken, after all.

@bors
Copy link
Contributor

bors commented Sep 29, 2022

📌 Commit d541412026b3245b7f94deb9eb1325ef5cd7518c has been approved by oli-obk

It is now in the queue for this repository.

@RalfJung
Copy link
Member Author

Of course the next josh sync from rustc to Miri will duplicate this commit there...

@oli-obk
Copy link
Contributor

oli-obk commented Sep 29, 2022

Do we still need p=1? Miri isn't broken, after all.

may be less annoying to resolve conflicts. Not sure what the policy on clippy syncs is

@RalfJung
Copy link
Member Author

RalfJung commented Sep 29, 2022 via email

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 29, 2022
@RalfJung
Copy link
Member Author

Argh, people changed their licenses...

tidy error: invalid license `0BSD` in `enum-iterator 1.2.0 (registry+[https://github.com/rust-lang/crates.io-index)`](https://github.com/rust-lang/crates.io-index)%60)
tidy error: invalid license `0BSD` in `enum-iterator-derive 1.1.0 (registry+[https://github.com/rust-lang/crates.io-index)`](https://github.com/rust-lang/crates.io-index)%60)
tidy error: invalid license `(MIT OR Apache-2.0) AND Unicode-DFS-2016` in `unicode-ident 1.0.4 (registry+[https://github.com/rust-lang/crates.io-index)`](https://github.com/rust-lang/crates.io-index)%60)
tidy error: Dependencies for main workspace not explicitly permitted:
* unicode-ident 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)

@RalfJung
Copy link
Member Author

RalfJung commented Sep 29, 2022

Maybe we should just get rid of vergen, that would help with some of these. clippy seems to use a local copy of https://crates.io/crates/rustc_tools_util. @oli-obk is it okay to use the crates.io version of that crate?

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Sep 29, 2022

@RalfJung the original source of the crate lives in the clippy repo, it shouldn't be a fork :D
(I wrote this back then so we could have nicer version info in clippy ^^)

@RalfJung
Copy link
Member Author

Yeah I realized it's not a fork, but if Miri now uses the crates.io version then we have code duplication in the rustc workspace. Also the crates.io version is outdated but I don't know if the diff is relevant.

Cc rust-lang/rust-clippy#9553

@RalfJung
Copy link
Member Author

RalfJung commented Sep 29, 2022

The unicode-ident error is strange -- this is a cargo-miri dependency, but we get an error for it supposedly being a rustc dependency?

@RalfJung
Copy link
Member Author

RalfJung commented Sep 29, 2022

Ah, this updates proc-macro2 which switched from unicode-xid to unicode-ident.

However if we avoid vergen I think we can also avoid updating syn and then this license becomes someone else's problem...^^

@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Sep 29, 2022
@RalfJung
Copy link
Member Author

Of course the next josh sync from rustc to Miri will duplicate this commit there...

... and this does seem to lead to conflicts down the road, when I try to sync rustc back into Miri after this PR.

I am not quite sure how to proceed here. This might be fine for a one-off change, but longer term we probably want to get josh to work for both directions, which I guess would work better if we had done the initial subtree merge with josh instead of git-subtree... but now we need to figure out how to live with the repo that we have.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 29, 2022

Well... the proper josh way would be to erase all miri history here, us not doing that is where the failure to sync miri->rustc is coming from.

Alternatively... we can re-sync the src/tools/miri folder from scratch... creating another duplication of all miri commits in the rustc repo 🙈

@RalfJung
Copy link
Member Author

Well... the proper josh way would be to erase all miri history here, us not doing that is where the failure to sync miri->rustc is coming from.

How can we erase history in git?

Alternatively... we can re-sync the src/tools/miri folder from scratch... creating another duplication of all miri commits in the rustc repo see_no_evil

Is it literally the original commits? Because then it wouldn't duplicate, it would reference the same commits that git-subtree already added to the repo.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 29, 2022

Is it literally the original commits? Because then it wouldn't duplicate, it would reference the same commits that git-subtree already added to the repo.

hmm... I don't believe it works the same way, no. josh rewrites the commits afaict.

@RalfJung
Copy link
Member Author

(after looking a bit more into josh)

Yeah, josh would rewrite the Miri commits to happen below src/tools/miri, basically making it a seamless part of the rustc repo. Pretty neat... but not compatible with what we have.^^

@RalfJung
Copy link
Member Author

Okay so I think the reason syncing usually just 'magically' works in josh is that when we sync Miri changes into rustc, and then export them to Miri again again, they end up with the original SHA. So then syncing them into Miri again should work neatly.

Using josh only for one direction will not work well I think.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 2, 2022

I got josh to create a sync, pushed to alternative PR #102573.

@RalfJung RalfJung closed this Oct 2, 2022
@RalfJung RalfJung deleted the miri branch October 2, 2022 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants