Revert "fix download-llvm logic for subtree sync branches #137593"

[jieyouxu]

Reverts #137593.

It's likely because the perf machine has an old git version that doesn't have the --diff-merges option or its forwarding in git rev-list yet...

This reverts commit 95994f9, reversing changes made to 7290b04.

This will unfortunately re-open #101907.

cc @RalfJung @Mark-Simulacrum for FYI
r? @onur-ozkan (or bootstrap or infra or anyone really)

[jieyouxu]

@bors p=101

[jieyouxu]

FTR, I'm posting a revert instead of a fix-forward because I know absolutely nothing about git show --diff-merge=first-parent vs git rev-list --first-parent at all.

[dianqk]

@bors r+ rollup=never
perf is broken.

[bors]

📌 Commit c71569a has been approved by dianqk

It is now in the queue for this repository.

[bors]

⌛ Testing commit c71569a with merge f1cf624...

[Kobzol]

#138591 will finally remove all these git hacks :)

[RalfJung]

Hm, very strange, I tried this locally with git rev-list and it worked. The docs do indeed not reflect this, but this command works here:

git rev-list [email protected] -n1 716dd22 --diff-merges=first-parent -- src/llvm-project src/bootstrap/download-ci-llvm-stamp src/version

[RalfJung]

Also, why did the PR even land if it completely breaks the invocation?!?

[Kobzol]

I think that the perf. collector machine just has an old version of git.

This code should be nuked soon, hopefully, so I don't think it's worth fixing too much.

[jieyouxu]

Hm, very strange, I tried this locally with git rev-list and it worked.

I'm not sure how it works for you locally, that is bizzare... https://git-scm.com/docs/git-rev-list has no results if I grep --diff-merges. Looking at the history, AFAICT it never was a valid option, e.g. git/git@452d264 from like 4 years ago.

[RalfJung]

Can you just try running the command I posted above locally in your rustc checkout? With git 2.47.2 it seems to just work. 🤷

[jieyouxu]

$ git rev-list [email protected] -n1 716dd22 --diff-merges=first-parent -- src/llvm-project src/bootstrap/download-ci-llvm-stamp src/version
ce36a966c79e109dabeef7a47fe68e5294c6d71e

That is bizarre, it works for me too...

[workingjubilee]

On the git show page:
@jieyouxu

This manual page describes only the most frequently used options.

This is so with most of the manual pages. In fact, a few commands have almost-perfect supersets of another command's option list.

[jieyouxu]

Which means this revert may not be reverting the right PR... But then looking at the rollup I can't tell if another PR could break the perf

[jieyouxu]

@bors retry r-

[RalfJung]

This is probably the right PR to revert, the perf machine might just have an old version of git (as @Kobzol already hypothesized). Maybe the CI log prints the git version somewhere?

So I'd say let's land this revert, and hope for @Kobzol's refactor to also fix my issue.

[jieyouxu]

@bors r=dianqk

[bors]

📌 Commit c71569a has been approved by dianqk

It is now in the queue for this repository.

[bors]

⌛ Testing commit c71569a with merge b95aac6...

[workingjubilee]

Even if this isn't the right PR to revert, as long as the tree is closed due to a rollup, I can't see any harm in filing a revert of each PR, one by one, until we find the one that did it.

[Kobzol]

The perf machine has git 2.17.1.

[jieyouxu]

The perf machine has git 2.17.1.

[RalfJung]

That git seems to be more than 6 years old... So yeah quite possible that it just lacks this flag.

[bors]

☀️ Test successful - checks-actions
Approved by: dianqk
Pushing b95aac6 to master...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing ae8ab87 (parent) -> b95aac6 (this PR)

Test differences

No test diffs found

[rust-timer]

Finished benchmarking commit (b95aac6): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -3.0%, secondary -2.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.3%	[-7.3%, -7.3%]	1
Improvements ✅ (secondary)	-2.6%	[-3.0%, -2.1%]	2
All ❌✅ (primary)	-3.0%	[-7.3%, 1.3%]	2

Cycles

Results (primary 3.5%, secondary 2.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.5%	[1.3%, 4.9%]	4
Regressions ❌ (secondary)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.5%	[1.3%, 4.9%]	4

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: missing data
Artifact size: 365.48 MiB -> 365.48 MiB (0.00%)