Re: [PATCH 08/12] merge-ort: provide a merge_get_conflicted_files() helper function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 7, 2022 at 12:38 AM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> Hi Dscho,
>
> On Mon, Jun 6, 2022 at 2:37 PM Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> >
> > Hi Elijah,
> >
> > [after this update, I will shut up until you chime in, promise!]
> >
> > On Mon, 6 Jun 2022, Johannes Schindelin wrote:
> >
> > > On Sun, 5 Jun 2022, Johannes Schindelin wrote:
> > >
> > > > On Sat, 4 Jun 2022, Johannes Schindelin wrote:
> > > >
> > > > > It would be great if you could have a quick look over the commits I
> > > > > added on top of your branch, to see whether things make more or less
> > > > > sense to you. But if you're too busy elsewhere, I am one of the best
> > > > > persons to understand that, too.
> > > >
> > > > Hopefully I will get this into a reviewable shape before you get to
> > > > looking at it, so that your time is spent more wisely than what I
> > > > asked for ;-)
> > >
> > > I hope to find some time to work on this more tomorrow; If not, I will
> > > get back to the project on Wednesday and push it further.
> >
> > I did get a chance, and polished the patch series a bit. I also rebased it
> > onto the current tip of Git's main branch, mainly to address some merge
> > conflicts preemptively. The result is pushed up to
> > https://github.com/dscho/git/tree/js/in-core-merge-tree. This is the
> > range-diff relative to `newren/wip-for-in-core-merge-tree`:
>
> This is so awesome; thanks for working on this.  Sorry I haven't had
> time to review yet, but I'm hoping to be able to near the end of this
> week.  I'm excited to see how it looks.  :-)
>
> > -- snip --
> >  1:  cccb3888070 <  -:  ----------- tmp-objdir: new API for creating temporary writable databases
> >  2:  4e44121c2d7 <  -:  ----------- tmp-objdir: disable ref updates when replacing the primary odb
> >  3:  0b94724311d <  -:  ----------- show, log: provide a --remerge-diff capability
> >  4:  f06de6c1b2f <  -:  ----------- log: clean unneeded objects during `log --remerge-diff`
> >  5:  8d6c3d48f0e <  -:  ----------- ll-merge: make callers responsible for showing warnings
> >  6:  de8e8f88fa4 <  -:  ----------- merge-ort: capture and print ll-merge warnings in our preferred fashion
> >  7:  6b535a4d55a <  -:  ----------- merge-ort: mark a few more conflict messages as omittable
> >  8:  e2441608c63 <  -:  ----------- merge-ort: format messages slightly different for use in headers
> >  9:  62734beb693 <  -:  ----------- diff: add ability to insert additional headers for paths
> > 10:  17eccf7e0d6 <  -:  ----------- show, log: include conflict/warning messages in --remerge-diff headers
> > 11:  b3e7656cfc6 <  -:  ----------- merge-ort: mark conflict/warning messages from inner merges as omittable
> > 12:  ea5df61cf35 <  -:  ----------- diff-merges: avoid history simplifications when diffing merges
> > 13:  4a7cd5542bb =  1:  8fb51817ed4 merge-tree: rename merge_trees() to trivial_merge_trees()
> > 14:  4780ff6784d =  2:  8e0a79fa1ad merge-tree: move logic for existing merge into new function
> > 15:  60253745f5c =  3:  baf0950bcb6 merge-tree: add option parsing and initial shell for real merge function
> > 16:  f8266d39c1b =  4:  697470e50ae merge-tree: implement real merges
> > 17:  6629af14919 =  5:  069af1ecc30 merge-ort: split out a separate display_update_messages() function
> > 18:  17b57efb714 =  6:  53c92a5d8d9 merge-tree: support including merge messages in output
> > 19:  4c8f42372dd =  7:  67a728d35f0 merge-ort: provide a merge_get_conflicted_files() helper function
> > 25:  8fe5be07cd0 !  8:  6419487e26b merge-ort: remove command-line-centric submodule message from merge-ort
> >     @@ merge-ort.c: static int merge_submodule(struct merge_options *opt,
> >                 strbuf_release(&sb);
> >                 break;
> >         default:
> >     +
> >     + ## t/t6437-submodule-merge.sh ##
> >     +@@ t/t6437-submodule-merge.sh: test_expect_success 'merging should conflict for non fast-forward' '
> >     +   (cd merge-search &&
> >     +    git checkout -b test-nonforward b &&
> >     +    (cd sub &&
> >     +-    git rev-parse sub-d > ../expect) &&
> >     ++    git rev-parse --short sub-d > ../expect) &&
> >     +     if test "$GIT_TEST_MERGE_ALGORITHM" = ort
> >     +     then
> >     +           test_must_fail git merge c >actual
> > 20:  7b1ee417f3d !  9:  c92b81e7366 merge-tree: provide a list of which files have conflicts
> >     @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
> >      +          string_list_clear(&conflicted_files, 1);
> >      +  }
> >         if (o->show_messages) {
> >     -           printf("\n");
> >     +-          printf("\n");
> >     ++          putchar(line_termination);
> >                 merge_display_update_messages(&opt, &result);
> >     +   }
> >     +   merge_finalize(&opt, &result);
> >      @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> >
> >         /* Do the relevant type of merge */
> > 21:  f1231a8fbc8 = 10:  d7360f58f16 merge-tree: provide easy access to `ls-files -u` style info
> >  -:  ----------- > 11:  b53ef9c2116 merge-ort: store messages in a list, not in a single strbuf
> >  -:  ----------- > 12:  b16d570d248 merge-ort: make `path_messages` a strmap to a string_list
> >  -:  ----------- > 13:  b575a6b5f8a merge-ort: store more specific conflict information
> >  -:  ----------- > 14:  4f245cc28ae merge-ort: optionally produce machine-readable output
> > 22:  22297e6ce75 ! 15:  6a369f837be merge-tree: allow `ls-files -u` style info to be NUL terminated
> >     @@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
> >         if (!result.clean) {
> >                 struct string_list conflicted_files = STRING_LIST_INIT_NODUP;
> >                 const char *last = NULL;
> >     -@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
> >     -           string_list_clear(&conflicted_files, 1);
> >     -   }
> >     -   if (o->show_messages) {
> >     --          printf("\n");
> >     -+          putchar(line_termination);
> >     -           merge_display_update_messages(&opt, &result);
> >     -   }
> >     -   merge_finalize(&opt, &result);
> >      @@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)
> >                             N_("do a trivial merge only"), MODE_TRIVIAL),
> >                 OPT_BOOL(0, "messages", &o.show_messages,
> >     @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Check conflicted oids and
> >      +
> >      +  test_expect_code 1 git merge-tree --write-tree -z tweak1 side2 >out &&
> >      +  anonymize_hash out >actual &&
> >     ++  printf "\\n" >>actual &&
> >      +
> >      +  # Expected results:
> >      +  #   "greeting" should merge with conflicts
> >     @@ t/t4301-merge-tree-write-tree.sh: test_expect_success 'Check conflicted oids and
> >      +
> >      +  EOF
> >      +
> >     -+  cat <<-EOF >>expect &&
> >     -+  Auto-merging greeting
> >     -+  CONFLICT (content): Merge conflict in greeting
> >     -+  CONFLICT (file/directory): directory in the way of whatever from tweak1; moving it to whatever~tweak1 instead.
> >     -+  CONFLICT (modify/delete): whatever~tweak1 deleted in side2 and modified in tweak1.  Version tweak1 of whatever~tweak1 left in tree.
> >     -+  Auto-merging Αυτά μου φαίνονται κινέζικα
> >     -+  CONFLICT (content): Merge conflict in Αυτά μου φαίνονται κινέζικα
> >     ++  q_to_nul <<-EOF >>expect &&
> >     ++  1QgreetingQAuto-mergingQAuto-merging greeting
> >     ++  Q1QgreetingQCONFLICT (contents)QCONFLICT (content): Merge conflict in greeting
> >     ++  Q2Qwhatever~tweak1QwhateverQCONFLICT (file/directory)QCONFLICT (file/directory): directory in the way of whatever from tweak1; moving it to whatever~tweak1 instead.
> >     ++  Q1Qwhatever~tweak1QCONFLICT (modify/delete)QCONFLICT (modify/delete): whatever~tweak1 deleted in side2 and modified in tweak1.  Version tweak1 of whatever~tweak1 left in tree.
> >     ++  Q1QΑυτά μου φαίνονται κινέζικαQAuto-mergingQAuto-merging Αυτά μου φαίνονται κινέζικα
> >     ++  Q1QΑυτά μου φαίνονται κινέζικαQCONFLICT (contents)QCONFLICT (content): Merge conflict in Αυτά μου φαίνονται κινέζικα
> >     ++  Q
> >      +  EOF
> >      +
> >      +  test_cmp expect actual
> > 23:  db73c6dd823 = 16:  47146dd59dd merge-tree: add a --allow-unrelated-histories flag
> > 24:  d58a7c7a9f6 ! 17:  3ce28f6fd97 git-merge-tree.txt: add a section on potentional usage mistakes
> >     @@ Documentation/git-merge-tree.txt: with linkgit:git-merge[1]:
> >      +<<IM,Informational messages>> section has the necessary info, though it
> >      +is not designed to be machine parseable.
> >      +
> >     ++Do NOT assume that each paths from <<CFI,Conflicted file info>>, and
> >     ++the logical conflicts in the <<IM,Informational messages>> have a
> >     ++one-to-one mapping, nor that there is a one-to-many mapping, nor a
> >     ++many-to-one mapping.  Many-to-many mappings exist, meaning that each
> >     ++path can have many logical conflict types in a single merge, and each
> >     ++logical conflict type can affect many paths.
> >     ++
> >      +Do NOT assume all filenames listed in the <<IM,Informational messages>>
> >      +section had conflicts.  Messages can be included for files that have no
> >      +conflicts, such as "Auto-merging <file>".
> > 26:  78e1243eca1 <  -:  ----------- WIP
> > -- snap --
> >
> > I am pretty happy with the current state of the patches, and hope that we
> > can push this patch series over the finish line.
> >
> > If you can think of anything I can do to help with this, please do let me
> > know, I am _very_ interested in getting this done, and finally am in a
> > position to help.
>
> Very much appreciated.  Looks like you're now blocking on my review,
> so I'll try to make some time by end of week to look over things.

Sorry for the delay.  However, I have looked over the changes in
detail.  Well done!  I very much like this version.  Thanks for taking
the time to break up my WIP patches, fix my bugs, address all the
FIXMEs, and then going and adding some nice cleanups and improvements
on top!  I'll add my Signed-off-by on a couple patches and have
gitgitgadget submit this round.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux