Elijah Newren wrote:
On Fri, Jun 11, 2021 at 7:25 AM Felipe Contreras [off-list ref]
wrote:
quoted
Personally I have never experienced what you posted, so maybe there's
something else happening behind the scenes.
Maybe merge-ort changed something.
merge-ort made no changes relative to content merges or choice of merge
bases. (In fact, merge ort doesn't even handle content merges; that's the
xdiff layer.) Even if merge-ort had made changes in this area, merge-ort
is not the default and I didn't see the necessary config tweaks in your
list of config options. (I would have recommended against people using
merge ort until 7bec8e7fa6 ("Merge branch 'en/ort-readiness'", 2021-04-16),
which only made it into a release last week with 2.32. I probably won't be
recommending it as the default at least until the optimization work is
merged and it's hard to predict how many more months that will take.)
Indeed, I tested on v2.25 and found the same output.
I thought of merge-ort because 1) I've never seen such kind of output
before, and 2) grepping the code I thought I saw merge-ort being the
default of something, but now I seem to be unable to find where.
It's more likely that the codebases you work with just don't have
criss-cross merges.
Yes, that's it.
I don't see why people in these kinds of codebases would like diff3
doing that by default.
Cheers.
--
Felipe Contreras
On Fri, Jun 11, 2021 at 10:32 AM Felipe Contreras
[off-list ref] wrote:
Elijah Newren wrote:
quoted
On Fri, Jun 11, 2021 at 7:25 AM Felipe Contreras [off-list ref]
wrote:
quoted
quoted
Personally I have never experienced what you posted, so maybe there's
something else happening behind the scenes.
Maybe merge-ort changed something.
merge-ort made no changes relative to content merges or choice of merge
bases. (In fact, merge ort doesn't even handle content merges; that's the
xdiff layer.) Even if merge-ort had made changes in this area, merge-ort
is not the default and I didn't see the necessary config tweaks in your
list of config options. (I would have recommended against people using
merge ort until 7bec8e7fa6 ("Merge branch 'en/ort-readiness'", 2021-04-16),
which only made it into a release last week with 2.32. I probably won't be
recommending it as the default at least until the optimization work is
merged and it's hard to predict how many more months that will take.)
Indeed, I tested on v2.25 and found the same output.
I thought of merge-ort because 1) I've never seen such kind of output
before, and 2) grepping the code I thought I saw merge-ort being the
default of something, but now I seem to be unable to find where.
We have briefly discussed multiple times what things might be needed
to eventually make merge-ort the default (though it's not even
complete yet; I'm five months into upstreaming the optimization
patches with an unknown number of months left). There were also a
couple patches to make the _tests_ default to using merge-ort on most
platforms, while still keeping one suite that tests merge-recursive to
ensure we don't add breakage. Perhaps one of those is what you're
thinking of?
quoted
It's more likely that the codebases you work with just don't have
criss-cross merges.
Yes, that's it.
I don't see why people in these kinds of codebases would like diff3
doing that by default.
I suspect they don't[1]. What's the alternative, though? Not using
diff3? Picking a different base to avoid the occasional nested
conflict in the inner merge region, but which overall has much worse
other side effects? I think Junio was addressing this when he
recently said elsewhere in this thread that "Rejecting diff3 style
output because of the way a conflicted part in the inner merge appears
as a common ancestor version may be throwing the baby out with the
bathwater"[2]. Sure, it's an annoyance, but diff3 is still a good
option and there is no current solution to the annoying nested merge
display.
Also, not sure whether it matters or not, but just for completeness I
should point out that you can get nested conflict markers without
criss-cross merges and without merge.conflictStyle=diff3. It's just
much more rare.
[1] https://lore.kernel.org/git/CAPc5daUC+6cHpexXTO24p4mG_5eL1JmxrYm8h3UfdTh_FMka=w@mail.gmail.com/
[2] https://lore.kernel.org/git/xmqqh7i5ci3t.fsf@gitster.g/
Elijah Newren wrote:
On Fri, Jun 11, 2021 at 10:32 AM Felipe Contreras
[off-list ref] wrote:
quoted
Elijah Newren wrote:
quoted
On Fri, Jun 11, 2021 at 7:25 AM Felipe Contreras [off-list ref]
wrote:
quoted
quoted
Personally I have never experienced what you posted, so maybe there's
something else happening behind the scenes.
Maybe merge-ort changed something.
merge-ort made no changes relative to content merges or choice of merge
bases. (In fact, merge ort doesn't even handle content merges; that's the
xdiff layer.) Even if merge-ort had made changes in this area, merge-ort
is not the default and I didn't see the necessary config tweaks in your
list of config options. (I would have recommended against people using
merge ort until 7bec8e7fa6 ("Merge branch 'en/ort-readiness'", 2021-04-16),
which only made it into a release last week with 2.32. I probably won't be
recommending it as the default at least until the optimization work is
merged and it's hard to predict how many more months that will take.)
Indeed, I tested on v2.25 and found the same output.
I thought of merge-ort because 1) I've never seen such kind of output
before, and 2) grepping the code I thought I saw merge-ort being the
default of something, but now I seem to be unable to find where.
We have briefly discussed multiple times what things might be needed
to eventually make merge-ort the default (though it's not even
complete yet; I'm five months into upstreaming the optimization
patches with an unknown number of months left). There were also a
couple patches to make the _tests_ default to using merge-ort on most
platforms, while still keeping one suite that tests merge-recursive to
ensure we don't add breakage. Perhaps one of those is what you're
thinking of?
Nah, I think I just read something wrong.
quoted
quoted
It's more likely that the codebases you work with just don't have
criss-cross merges.
Yes, that's it.
I don't see why people in these kinds of codebases would like diff3
doing that by default.
I suspect they don't[1]. What's the alternative, though? Not using
diff3? Picking a different base to avoid the occasional nested
conflict in the inner merge region, but which overall has much worse
other side effects?
Picking a different conflict style for the inner merge.
I think Junio was addressing this when he
recently said elsewhere in this thread that "Rejecting diff3 style
output because of the way a conflicted part in the inner merge appears
as a common ancestor version may be throwing the baby out with the
bathwater"[2]. Sure, it's an annoyance, but diff3 is still a good
option and there is no current solution to the annoying nested merge
display.
Yes, but he proceeded to say we could not recommend diff3 to new users,
so I see the baby out on the floor.
I don't think it's likely new users will experience a problem with diff3
inner conflict markers (in 15 years of using git I've never encountered
them myself [or I blocked them out my mind]).
Even with this problem, experienced developers seem to be unable to live
without diff3, so I don't know if there's a valid argument against
making it the default.
Yes, improving inner conflict markers would make life better for new
users, but also everyone else. Software can always be better.
Let's not make perfect be the enemy of the good; diff3 is more than good
enough already.
Cheers.
--
Felipe Contreras