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

Miri subtree instructions #2561

Closed
wants to merge 5 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Sep 23, 2022

supercedes #2560 (sorry for all the PRs, I needed the branch clean as I was git subtree pushing to it)

@RalfJung
Copy link
Member

Wouldn't it be easier to make this a regular Miri PR?
5 commits is a bit too much for a small doc change anyway so I was going to ask you yo squash it...^^


Every time:

Note: the first time you run this, `git subtree` builds a cache and this command may thusly take an hour or so.
Copy link
Member

Choose a reason for hiding this comment

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

An hour?!??? :(

I have many rustc checkouts. How can I make it share the cache between my checkouts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea, this worked faster for clippy and other repos...

Copy link
Member

Choose a reason for hiding this comment

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

If you use git worktree for creating the extra git checkouts I believe the subtree cache is shared. Not entirely sure though.

I have no idea, this worked faster for clippy and other repos...

The first common commit between rust and miri is from 2018 (when rust-lang/rust#46882 merged it into rustc as const eval engine). Git subtree has to walk all commits since then I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, that's not really useful... why doesn't it just walk the subtree_add_commit..HEAD range and rewrite those commits...

git remote add miri https://github.com/rust-lang/miri.git
```

Every time:
Copy link
Member

Choose a reason for hiding this comment

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

Does this also need that cache or is this always fast(er)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is always fast, just a few seconds at most, does not get slower with time

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 23, 2022

Wouldn't it be easier to make this a regular Miri PR?

it is a regular miri PR, there was just a collision with the miri subtree push that has been running for 3h now 😱 idk what's going on (yes I do have the patch). And that push was started with the same name as the previous branch name, and would have failed at the end.

I am currently figuring out what the perf issue is, and why I didn't see it while working on the PR

@RalfJung
Copy link
Member

@rustbot author

@rustbot rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Sep 24, 2022
@bors
Copy link
Contributor

bors commented Sep 27, 2022

☔ The latest upstream changes (presumably #2568) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk closed this Sep 28, 2022
@oli-obk oli-obk deleted the miri_subtree_instructions branch October 4, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: Waiting for the PR author to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants