From: Heba Waly <heba.waly@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: Heba Waly via GitGitGadget <gitgitgadget@gmail.com>,
Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH 04/10] merge: move doc to ll-merge.h
Date: Fri, 1 Nov 2019 08:35:02 +1300 [thread overview]
Message-ID: <CACg5j25WCHf8_pMVPeakYx-sdSG+-naPPchqUCesfaNQHVCCNQ@mail.gmail.com> (raw)
In-Reply-To: <CABPp-BEYeCwTKXLTdaORrBGAFYb0X13rMMiQXwXv=UDSBKHnYQ@mail.gmail.com>
On Thu, Oct 31, 2019 at 11:09 AM Elijah Newren <newren@gmail.com> wrote:
>
> Hi Heba,
> Thanks for the contribution. I know you weren't the original author
> of most this stuff, but I was curious if it really all belonged in
> ll-merge.c and then noticed other issues...
Hi Elijah, thanks a lot for the feedback.
This is my first interaction with the merge API, so I wasn't
completely sure where the intro of the doc needed to go, looks like
ll-merge.h is not the perfect place, is there a top level merge file
where this generic intro would be more suitable and helpful?
> On Tue, Oct 29, 2019 at 11:49 AM Heba Waly via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> [...]
> > diff --git a/ll-merge.h b/ll-merge.h
> > index e78973dd55..ec3617c627 100644
> > --- a/ll-merge.h
> > +++ b/ll-merge.h
> > @@ -7,16 +7,94 @@
> >
> > #include "xdiff/xdiff.h"
> >
> > +/**
> > + * The merge API helps a program to reconcile two competing sets of
>
> Is this talking about xdiff/xmerge.c, ll_merge.c, merge-recursive.c,
> or builtin/merge.c? Those are all different level of "merge API" and
> it's not clear. Perhaps "The Low Level Merge API" or something like
> that since you are moving it into ll-merge.h?
Yea, that's why I'm thinking maybe move this paragraph to another
top-level file?
> > + * improvements to some files (e.g., unregistered changes from the work
> > + * tree versus changes involved in switching to a new branch), reporting
> > + * conflicts if found.
>
> Seems weird to bring up checkout -m without mentioning in by name
> given that it isn't the default checkout behavior. Would seem more
> natural to mention a merge or rebase case.
I agree with you, can change the example if we agreed on keeping this paragraph.
> > + * The library called through this API is
> > + * responsible for a few things.
> > + *
> > + * - determining which trees to merge (recursive ancestor consolidation);
>
> Um, that's done at the merge-recursive.c level, not at the ll-merge.c
> level. I'm confused why it'd be mentioned here.
you're right.
> > + * - lining up corresponding files in the trees to be merged (rename
> > + * detection, subtree shifting), reporting edge cases like add/add
> > + * and rename/rename conflicts to the user;
>
> All of that is also clearly stuff for merge-recursive.c; I'm not sure
> why it'd be mentioned in the Low-Level merge file.
got it.
> > + * - performing a three-way merge of corresponding files, taking
> > + * path-specific merge drivers (specified in `.gitattributes`)
> > + * into account.
>
> This, however, is ll-merge.c stuff.
So, move the whole paragraph to another file?
because, I think the paragraph is helpful as a whole, and I don't see
the value in dividing it between merge-recursive and ll-merge.
What do you think?
> > + *
> > + * Calling sequence:
> > + * ----------------
> > + *
> > + * - Prepare a `struct ll_merge_options` to record options.
> > + * If you have no special requests, skip this and pass `NULL`
> > + * as the `opts` parameter to use the default options.
> > + *
> > + * - Allocate an mmbuffer_t variable for the result.
> > + *
> > + * - Allocate and fill variables with the file's original content
> > + * and two modified versions (using `read_mmfile`, for example).
> > + *
> > + * - Call `ll_merge()`.
> > + *
> > + * - Read the merged content from `result_buf.ptr` and `result_buf.size`.
> > + *
> > + * - Release buffers when finished. A simple
> > + * `free(ancestor.ptr); free(ours.ptr); free(theirs.ptr);
> > + * free(result_buf.ptr);` will do.
> > + *
> > + * If the modifications do not merge cleanly, `ll_merge` will return a
> > + * nonzero value and `result_buf` will generally include a description of
> > + * the conflict bracketed by markers such as the traditional `<<<<<<<`
> > + * and `>>>>>>>`.
> > + *
> > + * The `ancestor_label`, `our_label`, and `their_label` parameters are
> > + * used to label the different sides of a conflict if the merge driver
> > + * supports this.
> > + */
>
> This part looks good.
>
> > +/**
> > + * This describes the set of options the calling program wants to affect
> > + * the operation of a low-level (single file) merge.
> > + */
> > struct ll_merge_options {
> > +
> > + /**
> > + * Behave as though this were part of a merge between common ancestors in
> > + * a recursive merge. If a helper program is specified by the
> > + * `[merge "<driver>"] recursive` configuration, it will be used.
> > + */
>
> This kind of leaves out the why. Maybe add "(merges of binary files
> may need to be handled differently in such cases, for example)" to the
> end of the first sentence?
Yes, ok.
> > unsigned virtual_ancestor : 1;
> > - unsigned variant : 2; /* favor ours, favor theirs, or union merge */
> > +
> > + /**
> > + * Resolve local conflicts automatically in favor of one side or the other
> > + * (as in 'git merge-file' `--ours`/`--theirs`/`--union`). Can be `0`,
> > + * `XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`,
> > + * or `XDL_MERGE_FAVOR_UNION`.
> > + */
> > + unsigned variant : 2;
> > +
> > + /**
> > + * Resmudge and clean the "base", "theirs" and "ours" files before merging.
> > + * Use this when the merge is likely to have overlapped with a change in
> > + * smudge/clean or end-of-line normalization rules.
> > + */
> > unsigned renormalize : 1;
>
> All looks good.
>
> > +
> > unsigned extra_marker_size;
>
> No documentation for this one? Perhaps:
>
> /*
> * Increase the length of conflict markers so that nested conflicts
> * can be differentiated.
> */
sure.
> > long xdl_opts;
>
> Perhaps document this one with:
>
> /* Extra xpparam_t flags as defined in xdiff/xdiff.h. */
great. thanks!
>
>
> > };
> >
> > +/**
> > + * Perform a three-way single-file merge in core. This is a thin wrapper
> > + * around `xdl_merge` that takes the path and any merge backend specified in
> > + * `.gitattributes` or `.git/info/attributes` into account.
> > + * Returns 0 for a clean merge.
> > + */
> > int ll_merge(mmbuffer_t *result_buf,
> > const char *path,
> > mmfile_t *ancestor, const char *ancestor_label,
next prev parent reply other threads:[~2019-10-31 19:35 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 10:00 [PATCH 00/10] [Outreachy] Move doc to header files Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 01/10] diff: move doc to diff.h and diffcore.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 02/10] dir: move doc to dir.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 03/10] graph: move doc to graph.h and graph.c Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 04/10] merge: move doc to ll-merge.h Heba Waly via GitGitGadget
2019-10-30 22:09 ` Elijah Newren
2019-10-31 19:35 ` Heba Waly [this message]
2019-11-02 4:28 ` Junio C Hamano
2019-10-29 10:00 ` [PATCH 05/10] sha1-array: move doc to sha1-array.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 06/10] remote: move doc to remote.h and refspec.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 07/10] refs: move doc to refs.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 08/10] attr: move doc to attr.h Heba Waly via GitGitGadget
2019-10-29 10:00 ` [PATCH 09/10] revision: move doc to revision.h Heba Waly via GitGitGadget
2019-10-29 23:57 ` Emily Shaffer
2019-10-29 10:00 ` [PATCH 10/10] pathspec: move doc to pathspec.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 00/20] [Outreachy] Move doc to header files Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 01/20] diff: move doc to diff.h and diffcore.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 02/20] dir: move doc to dir.h Heba Waly via GitGitGadget
2019-11-07 1:16 ` Emily Shaffer
2019-11-11 0:20 ` Heba Waly
2019-11-06 9:59 ` [PATCH v2 03/20] graph: move doc to graph.h and graph.c Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 04/20] merge: move doc to ll-merge.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 05/20] sha1-array: move doc to sha1-array.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 06/20] remote: move doc to remote.h and refspec.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 07/20] refs: move doc to refs.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 08/20] attr: move doc to attr.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 09/20] revision: move doc to revision.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 10/20] pathspec: move doc to pathspec.h Heba Waly via GitGitGadget
2019-11-07 1:26 ` Emily Shaffer
2019-11-10 1:40 ` Heba Waly
2019-11-06 9:59 ` [PATCH v2 11/20] sigchain: move doc to sigchain.h Heba Waly via GitGitGadget
2019-11-06 22:03 ` Emily Shaffer
2019-11-11 1:04 ` Heba Waly
2019-11-06 9:59 ` [PATCH v2 12/20] cache: move doc to cache.h Heba Waly via GitGitGadget
2019-11-06 22:04 ` Emily Shaffer
2019-11-06 9:59 ` [PATCH v2 13/20] argv-array: move doc to argv-array.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 14/20] credential: move doc to credential.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 15/20] parse-options: move doc to parse-options.h Heba Waly via GitGitGadget
2019-11-11 2:45 ` Junio C Hamano
2019-11-11 21:38 ` Heba Waly
2019-11-12 5:57 ` Junio C Hamano
2019-11-15 9:55 ` Heba Waly
2019-11-15 11:37 ` Junio C Hamano
2019-11-15 23:28 ` Emily Shaffer
2019-11-17 11:34 ` Heba Waly
2019-11-06 9:59 ` [PATCH v2 16/20] run-command: move doc to run-command.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 17/20] trace: move doc to trace.h Heba Waly via GitGitGadget
2019-11-07 1:32 ` Emily Shaffer
2019-11-06 9:59 ` [PATCH v2 18/20] tree-walk: move doc to tree-walk.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 19/20] submodule-config: move doc to submodule-config.h Heba Waly via GitGitGadget
2019-11-06 9:59 ` [PATCH v2 20/20] trace2: move doc to trace2.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 00/21] [Outreachy] Move doc to header files Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 01/21] diff: move doc to diff.h and diffcore.h Heba Waly via GitGitGadget
2019-11-12 7:20 ` Junio C Hamano
2019-11-14 12:22 ` Heba Waly
2019-11-11 21:27 ` [PATCH v3 02/21] dir: move doc to dir.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 03/21] graph: move doc to graph.h and graph.c Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 04/21] merge: move doc to ll-merge.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 05/21] sha1-array: move doc to sha1-array.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 06/21] remote: move doc to remote.h and refspec.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 07/21] refs: move doc to refs.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 08/21] attr: move doc to attr.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 09/21] revision: move doc to revision.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 10/21] pathspec: move doc to pathspec.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 11/21] sigchain: move doc to sigchain.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 12/21] cache: move doc to cache.h Heba Waly via GitGitGadget
2019-11-12 7:05 ` Junio C Hamano
2019-11-14 10:14 ` Heba Waly
2019-11-11 21:27 ` [PATCH v3 13/21] argv-array: move doc to argv-array.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 14/21] credential: move doc to credential.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 15/21] parse-options: move doc to parse-options.h Heba Waly via GitGitGadget
2019-11-11 21:27 ` [PATCH v3 16/21] run-command: move doc to run-command.h Heba Waly via GitGitGadget
2019-11-11 21:28 ` [PATCH v3 17/21] trace: move doc to trace.h Heba Waly via GitGitGadget
2019-11-11 21:28 ` [PATCH v3 18/21] tree-walk: move doc to tree-walk.h Heba Waly via GitGitGadget
2019-11-11 21:28 ` [PATCH v3 19/21] submodule-config: move doc to submodule-config.h Heba Waly via GitGitGadget
2019-11-11 21:28 ` [PATCH v3 20/21] trace2: move doc to trace2.h Heba Waly via GitGitGadget
2019-11-12 7:02 ` Junio C Hamano
2019-11-14 10:30 ` Heba Waly
2019-11-11 21:28 ` [PATCH v3 21/21] api-index: remove api doc index files Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 00/21] [Outreachy] Move doc to header files Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 01/21] diff: move doc to diff.h and diffcore.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 02/21] dir: move doc to dir.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 03/21] graph: move doc to graph.h and graph.c Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 04/21] merge: move doc to ll-merge.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 05/21] sha1-array: move doc to sha1-array.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 06/21] remote: move doc to remote.h and refspec.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 07/21] refs: move doc to refs.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 08/21] attr: move doc to attr.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 09/21] revision: move doc to revision.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 10/21] pathspec: move doc to pathspec.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 11/21] sigchain: move doc to sigchain.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 12/21] cache: move doc to cache.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 13/21] argv-array: move doc to argv-array.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 14/21] credential: move doc to credential.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 15/21] parse-options: move doc to parse-options.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 16/21] run-command: move doc to run-command.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 17/21] trace: move doc to trace.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 18/21] tree-walk: move doc to tree-walk.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 19/21] submodule-config: move doc to submodule-config.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 20/21] trace2: move doc to trace2.h Heba Waly via GitGitGadget
2019-11-15 9:53 ` [PATCH v4 21/21] api-index: remove api doc index files Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 00/21] [Outreachy] Move doc to header files Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 01/21] diff: move doc to diff.h and diffcore.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 02/21] dir: move doc to dir.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 03/21] graph: move doc to graph.h and graph.c Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 04/21] merge: move doc to ll-merge.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 05/21] sha1-array: move doc to sha1-array.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 06/21] remote: move doc to remote.h and refspec.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 07/21] refs: move doc to refs.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 08/21] attr: move doc to attr.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 09/21] revision: move doc to revision.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 10/21] pathspec: move doc to pathspec.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 11/21] sigchain: move doc to sigchain.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 12/21] cache: move doc to cache.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 13/21] argv-array: move doc to argv-array.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 14/21] credential: move doc to credential.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 15/21] parse-options: add link to doc file in parse-options.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 16/21] run-command: move doc to run-command.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 17/21] trace: move doc to trace.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 18/21] tree-walk: move doc to tree-walk.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 19/21] submodule-config: move doc to submodule-config.h Heba Waly via GitGitGadget
2019-11-17 21:04 ` [PATCH v5 20/21] trace2: move doc to trace2.h Heba Waly via GitGitGadget
2019-11-17 21:05 ` [PATCH v5 21/21] api-index: remove api doc index files Heba Waly via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACg5j25WCHf8_pMVPeakYx-sdSG+-naPPchqUCesfaNQHVCCNQ@mail.gmail.com \
--to=heba.waly@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).