git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] submodule merge: update conflict error message

From
Calvin Wan <calvinwan@google.com>
Date
Jul 18, 2022, 17:03 UTC
Message-ID
<CAFySSZD-GRwXvaK4sPJUOzU78GiJLunP7tZtF4i7nZ+f8i8WAg@mail.gmail.com>
In-Reply-To
<7roq6p2s-17rn-477p-7n60-5p10926on785@tzk.qr>
> No matter how hard I tried to stare at the code, no matter whether I
> looked at `cw/submodule-merge-messages` or `seen`, I cannot see how this
> `grep` could ever succeed when `GIT_TEST_MERGE_ALGORITHM=recursive`: only
> the `ort` code has been taught this new trick.

I definitely overlooked this removing the merge-recursive section of my code. Thanks for catching this!

On Fri, Jul 15, 2022 at 5:57 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 205 quoted lines
>
> Hi Calvin,
>
> On Tue, 12 Jul 2022, Calvin Wan wrote:
>
> > diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
> > index 178413c22f..cc1a312661 100755
> > --- a/t/t6437-submodule-merge.sh
> > +++ b/t/t6437-submodule-merge.sh
> > @@ -103,8 +103,25 @@ test_expect_success 'setup for merge search' '
> >        echo "file-c" > file-c &&
> >        git add file-c &&
> >        git commit -m "sub-c") &&
> > -     git commit -a -m "c" &&
> > +     git commit -a -m "c")
> > +'
> > +
> > +test_expect_success 'merging should conflict for non fast-forward' '
> > +     (cd merge-search &&
> > +      git checkout -b test-nonforward-a b &&
> > +       if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > +       then
> > +             test_must_fail git merge c >actual
> > +       else
> > +             test_must_fail git merge c 2> actual
> > +       fi &&
> > +      sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> > +      grep "$sub_expect" actual &&
>
> No matter how hard I tried to stare at the code, no matter whether I
> looked at `cw/submodule-merge-messages` or `seen`, I cannot see how this
> `grep` could ever succeed when `GIT_TEST_MERGE_ALGORITHM=recursive`: only
> the `ort` code has been taught this new trick.
>
> In fact, when I run this locally with
> `GIT_TEST_MERGE_ALGORITHM=recursive`, it not only fails (as shown here:
> https://github.com/gitgitgadget/git/runs/7349612861?check_suite_focus=true#step:4:1833
> and here:
> https://github.com/gitgitgadget/git/runs/7326925485?check_suite_focus=true#step:4:1820),
> it also leaves no error message in `actual`.
>
> So you probably want to move the `sub_expect` assignment and the `grep`
> into the `ort` clause of the conditional above.
>
> With this fixup, things seem to work over here:
>
> -- snip --
> diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
> index cc1a3126619..3892d0bf742 100755
> --- a/t/t6437-submodule-merge.sh
> +++ b/t/t6437-submodule-merge.sh
> @@ -111,12 +111,12 @@ test_expect_success 'merging should conflict for non fast-forward' '
>          git checkout -b test-nonforward-a b &&
>           if test "$GIT_TEST_MERGE_ALGORITHM" = ort
>           then
> -               test_must_fail git merge c >actual
> +               test_must_fail git merge c >actual &&
> +               sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> +               grep "$sub_expect" actual
>           else
>                 test_must_fail git merge c 2> actual
>           fi &&
> -        sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> -        grep "$sub_expect" actual &&
>          git reset --hard)
>  '
>
> @@ -153,13 +153,13 @@ test_expect_success 'merging should conflict for non fast-forward (resolution ex
>           git rev-parse sub-d > ../expect) &&
>           if test "$GIT_TEST_MERGE_ALGORITHM" = ort
>           then
> -               test_must_fail git merge c >actual
> +               test_must_fail git merge c >actual &&
> +               sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> +               grep "$sub_expect" actual
>           else
>                 test_must_fail git merge c 2> actual
>           fi &&
>          grep $(cat expect) actual > /dev/null &&
> -        sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> -        grep "$sub_expect" actual &&
>          git reset --hard)
>  '
>
> @@ -180,14 +180,14 @@ test_expect_success 'merging should fail for ambiguous common parent' '
>          ) &&
>          if test "$GIT_TEST_MERGE_ALGORITHM" = ort
>          then
> -               test_must_fail git merge c >actual
> +               test_must_fail git merge c >actual &&
> +               sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> +               grep "$sub_expect" actual
>          else
>                 test_must_fail git merge c 2> actual
>          fi &&
>         grep $(cat expect1) actual > /dev/null &&
>         grep $(cat expect2) actual > /dev/null &&
> -       sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> -       grep "$sub_expect" actual &&
>         git reset --hard)
>  '
>
> @@ -227,8 +227,11 @@ test_expect_success 'merging should fail for changes that are backwards' '
>
>         git checkout -b test-backward e &&
>         test_must_fail git merge f >actual &&
> -       sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" &&
> -       grep "$sub_expect" actual)
> +       if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> +       then
> +               sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" &&
> +               grep "$sub_expect" actual
> +       fi)
>  '
>
> -- snap --
>
> > +      git reset --hard)
>
> It would be better to move this before the subshell like this:
>
>         test_when_finished "git -C merge-search reset --hard" &&
>
> Ciao,
> Dscho
>
> > +'
> >
> > +test_expect_success 'finish setup for merge-search' '
> > +     (cd merge-search &&
> >       git checkout -b d a &&
> >       (cd sub &&
> >        git checkout -b sub-d sub-b &&
> > @@ -129,9 +146,9 @@ test_expect_success 'merge with one side as a fast-forward of the other' '
> >        test_cmp expect actual)
> >  '
> >
> > -test_expect_success 'merging should conflict for non fast-forward' '
> > +test_expect_success 'merging should conflict for non fast-forward (resolution exists)' '
> >       (cd merge-search &&
> > -      git checkout -b test-nonforward b &&
> > +      git checkout -b test-nonforward-b b &&
> >        (cd sub &&
> >         git rev-parse sub-d > ../expect) &&
> >         if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> > @@ -141,6 +158,8 @@ test_expect_success 'merging should conflict for non fast-forward' '
> >               test_must_fail git merge c 2> actual
> >         fi &&
> >        grep $(cat expect) actual > /dev/null &&
> > +      sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> > +      grep "$sub_expect" actual &&
> >        git reset --hard)
> >  '
> >
> > @@ -167,6 +186,8 @@ test_expect_success 'merging should fail for ambiguous common parent' '
> >        fi &&
> >       grep $(cat expect1) actual > /dev/null &&
> >       grep $(cat expect2) actual > /dev/null &&
> > +     sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
> > +     grep "$sub_expect" actual &&
> >       git reset --hard)
> >  '
> >
> > @@ -205,7 +226,9 @@ test_expect_success 'merging should fail for changes that are backwards' '
> >       git commit -a -m "f" &&
> >
> >       git checkout -b test-backward e &&
> > -     test_must_fail git merge f)
> > +     test_must_fail git merge f >actual &&
> > +     sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" &&
> > +     grep "$sub_expect" actual)
> >  '
> >
> >
> > diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
> > index 8e32f19007..f1bb29681f 100755
> > --- a/t/t7402-submodule-rebase.sh
> > +++ b/t/t7402-submodule-rebase.sh
> > @@ -104,7 +104,7 @@ test_expect_success 'rebasing submodule that should conflict' '
> >       test_tick &&
> >       git commit -m fourth &&
> >
> > -     test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 &&
> > +     test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 >actual_output &&
> >       git ls-files -s submodule >actual &&
> >       (
> >               cd submodule &&
> > @@ -112,7 +112,9 @@ test_expect_success 'rebasing submodule that should conflict' '
> >               echo "160000 $(git rev-parse HEAD^^) 2  submodule" &&
> >               echo "160000 $(git rev-parse HEAD) 3    submodule"
> >       ) >expect &&
> > -     test_cmp expect actual
> > +     test_cmp expect actual &&
> > +     sub_expect="go to submodule (submodule), and either merge commit $(git -C submodule rev-parse --short HEAD^0)" &&
> > +     grep "$sub_expect" actual_output
> >  '
> >
> >  test_done
> >
> > base-commit: ab336e8f1c8009c8b1aab8deb592148e69217085
> > --
> > 2.37.0.144.g8ac04bfd2-goog
> >
> >
> >
Previous: Elijah NewrenNext: Calvin Wan
Message 27 of 60 in “submodule merge: update conflict error message”
  1. submodule merge: update conflict error messageCalvin Wan, Jun 6, 2022
  2. Junio C HamanoJun 7, 2022
  3. Calvin WanJun 8, 2022
  4. Glen ChooJun 8, 2022
  5. Calvin WanJun 8, 2022
  6. Junio C HamanoJun 8, 2022
  7. submodule merge: update conflict error messageCalvin Wan, Jun 10, 2022
  8. Elijah NewrenJun 11, 2022
  9. Philippe BlainJun 11, 2022
  10. Elijah NewrenJun 12, 2022
  11. Calvin WanJun 13, 2022
  12. Junio C HamanoJun 12, 2022
  13. Calvin WanJun 13, 2022
  14. submodule merge: update conflict error messageCalvin Wan, Jun 29, 2022
  15. Elijah NewrenJun 30, 2022
  16. Calvin WanJun 30, 2022
  17. Elijah NewrenJul 1, 2022
  18. Glen ChooJun 30, 2022
  19. Glen ChooJun 30, 2022
  20. Calvin WanJun 30, 2022
  21. submodule merge: update conflict error messageCalvin Wan, Jul 12, 2022
  22. Junio C HamanoJul 13, 2022
  23. Elijah NewrenJul 17, 2022
  24. Johannes SchindelinJul 15, 2022
  25. Junio C HamanoJul 16, 2022
  26. Elijah NewrenJul 17, 2022
  27. Calvin WanJul 18, 2022
  28. submodule merge: update conflict error messageCalvin Wan, Jul 18, 2022
  29. Junio C HamanoJul 19, 2022
  30. Calvin WanJul 19, 2022
  31. Junio C HamanoJul 19, 2022
  32. Junio C HamanoJul 19, 2022
  33. Calvin WanJul 19, 2022
  34. Junio C HamanoJul 19, 2022
  35. Ævar Arnfjörð BjarmasonJul 25, 2022
  36. Ævar Arnfjörð BjarmasonJul 25, 2022
  37. Calvin WanJul 25, 2022
  38. Ævar Arnfjörð BjarmasonJul 25, 2022
  39. Calvin WanJul 25, 2022
  40. submodule merge: update conflict error messageCalvin Wan, Jul 26, 2022
  41. Elijah NewrenJul 27, 2022
  42. Calvin WanJul 27, 2022
  43. Elijah NewrenJul 28, 2022
  44. Calvin WanJul 28, 2022
  45. Ævar Arnfjörð BjarmasonJul 27, 2022
  46. submodule merge: update conflict error messageCalvin Wan, Jul 28, 2022
  47. Ævar Arnfjörð BjarmasonJul 28, 2022
  48. Elijah NewrenJul 29, 2022
  49. Calvin WanAug 1, 2022
  50. Ævar Arnfjörð BjarmasonAug 1, 2022
  51. Junio C HamanoAug 2, 2022
  52. Calvin WanAug 2, 2022
  53. Junio C HamanoAug 2, 2022
  54. Calvin WanAug 2, 2022
  55. Junio C HamanoAug 2, 2022
  56. submodule merge: update conflict error messageCalvin Wan, Aug 4, 2022
  57. Junio C HamanoAug 16, 2022
  58. Junio C HamanoAug 16, 2022
  59. Calvin WanAug 16, 2022
  60. Junio C HamanoAug 16, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.