Cherry-pick some downstream commits

[lnicola]

I couldn't use git merge for this, it seemed to give conflicts on files only modified on our side, like crates/project-model/Cargo.toml, so I cherry-picked a couple of commits from there.

[Veykril]

I don't know subtrees too well but could this cause problems with syncs? (I have no idea how subtree syncs were tbh, is it just a merge commit?)

[bjorn3]

I have no idea how subtree syncs were tbh, is it just a merge commit?

For rust-analyzer -> rust it is a merge commit created using the subtree merge strategy. For rust -> rust-analyzer it synthesizes new commits. Not sure if the commits in this PR will conflict.

[lnicola]

Yeah, I think the sync back looks like this:

a99a48e78 (HEAD -> sync-from-rust, mine/sync-from-rust) :arrow_up: rust-analyzer
8536eb016 rename rustc_allocator_nounwind to rustc_nounwind
4f55ebbd4 :arrow_up: rust-analyzer
3a57388d1 update to syn-1.0.102
ed532e5a3 Fix duplicate usage of `a` article.
6f13f1230 rustc_typeck to rustc_hir_analysis
f5fde4df4 :arrow_up: rust-analyzer
459bbb422 :arrow_up: rust-analyzer
65e1dc4d9 :arrow_up: rust-analyzer
c1918fcb9 Auto merge of #100210 - mystor:proc_macro_diag_struct, r=eddyb
3e358a682 :arrow_up: rust-analyzer
31519bb39 :arrow_up: rust-analyzer
134701885 Rollup merge of #100643 - TaKO8Ki:point-at-type-parameter-shadowing-another-type, r=estebank
3a1aa376c avoid a `&str` to `String` conversion
8231fee46 :arrow_up: rust-analyzer
22c8c9c40 :arrow_up: rust-analyzer
2c7f2c105 proc_macro/bridge: send diagnostics over the bridge as a struct
9d2cb42a4 :arrow_up: rust-analyzer
a1f1b95d0 Merge commit 'e36a20c24f35a4cee82bbdc600289104c9237c22' into ra-sync-and-pms-component
dfe84494c Make macros test order-resistant
56c369db4 Sort when iterating through CrateGraph
d8c0d88e4 Sort in DefMap::dump, since HashMap iteration order isn't defined
ff317858c hir-def tests: sort results before comparing, since FxHashSet iteration order isn't guaranteed
74998e46e Fix .gitattributes for test_data
20eb2ddb2 Small fixups
[...]

[lnicola]

r? @fasterthanlime, maybe?

[fasterthanlime]

Just discovering this PR now — does the original PR description mean that following this guide didn't work? (I'm not sure what "merge" means in that context) https://doc.rust-lang.org/clippy/development/infrastructure/sync.html#performing-the-sync-from-rust-langrust-to-clippy

Cherry-picking seems like a bad idea and I'd rather ping some folks working on clippy/other subtrees before merging this.

[lnicola]

I cherry picked some commits from the branch created by git subtree. But I can't merge the master branch into that one.

[fasterthanlime]

it seemed to give conflicts on files only modified on our side, like crates/project-model/Cargo.toml

can't those merge conflicts be fixed? I feel like I should try to do the sync myself to see exactly what's going on.

[lnicola]

I tried, but there were too many. It seemed to me that some of them were in files not touched by the downstream commits.

[bjorn3]

Whenever syncing from rust-analyzer -> rust you have to immediately sync the merge commit from rust -> rust-analyzer to prevent merge conflicts in the future.

[fasterthanlime]

I unfortunately won't have time to look more into this until next week, sorry - pinging @oli-obk for additional git-subtree expertise.

[oli-obk]

yea I think you need to either redo a rustc->here sync right after the last here->rustc sync or move to using https://github.com/josh-project/josh like miri did: https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.md#advanced-topic-syncing-with-the-rustc-repo

[lnicola]

Is there any way to fix this now that I've missed some syncs? I think I tried it right after a filing a sync PR, but before it being merged. Cherry-picking the commits doesn't sound too bad to me in the meanwhile.

On the other hand, we should probably give josh a try.

[oli-obk]

So is there any way to fix this now that I've missed some syncs

hmm... I'm not sure. You could try to do a sync in the other direction at each of the past syncs and basically rebuild what was "lost". But that could be a lot of work

[lnicola]

I'm still inclined to merge this. If we ever make it work in the future, they should "go away" and if not, the rust-lang/rust side isn't that hard to handle in the same way (with cherry-picks).

[fasterthanlime]

I'm still inclined to merge this. If we ever make it work in the future, they should "go away" and if not, the rust-lang/rust side isn't that hard to handle in the same way (with cherry-picks).

Now that I'm full-time on open-source stuff (among other things), I actually do have time to work on this — do we have time for me to take a closer look, or should I do so after this is merged?

josh looks interesting, if it fixes most of our subtree woes, I would definitely invest effort into that.

[lnicola]

No hurry there. The commits here are pretty old and I don't think we've missed them.

There's at least one more I didn't pick because it was a mass replace in a test fixture.

[fasterthanlime]

So I gave josh a quick try (using RalfJung's fork), with:

$ cd rust-analyzer/
$ JOSH_FILTER=":at_commit=a63222cd240d9b5405826783603f3b391c90885d[:prefix=src/tools/rust-analyzer]:/src/tools/rust-analyzer"
$ git fetch http://localhost:8000/rust-lang/rust.git$JOSH_FILTER.git master
From http://localhost:8000/rust-lang/rust.git:at_commit=a63222cd240d9b5405826783603f3b391c90885d[:prefix=src/tools/rust-analyzer]:/src/tools/rust-analyzer                                    * branch                master     -> FETCH_HEAD
$ git merge FETCH_HEAD --no-ff -m "Merge from rustc"
fatal: refusing to merge unrelated histories

(Where https://github.com/rust-lang/rust/commits/a63222cd240d9b5405826783603f3b391c90885d is the first commit in rust-lang/rust where the rust-analyzer subtree history begins — not 100% sure that's right)

Passing --allow-unrelated-histories made the merge succeed, although, with numerous conflicts, which I imagine looks like what you saw @lnicola:

https://gist.github.com/fasterthanlime/bb66f688764de5b4223b715e1704dbad

I think I could go through all these merge conflicts if we were convinced that was the way forward? Here's a sample one:

I think like often the "Current change" is the one we want to keep:

...there might even be a git option to do just that...

[fasterthanlime]

Just checking in - do we think the way forward is switching to josh + resolving merge conflicts once? I have time to resolve the merge conflicts, it's time-consuming and annoying but if we do it once and "do it right" moving forward, it might be worth it, I just want to confirm that it's what we think is the right thing to do.

[lnicola]

Some quick (but still late) thoughts:

• I'm not sure we're getting the same conflicts, but it's likely
• the conflicts don't really make sense to me
• git isn't great IMHE at remembering how you resolved conflicts
• cherry picking as I did here seems the right thing to do: we might not want every commit from the downstream, cherry picking worked and preserved authorship without causing any conflicts
• I suspect I merged the two branches in the wrong direction, or something like that, which caused the conflicts

So josh might be nice to avoid patching git-subtree after every update, but I don't think it's worth spending time on those conflicts that don't even appear to make sense. Also, there's no guarantee we won't need to do it again on the next merge.

[fasterthanlime]

So josh might be nice to avoid patching git-subtree after every update, but I don't think it's worth spending time on those conflicts that don't even appear to make sense. Also, there's no guarantee we won't need to do it again on the next merge.

That's actually my question: if we resolve the conflicts now and "do it right" moving forward, will it be the last time we have to deal with merge conflicts? (Unless there were actual conflicting changes, which I don't think is what happened there).

And also: if we do cherry pick those commits, will the next time we run a git-subtree or josh sync be conflict-free? Could we make sure that after cherry-picking, the file trees under rust-lang/rust-analyzer and rust-lang/rust/src/tools/rust-analyzer are the same? josh lets us specify a commit to start from, so if the contents are the same, maybe we would be able to have a conflict-free life going forward.

Maybe @RalfJung knows more? (since their fork of josh includes additional functionality)

[RalfJung]

Josh needs to be used with care. :) And you cannot mix josh with other sync strategies, it's use only-josh or no-josh. Using josh here means you should stop using git subtree entirely, and you sholuldn't do manual cherry-picking. Also no need to use my fork, josh master has all the patches (and my fork is probably outdated at this point).

The key thing to ensure with josh is that extracting the history from rustc must exactly produce the history in this repo.

When was the last time RA got pushed to rustc, and which RA commit was the one that got pushed? The first order of business is to ensure that extracting RA from rustc with josh yields a history that contains that commit. Since you used git subtree in the past, this will require a rev filter like in Miri, since josh and git subtree differ in how RA commits look like in rustc: with git subtree they contain just the subtree, but with josh they actually contain all of rustc.

In Miri we use the filter

:rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri

where 75dd959a3a40eb5b4574f8d2e23aa6efbeb33573 is the last (most recent) Miri commit in rustc that was created by git subtree. So this basically tells josh to move the git subtree part of the Miri history in rustc into a subfolder (where it would be if that history was created with josh), and then to project to that subfolder (the normal procedure with josh), which means we get a full Miri history that looks exactly like what we have in the Miri repo.

Once you have that first josh fetch figured out, you should produce a merge of that with RA master, resolving any conflicts that have occurred. Those should be real conflicts, if there are nonsense conflicts here then likely something went wrong. Prepare that commit but do not merge this in RA yet, you want to be sure this all went well before permanently merging something into RA! Merging the wrong thing can make it permanently impossible to use josh properly, fixable only with a force-push. Still it's a good idea to make a GitHub PR at this point, as that will show very nicely which new commits are being added, so it's a good sanity-check to look at the new commits and the diff.

To make sure it all went well, you want to test that pushing this new version of RA to rustc via josh (a) works, and (b) round-trips, i.e. fetching that indeed yields the new RA master you constructed. It's also worth opening a rustc PR for the push, again just to sanity-check things by looking at the new commits and the diff. If anything seems odd, stop and investigate.

The josh parts of the ./miri script contain the lessons we learned so far for how to best use josh. I would suggest you use a script for that as well. Note that we are using the fact that Miri already has a file that tracks the rustc git commit to use for CI; we now define that this file always contains the last rustc git ID we pulled via josh -- this helps a lot with josh pushes, because it means we can base them off of that, rather than off of rustc master, which makes josh's life a lot easier.

Also I think it is a bad idea to have pull and push syncs that race: at any point in time, there should either be an open PR in the RA repo, waiting to merge the latest rustc-pull, or an open PR in the rustc repo, waiting to merge the latest rustc-push. I have not tried what happens when these happen in parallel, but it seems like trouble worth avoiding. ;) In fact what I usually do is that I do a pull immediately before each push. IOW, when I push, the commit I am pushing is a just-merged rustc-pull. pushing is the much harder less well-defined task for josh, so this ensures that that has the highest chance of succeeding. I do not know if it is strictly required to be so careful, but it seems to help avoid josh-project/josh#998 and also we had 2 force-pushes in Miri recently and I'd rather avoid having more. ;) (So this is basically the opposite of what @bjorn3 said about subtree -- with josh, you want to pull immediately before you push, but I don't think you need to immediately pull after you pushed.)

(Where https://github.com/rust-lang/rust/commits/a63222cd240d9b5405826783603f3b391c90885d is the first commit in rust-lang/rust where the rust-analyzer subtree history begins — not 100% sure that's right)

Nope it's wrong, you want the last (most recent) subtree commit.

Passing --allow-unrelated-histories made the merge succeed

If you needed that flag it means the roundtrip failed, so something went wrong. (Not surprising given the wrong filter.)

[RalfJung]

Oh I should also say that I consider josh to still be on probation in Miri. We had some annoying trouble last week, after which we updated our scripts and a josh bug got fixed and hopefully that is dealt with now. But if this happens again we'll probably give up on trying to sync history, and do something home-brew that just tries to get the file contents back and forth.

If you wait a month I can tell you more, but at this point I cannot yet wholeheartedly recommend switching to josh. Once you merge any of the josh-produced history, I am not sure if switching back to git subtree is possible. In Miri, git subtree just doesn't work so we went with josh for lack of alternatives.

So it depends on how fed up you are with git subtree -- if switching to a home-brew hack in case josh turns out to not work well enough is okay for you, go ahead. If you'd rather stick to git subtree than risk having to stop syncing history then I suggest you wait a bit and see how things go in Miri.

[fasterthanlime]

@RalfJung thanks for the thoughtful response: what I'm getting from this is "don't switch to josh yet because it can lock us out of git-subtree".

My only remaining question is: if we do these cherry-picks, can we still use git-subtree later or are we forever locked into doing these manual cherry-picks (unless we're willing to force-push rust-lang/rust-analyzer?)

If not, then apologies for wasting everyone's time and let's move forward with the cherry-picking.

[RalfJung]

I know nothing about git subtree so I can't help with that question.

If this was josh, then I think cherry-picks would lead to annoying conflicts on the next rustc-push and to these commits being duplicated, but the situation should stabilize afterwards.

[fasterthanlime]

Update: I'm trying something locally with some external help. Please don't merge this today (but if I don't report back before monday, feel free).

Edit: I'm cautiously optimistic, will open PR when done.

Edit 2: whoomp there it is: #13676

[Veykril]

Closing in favor of #13676