Repository navigation
Merge commits break LLVM CI download #101907
Description
While PRs are not allowed to have merge commits, they can still be useful locally (or for not rewriting history for longer-running branches).
However, they break the commit detection logic for downloading CI artifacts.
Steps to Reproduce
git checkout <old-commit>git merge --no-ff master./x.py build library/std
Expected Result
Builds fine.
Actual Result
...
downloading https://ci-artifacts.rust-lang.org/rustc-builds/<old-commit>/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
curl: (22) The requested URL returned error: 404 #
error: failed to download llvm from ci
help: old builds get deleted after a certain time
help: if trying to compile an old commit of rustc, disable `download-ci-llvm` in config.toml:
[llvm]
download-ci-llvm = false
Build completed unsuccessfully in 0:01:43
Activity
Mark-Simulacrum commentedon Sep 16, 2022
Can you say more about their usefulness? What are you merging in? We can maybe support merging non-bors commits, though it brings some complexity (e.g., which parent do we follow for the "nearest" bors commit, and what does nearest mean at that point), so in general I would be inclined to say this isn't supported and you should build locally.
jachris commentedon Sep 16, 2022
I don't think it's a must-have. I'm personally fine with building locally or rebasing. But this would allow longer-running branches to use the CI artifacts without rewriting history due to rebasing. Also, the current behavior can be a little bit confusing if one is not aware of this issue.
RalfJung commentedon Oct 6, 2022
Yeah it is confusing. :D Could you explain what happens?
Is this about having to detect the "latest commit from rustc master that is in this branch", so that we can download the right LLVM, and that fails?
jachris commentedon Oct 6, 2022
From what I can tell, yes. The logic that determines that commit seems to not account for merge commits. But @jyn514, who helped me troublshoot this issue initially, might know the details.
[-]Merge commits break CI download[/-][+]Merge commits break LLVM CI download[/+]RalfJung commentedon Oct 22, 2022
I now also ran into this because syncing Miri changes back into rustc creates merge commits. Not sure how to work around this problem; rebasing is not an option for these syncs and a local build takes forever...
Cc @jyn514
RalfJung commentedon Oct 22, 2022
Specifically that Miri history contains commits made by bors in Miri, which then fools the detection logic to think that was a rustc bors commit.
RalfJung commentedon Oct 22, 2022
Currently, bootstrap uses the following command to determine the commit to fetch LLVM for
I can see two ways of making things work even for the Miri sync situation:
-n1, use-n10or so and try several commits. For the case I have right now, the 2nd commit should already work.HEAD, use$(git merge-base HEAD origin/master). However we can't really know what theoriginremote is called, so it'd be more likejachris commentedon Oct 22, 2022
I think the second option would be a solid solution, and it also works for the usecase I had in mind.
(Side note: Though I'm sure why this uses
--first-parent.If we are onIf we are on another branch, it discards merges into that branch... which is basically the original issue.)master, there should be no merge commits, and this has no effect.RalfJung commentedon Oct 22, 2022
On master there are tons of merge commits -- the ones created by bors. In fact the commit we are looking for is always a merge commit, so maybe we should add
--merges.jachris commentedon Oct 22, 2022
Ah yes, right, of course. The "no merge-commit policy" is only for PRs... Nevermind.
Mark-Simulacrum commentedon Oct 22, 2022
Fetching introduces a network call on each x.py invocation, I think, since we need to know if something has changed. I guess we could try to cache based on the head sha, but I'm not generally a fan of that.
I think the right answer is probably to change bors to include a repository in the commits it generates more explicitly - perhaps the email could be bors+rust@rust-lang.org, for example.
Would need some care to update all of our automation, but seems achievable.
RalfJung commentedon Oct 22, 2022
Yeah I agree having to invoke the network is not great. In principle we could search the user's remotes for one that points to our github repo and use
$remote/master... but that seems pretty fragile.Having different bors email addresses for different repos would help for the Miri case, yeah. And I guess for @jachris we could replace the
--first-parentby--merges; the email filtering should be enough to trim down the candidates.@Mark-Simulacrum I take it you are also not a fan of trying multiple commit candidates? I guess that would require caching negative lookups so it's also not great.
A temporary hack that would be helpful is some way to overwrite the
HEADcommit from which it starts searching. For the Miri PR I just locally patched bootstrap to be able to dox.py checkwhich I needed to update the lockfile.Mark-Simulacrum commentedon Oct 22, 2022
Updating the lockfile should be cargo metadata >/dev/null or something like that, no need for x.py.
Yeah, multiple candidates I think has similar problems in terms of network hits (or negative cache of some kind).
--merges seems fine to me, but I'm not sure we can do away with --first-parent - for example, in the case of rollups, we need to be sure we're looking at the first parent commit to traverse the commit graph. I guess with -n1 that might not matter but it feels iffy to me.
Can you point me at the SHA/branch for the miri merges that are causing trouble? I wonder if the merge commits might be distinguishable by (for example) not having a tree associated with them that contains src/stage0.json or something like that.
49 remaining items
Auto merge of rust-lang#138878 - jieyouxu:revert-ci-llvm, r=dianqk
RalfJung commentedon Mar 24, 2025
Where can I see the failing CI log that is fixed by the revert?
Auto merge of rust-lang#138878 - jieyouxu:revert-ci-llvm, r=dianqk
jieyouxu commentedon Mar 24, 2025
(Asking in #t-infra > benchmarks failed?)
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=<try>
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=<try>
Auto merge of rust-lang#138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of #138591 - Kobzol:git-ci, r=Mark-Simulacrum
Auto merge of #138591 - Kobzol:git-ci, r=Mark-Simulacrum