rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.05k stars 1.56k forks source link

`josh` syncs are broken #17733

Closed lnicola closed 3 weeks ago

lnicola commented 1 month ago
$ cargo xtask rustc-push --rust-path ../rust-rust-analyzer --rust-fork lnicola/rust
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/xtask rustc-push --rust-path ../rust-rust-analyzer --rust-fork lnicola/rust`
Preparing lnicola/rust (base: 1b51d80027919563004918eaadfa0d890ac0eb93)...
$ git fetch https://github.com/rust-lang/rust 1b51d80027919563004918eaadfa0d890ac0eb93
From https://github.com/rust-lang/rust
 * branch                    1b51d80027919563004918eaadfa0d890ac0eb93 -> FETCH_HEAD
$ git push https://github.com/lnicola/rust 1b51d80027919563004918eaadfa0d890ac0eb93:refs/heads/sync-from-ra

Pushing rust-analyzer changes...
$ git push http://localhost:42042/lnicola/rust.git:rev(55d9a533b309119c8acd13061581b43ae8840823:prefix=src/tools/rust-analyzer):/src/tools/rust-analyzer.git HEAD:sync-from-ra
Enumerating objects: 4740, done.
Counting objects: 100% (3116/3116), done.
Delta compression using up to 32 threads
Compressing objects: 100% (1309/1309), done.
Writing objects: 100% (2536/2536), 533.09 KiB | 21.32 MiB/s, done.
Total 2536 (delta 1728), reused 1913 (delta 1146), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (1728/1728), completed with 463 local objects.
remote: josh-proxy: pre-receive hook
remote: upstream: response status: 200 OK
remote: upstream: response body:
remote: 
remote: To https://github.com/lnicola/rust.git
remote:    1b51d800279..1056720a1fd  JOSH_PUSH -> sync-from-ra
remote: REWRITE(fd74511f34ae6c165466543cc6e55ea60e7365af -> 338888132703ff39b0cb7fdcd7b02b8425088da9)
To http://localhost:42042/lnicola/rust.git:rev(55d9a533b309119c8acd13061581b43ae8840823:prefix=src/tools/rust-analyzer):/src/tools/rust-analyzer.git
   d401839753..fd74511f34  HEAD -> sync-from-ra

Error: Josh created a non-roundtrip push! Do NOT merge this into rustc!

I'm not sure what changed, this worked well until today.

CC @RalfJung

RalfJung commented 1 month ago

What's your josh version?

lnicola commented 1 month ago

josh-proxy v22.4.15 (https://github.com/josh-project/josh#012f8dcc)

But I also tried https://github.com/josh-project/josh/commit/60dc5873ef26f175152aa6ad81d39de76d2a4950 without any luck. I did clear my cache both before and after upgrading.

RalfJung commented 1 month ago

Strangely I am getting a different error:

Pushing rust-analyzer changes...
$ git push http://localhost:42042/RalfJung/rust.git:rev(55d9a533b309119c8acd13061581b43ae8840823:prefix=src/tools/rust-analyzer):/src/tools/rust-analyzer.git HEAD:sync-from-ra
To http://localhost:42042/RalfJung/rust.git:rev(55d9a533b309119c8acd13061581b43ae8840823:prefix=src/tools/rust-analyzer):/src/tools/rust-analyzer.git
 ! [rejected]              HEAD -> sync-from-ra (fetch first)
error: failed to push some refs to 'http://localhost:42042/RalfJung/rust.git:rev(55d9a533b309119c8acd13061581b43ae8840823:prefix=src/tools/rust-analyzer):/src/tools/rust-analyzer.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally. This is usually caused by another repository pushing to
hint: the same ref. If you want to integrate the remote changes, use
hint: 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

It's not even getting to the round-trip check, it already fails to push before because it produced something that's not a fast-forward from the base commit in rust-version. I'm getting this even for old versions of RA, like when I go back to commit ffbc5ad (which I guess was the basis for pushing https://github.com/rust-lang/rust/pull/127617), even after removing the cache.

Not sure what's up with that, I won't have the time to dig into this. This looks like a josh bug to me so I'd suggest reporting it upstream.

EDIT: I may be using a too-old josh, since Miri doesn't need the josh patch that was landed to make RA work initially. I'll try again with a current version of josh.

RalfJung commented 1 month ago

With the right version of josh, I tried pushing various commits in the repo, and the first one that fails is 0ba6f4eda0, the commit that landed https://github.com/rust-lang/rust-analyzer/pull/17676. Looking at what is different between the round-tripped version and the original, 5af51fa is the first commit that didn't make it.

Original commit info:

commit 5af51fa47610d0f88dcab99aaaff2ee4fca17c00
tree 5cf661188e20157ffab5d742ce22d5fa44d18bd1
parent 4a91453409aaf58eba0a91e4b62a35f49814b394
author Winston H. <56998716+winstxnhdw@users.noreply.github.com> 1721814140 +0100
committer GitHub <noreply@github.com> 1721814140 +0100

After extracted from josh:

commit fc919994d0759bca7423444e0d0bc044df80359a
tree 5cf661188e20157ffab5d742ce22d5fa44d18bd1
parent 4a91453409aaf58eba0a91e4b62a35f49814b394
author Winston H <56998716+winstxnhdw@users.noreply.github.com> 1721814140 +0100
committer GitHub <noreply@github.com> 1721814140 +0100

The . after Winston H went missing.

Filed upstream as https://github.com/josh-project/josh/issues/1345.

RalfJung commented 1 month ago

With https://github.com/josh-project/josh/pull/1346, josh syncs are working again with RA.

lnicola commented 1 month ago

Thanks for saving the day again! Sync PR at https://github.com/rust-lang/rust/pull/128490.