{"thread":{"id":"57018","subject":"Confusing (maybe wrong?) conflict output with ort","startedAt":"2021-12-02T19:08:36Z","lastAt":"2021-12-03T00:40:18Z","messageCount":3,"participants":["Alex Xu (Hello71)","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"442911","messageId":"1638470726.ql5i6zljva.none@localhost","threadId":"57018","inReplyTo":"1638470726.ql5i6zljva.none.ref@localhost","subject":"Confusing (maybe wrong?) conflict output with ort","fromName":"Alex Xu (Hello71)","fromEmail":"alex_y_xu@yahoo.ca","sentAt":"2021-12-02T19:08:20Z","receivedAt":"2021-12-02T19:08:36Z","isPatch":false,"sender":{"key":"alex_y_xu@yahoo.ca","avatar":null},"body":"Hi all,\n\nAfter upgrading to git 2.34.1, I tried to rebase [0] onto [1], but \nencountered \"strange\" conflict results.\n\ngit rebase -s recursive main produces [[RECURSIVE]]. It is roughly what \nI expected to be output. If I take all the changes from the upper \nsection of the conflict, my changes will be effectively undone. If I \ntake all the changes from the lower section, then the upstream changes \nwill be undone.\n\nOn the other hand, running git rebase -s ort main produces [[ORT]]. I am \nunsure if it is wrong, strictly speaking, but it is certainly unexpected \nand difficult for me to resolve. Selecting the upper section of the \nconflict does erase my changes, as before, but selecting the lower \nsection results in syntactically incorrect code (foreach is ended by \nendif). The diff3 output makes even less sense to me.\n\nA script is attached to assist in reproducing my results. Running it \ninitializes the repository to the desired state. Then, run \"git rebase \n-s strategy master\" to produce the conflict.\n\nThanks,\nAlex.\n\n[0] https://gitlab.freedesktop.org/alxu/mesa/-/commit/4ad18ab613101e3489ca2d9e7151125f670e1ea5\n[1] https://gitlab.freedesktop.org/alxu/mesa/-/commit/c47fd3dc0062101b3e75a414b17d2765735f7424\n\n[[RECURSIVE]]\n\n<<<<<<< HEAD\nuse_elf_tls = true\npre_args += '-DUSE_ELF_TLS'\n\nif with_platform_android and get_option('platform-sdk-version') >= 29\n  # By default the NDK compiler, at least, emits emutls references instead of\n  # ELF TLS, even when building targeting newer API levels.  Make it actually do\n  # ELF TLS instead.\n  c_args += '-fno-emulated-tls'\n  cpp_args += '-fno-emulated-tls'\nendif\n\n# -mtls-dialect=gnu2 speeds up non-initial-exec TLS significantly but requires\n# full toolchain (including libc) support.\nhave_mtls_dialect = false\nforeach c_arg : get_option('c_args')\n  if c_arg.startswith('-mtls-dialect=')\n    have_mtls_dialect = true\n    break\n  endif\nendforeach\nif not have_mtls_dialect\n  # need .run to check libc support. meson aborts when calling .run when\n  # cross-compiling, but because this is just an optimization we can skip it\n  if meson.is_cross_build()\n    warning('cannot auto-detect -mtls-dialect when cross-compiling, using compiler default')\n  else\n    # -fpic to force dynamic tls, otherwise TLS relaxation defeats check\n    gnu2_test = cc.run('int __thread x; int main() { return x; }', args: ['-mtls-dialect=gnu2', '-fpic'], name: '-mtls-dialect=gnu2')\n    if gnu2_test.returncode() == 0\n      c_args += '-mtls-dialect=gnu2'\n      cpp_args += '-mtls-dialect=gnu2'\n=======\nuse_elf_tls = false\nif not with_platform_windows or not with_shared_glapi\n  pre_args += '-DUSE_ELF_TLS'\n  use_elf_tls = true\n\n  if with_platform_android and get_option('platform-sdk-version') >= 29\n    # By default the NDK compiler, at least, emits emutls references instead of\n    # ELF TLS, even when building targeting newer API levels.  Make it actually do\n    # ELF TLS instead.\n    c_args += '-fno-emulated-tls'\n    cpp_args += '-fno-emulated-tls'\n  endif\n\n  # -mtls-dialect=gnu2 speeds up non-initial-exec TLS significantly but requires\n  # full toolchain (including libc) support.\n  have_mtls_dialect = false\n  foreach c_arg : get_option('c_args')\n    if c_arg.startswith('-mtls-dialect=')\n      have_mtls_dialect = true\n      break\n    endif\n  endforeach\n  if not have_mtls_dialect\n    # need .run to check libc support. meson aborts when calling .run when\n    # cross-compiling, but because this is just an optimization we can skip it\n    if meson.is_cross_build()\n      warning('cannot auto-detect -mtls-dialect when cross-compiling, using compiler default')\n    else\n      # -fpic to force dynamic tls, otherwise TLS relaxation defeats check\n      gnu2_test = cc.run('int __thread x; int main() { return x; }', args: ['-mtls-dialect=gnu2', '-fpic'], name: '-mtls-dialect=gnu2')\n      # https://gitlab.freedesktop.org/mesa/mesa/-/issues/5665\n      if gnu2_test.returncode() == 0 and (\n           host_machine.cpu_family() != 'x86_64' or\n           # https://github.com/mesonbuild/meson/issues/6377\n           #cc.get_linker_id() != 'ld.lld' or\n           cc.links('''int __thread x; int y; int main() { __asm__(\n                  \"leaq x@TLSDESC(%rip), %rax\\n\"\n                  \"movq y@GOTPCREL(%rip), %rdx\\n\"\n                  \"call *x@TLSCALL(%rax)\\n\"); }''', name: 'split TLSDESC')\n           )\n        c_args += '-mtls-dialect=gnu2'\n        cpp_args += '-mtls-dialect=gnu2'\n      endif\n>>>>>>> 4ad18ab6131 (meson: check for lld split TLSDESC bug (fixes #5665))\n    endif\n  endif\nendif\n\n[[ORT]]\n\n# -mtls-dialect=gnu2 speeds up non-initial-exec TLS significantly but requires\n# full toolchain (including libc) support.\nhave_mtls_dialect = false\nforeach c_arg : get_option('c_args')\n  if c_arg.startswith('-mtls-dialect=')\n    have_mtls_dialect = true\n    break\n  endif\n<<<<<<< HEAD\nendforeach\nif not have_mtls_dialect\n  # need .run to check libc support. meson aborts when calling .run when\n  # cross-compiling, but because this is just an optimization we can skip it\n  if meson.is_cross_build()\n    warning('cannot auto-detect -mtls-dialect when cross-compiling, using compiler default')\n  else\n    # -fpic to force dynamic tls, otherwise TLS relaxation defeats check\n    gnu2_test = cc.run('int __thread x; int main() { return x; }', args: ['-mtls-dialect=gnu2', '-fpic'], name: '-mtls-dialect=gnu2')\n    if gnu2_test.returncode() == 0\n      c_args += '-mtls-dialect=gnu2'\n      cpp_args += '-mtls-dialect=gnu2'\n=======\n\n  # -mtls-dialect=gnu2 speeds up non-initial-exec TLS significantly but requires\n  # full toolchain (including libc) support.\n  have_mtls_dialect = false\n  foreach c_arg : get_option('c_args')\n    if c_arg.startswith('-mtls-dialect=')\n      have_mtls_dialect = true\n      break\n    endif\n  endforeach\n  if not have_mtls_dialect\n    # need .run to check libc support. meson aborts when calling .run when\n    # cross-compiling, but because this is just an optimization we can skip it\n    if meson.is_cross_build()\n      warning('cannot auto-detect -mtls-dialect when cross-compiling, using compiler default')\n    else\n      # -fpic to force dynamic tls, otherwise TLS relaxation defeats check\n      gnu2_test = cc.run('int __thread x; int main() { return x; }', args: ['-mtls-dialect=gnu2', '-fpic'], name: '-mtls-dialect=gnu2')\n      # https://gitlab.freedesktop.org/mesa/mesa/-/issues/5665\n      if gnu2_test.returncode() == 0 and (\n           host_machine.cpu_family() != 'x86_64' or\n           # https://github.com/mesonbuild/meson/issues/6377\n           #cc.get_linker_id() != 'ld.lld' or\n           cc.links('''int __thread x; int y; int main() { __asm__(\n                  \"leaq x@TLSDESC(%rip), %rax\\n\"\n                  \"movq y@GOTPCREL(%rip), %rdx\\n\"\n                  \"call *x@TLSCALL(%rax)\\n\"); }''', name: 'split TLSDESC')\n           )\n        c_args += '-mtls-dialect=gnu2'\n        cpp_args += '-mtls-dialect=gnu2'\n      endif\n>>>>>>> 4ad18ab6131 (meson: check for lld split TLSDESC bug (fixes #5665))\n    endif\n  endif\nendif\n"},{"id":"442924","messageId":"CABPp-BEyy97x-9od+9E9DxCDHHq-m8hyZJWqxFutkRy3G7Hu9g@mail.gmail.com","threadId":"57018","inReplyTo":"1638470726.ql5i6zljva.none@localhost","subject":"Re: Confusing (maybe wrong?) conflict output with ort","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-12-02T23:58:35Z","receivedAt":"2021-12-02T23:58:52Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Thu, Dec 2, 2021 at 11:08 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:\n>\n> Hi all,\n>\n> After upgrading to git 2.34.1, I tried to rebase [0] onto [1], but\n> encountered \"strange\" conflict results.\n>\n> git rebase -s recursive main produces [[RECURSIVE]]. It is roughly what\n> I expected to be output. If I take all the changes from the upper\n> section of the conflict, my changes will be effectively undone. If I\n> take all the changes from the lower section, then the upstream changes\n> will be undone.\n\nIn general, this does not work.  The only time it can work is if every\nregion of the code considered by the three-way content merge ended up\nwith conflicts.  (If any of those regions had automatically resolvable\nchanges, then after taking just the upper section(s) or just the lower\nsection(s) of each conflict would still result in a file that is a\nmixture of changes from both sides due to the automatically resolvable\nchunks that the merge already handled.)\n\n> On the other hand, running git rebase -s ort main produces [[ORT]]. I am\n> unsure if it is wrong, strictly speaking, but it is certainly unexpected\n> and difficult for me to resolve. Selecting the upper section of the\n> conflict does erase my changes, as before, but selecting the lower\n> section results in syntactically incorrect code (foreach is ended by\n> endif). The diff3 output makes even less sense to me.\n\nThe output from using ort is identical to that obtained by\n\n   git rebase -s recursive -Xdiff-algorithm=histogram ...\n\non your testcase; i.e. this is due to a difference between the\nhistogram and myers diff algorithms.\n(recursive defaults to using myers diff; ort uses histogram diff.)\n\n> A script is attached to assist in reproducing my results. Running it\n> initializes the repository to the desired state. Then, run \"git rebase\n> -s strategy master\" to produce the conflict.\n\nThanks.  I'll note for others that there's a missing 'git add\nmeson.build' (without which the script errors out), and folks should\nprobably run this from an empty directory that they can nuke later.\n\n\nLet's use a simpler example for demonstration purposes with some made\nup pseudocode.  I'll label each line so I can refer to it ('B' for\nbase, plus a line number), but the lines are everything after the\nlabel:\n\nB01 if condition1:\nB02   if condition2:\nB03     do_stuff2()\nB04   endif\nB05   for var in range:\nB06     if condition3:\nB07       do_stuff3()\nB08     endif\nB09   endfor\nB10 endif\n\nAnd let's say that locally, you modified line 7 to do something more\ncomplex, so that the local version looks like this: (prefixing the\nline numbers with 'L' for local)\n\nL01 if condition1:\nL02   if condition2:\nL03     do_stuff2()\nL04   endif\nL05   for var in range:\nL06     if condition3:\nL07       more_detailed_stuff3()\nL08     endif\nL09   endfor\nL10 endif\n\nFurther, let's say that upstream either determined that condition1 was\nalways true, or just that they wanted to run all the code\nunconditionally so they removed the outer if and un-indented\neverything.  So they have (prefixing the line numbers with 'U' for\nupstream):\n\nU01 if condition2:\nU02   do_stuff2()\nU03 endif\nU04 for var in range:\nU05   if condition3:\nU06     do_stuff3()\nU07   endif\nU08 endfor\n\n\nThere's multiple equally valid ways to attempt to merge this.  One\ncould be just considering the entire region to conflict, so you'd end\nup with a conflict region that looks like this:\n\n    <<<<<<\nL01 if condition1:\nL02   if condition2:\nL03     do_stuff2()\nL04   endif\nL05   for var in range:\nL06     if condition 3:\nL07       do_stuff3()\nL08     endif\nL09   endfor\nL10 endif\n    ||||||\nB01 if condition1:\nB02   if condition2:\nB03     do_stuff2()\nB04   endif\nB05   for var in range:\nB06     if condition3:\nB07       do_stuff3()\nB08     endif\nB09   endfor\nB10 endif\n    ======\nU01 if condition2:\nU02   do_stuff2()\nU03 endif\nU04 for var in range:\nU05   if condition3:\nU06     do_stuff3()\nU07   endif\nU08 endfor\n    >>>>>>\n\nOf course, you can leave out the middle region if not doing diff3.\n\nAlternatively, if you look closely, there is exactly one line that\nmatches in all three versions of the code: B04 == L04 == U07 (if you\nthink there are other lines that match, you're not paying enough\nattention to leading whitespace).  That one matching line could be\nused to break us into two conflict regions, which we'll take as a\nfirst step towards simplifying this:\n\n    <<<<<<\nL01 if condition1:\nL02   if condition2:\nL03     do_stuff2()\n    ||||||\nB01 if condition1:\nB02   if condition2:\nB03     do_stuff2()\n    ======\nU01 if condition2:\nU02   do_stuff2()\nU03 endif\nU04 for var in range:\nU05   if condition3:\nU06     do_stuff3()\n    >>>>>>\nB04   endif\n    <<<<<<\nL05   for var in range:\nL06     if condition 3:\nL07       more_detailed_stuff3()\nL08     endif\nL09   endfor\nL10 endif\n    ||||||\nB05   for var in range:\nB06     if condition3:\nB07       do_stuff3()\nB08     endif\nB09   endfor\nB10 endif\n    ======\nU08 endfor\n    >>>>>>\n\nNow, if you look at the first conflict region, the local side matches\nthe base side exactly, so it can be trivially merged -- we should just\ntake the upstream side.  Doing that gives us the following:\n\nU01 if condition2:\nU02   do_stuff2()\nU03 endif\nU04 for var in range:\nU05   if condition3:\nU06     do_stuff3()\nB04   endif\n    <<<<<<\nL05   for var in range:\nL06     if condition 3:\nL07       more_detailed_stuff3()\nL08     endif\nL09   endfor\nL10 endif\n    ||||||\nB05   for var in range:\nB06     if condition3:\nB07       do_stuff3()\nB08     endif\nB09   endfor\nB10 endif\n    ======\nU08 endfor\n    >>>>>>\n\nOr, if you don't use the diff3 format, then we can leave out the\nmiddle section of the remaining conflict and get just:\n\nU01 if condition2:\nU02   do_stuff2()\nU03 endif\nU04 for var in range:\nU05   if condition3:\nU06     do_stuff3()\nB04   endif\n    <<<<<<\nL05   for var in range:\nL06     if condition 3:\nL07       more_detailed_stuff3()\nL08     endif\nL09   endfor\nL10 endif\n    ======\nU08 endfor\n    >>>>>>\n\nNow, if you try to use just the \"left\" side of the remaining conflict,\nyou get code that doesn't even compile because it's mixing upstream\nand local code (and in particular ends U04's \"for\" with L10's \"endif\",\nsimilar to the example you gave).  The lines U04-B04 roughly\ncorrespond to L05-L08 (though U06 needs updating based on L07), and\nyour confusion is probably that both are included in the result.  But\nthe above is why.\n\nDoes that help explain things?\n"},{"id":"442925","messageId":"1638491699.9sjos0rkh4.none@localhost","threadId":"57018","inReplyTo":"CABPp-BEyy97x-9od+9E9DxCDHHq-m8hyZJWqxFutkRy3G7Hu9g@mail.gmail.com","subject":"Re: Confusing (maybe wrong?) conflict output with ort","fromName":"Alex Xu (Hello71)","fromEmail":"alex_y_xu@yahoo.ca","sentAt":"2021-12-03T00:40:10Z","receivedAt":"2021-12-03T00:40:18Z","isPatch":false,"sender":{"key":"alex_y_xu@yahoo.ca","avatar":null},"body":"Excerpts from Elijah Newren's message of December 2, 2021 6:58 pm:\n> Hi,\n> \n> On Thu, Dec 2, 2021 at 11:08 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:\n>>\n>> Hi all,\n>>\n>> After upgrading to git 2.34.1, I tried to rebase [0] onto [1], but\n>> encountered \"strange\" conflict results.\n>>\n>> git rebase -s recursive main produces [[RECURSIVE]]. It is roughly what\n>> I expected to be output. If I take all the changes from the upper\n>> section of the conflict, my changes will be effectively undone. If I\n>> take all the changes from the lower section, then the upstream changes\n>> will be undone.\n> \n> In general, this does not work.  The only time it can work is if every\n> region of the code considered by the three-way content merge ended up\n> with conflicts.  (If any of those regions had automatically resolvable\n> changes, then after taking just the upper section(s) or just the lower\n> section(s) of each conflict would still result in a file that is a\n> mixture of changes from both sides due to the automatically resolvable\n> chunks that the merge already handled.)\n> \n>> On the other hand, running git rebase -s ort main produces [[ORT]]. I am\n>> unsure if it is wrong, strictly speaking, but it is certainly unexpected\n>> and difficult for me to resolve. Selecting the upper section of the\n>> conflict does erase my changes, as before, but selecting the lower\n>> section results in syntactically incorrect code (foreach is ended by\n>> endif). The diff3 output makes even less sense to me.\n> \n> The output from using ort is identical to that obtained by\n> \n>    git rebase -s recursive -Xdiff-algorithm=histogram ...\n> \n> on your testcase; i.e. this is due to a difference between the\n> histogram and myers diff algorithms.\n> (recursive defaults to using myers diff; ort uses histogram diff.)\n\n[ ... ]\n\n> Does that help explain things?\n\nHm, I did try both default and patience and it didn't make a difference. \ngit rebase -s recursive -Xdiff-algorithm=histogram master does produce \nthe same result as ort though. I probably should have tried that first. \nThanks for the explanation though, that's very helpful!\n\nCheers,\nAlex.\n"}]}