add fallback logic to git_upstream_merge_base#137994
add fallback logic to git_upstream_merge_base#137994onur-ozkan wants to merge 1 commit intorust-lang:masterfrom
git_upstream_merge_base#137994Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
rustbot
added
S-waiting-on-review
onur-ozkan
force-pushed
the
git-first-parent-fallback
branch
2 times, most recently
from
8395e9b to
3ea9ed3
Compare
Signed-off-by: onur-ozkan <work@onurozkan.dev>
onur-ozkan
force-pushed
the
git-first-parent-fallback
branch
from
3ea9ed3 to
ee18c39
Compare
Mark-Simulacrum
left a comment
Mark-Simulacrum
left a comment
There was a problem hiding this comment.
I am personally reluctant to poke this in a hot-fix kind of way. CI does need different treatment, but merge-base being needed only in CI seems highly suspicious to me. I don't think we've landed on the right strategy for commit discovery, and I worry about incrementally adding conditions to it without a cohesive narrative on what we're trying to accomplish and without tests (even manual ones!).
| // outdated very quickly. | ||
| "HEAD".to_string() | ||
| { | ||
| git_upstream_merge_base(config, git_dir).unwrap() |
There was a problem hiding this comment.
As mentioned in my comment (#101907 (comment), footnote 1) this never executes in practice in (our) CI.
There was a problem hiding this comment.
Are you sure? We are running that on every pipeline (unless it's beta/stable channel) as far as I know.
There was a problem hiding this comment.
As Mark explained, channel is "nightly\n", so I don't think we are...
There was a problem hiding this comment.
Ooh, what the hack... lol
There was a problem hiding this comment.
Maybe that is the reason why sometimes CI fails?
I am fine with not merging this right away. I tried to unblock the contribution roadblock with the first solution that came to my mind but I agree that if it's not very critical, it's better not to apply a workaround. |
|
☔ The latest upstream changes (presumably #138127) made this pull request unmergeable. Please resolve the merge conflicts. |
|
#138591 will hopefully improve and handle git logic properly and make this hack unnecessary. |
5 participants
Fixes #101907
cc @RalfJung