All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Rast <tr@thomasrast.ch>
To: git@vger.kernel.org
Subject: [PATCH 0/9] remerge diff proof of concept/RFC
Date: Tue,  4 Feb 2014 23:17:29 +0100	[thread overview]
Message-ID: <cover.1391549294.git.tr@thomasrast.ch> (raw)

Hi,

This may look intimidating, but it's actually 3.5 separate things:

  merge-recursive: remove dead conditional in update_stages()
  merge-recursive: internal flag to avoid touching the worktree
  merge-recursive: -Xindex-only to leave worktree unchanged

These are unchanged from tr/merge-recursive-index-only.  I'm just
resending them here because it's been a while, the rest depends on it,
and it makes it more obvious how patch 8 fits in.

  pretty: refactor add_merge_info() into parts
  log: add a merge base inspection option

This seemed a good idea when I wrote it, but the more I think about
it, the less useful it appears to me.  I left it here as a
demonstration that computing the merge bases as we go is feasible.

  combine-diff: do not pass revs->dense_combined_merges redundantly
  Fold all merge diff variants into an enum

Some initial refactoring.

  merge-recursive: allow storing conflict hunks in index
  log --remerge-diff: show what the conflict resolution changed

The real meat.

I'll only describe the last patches here.

Context: last summer (yes, it's been a while) I investigated some
angles of looking at merges that make it easier to investigate
mismerges.  The problem space is: it is exceedingly hard to find
instances of

  git merge -s ours foo
  git merge -Xours foo
  manually resolve some hunks in favor of one side

after some time has passed.  The only ways I found with existing
machinery involve too much manual inspection.

At the time I made some scripts that look at the problem in various
ways:

  https://github.com/trast/evilmergediff

This series explores another angle, which I call "remerge diff".  It
works by re-doing the merge in core, using features from patches 1-3
and 7.  Likely that will result in conflicts, which are formatted in
the usual <<<<<<< way.  Then it diffs this "remerge" against the
merge's tree that is recorded in history.

This tends to give nice results in practice; I pasted a particularly
pretty example at the bottom.


However, this is RFC because nontrivial merges can easily lead to
states that I cannot handle with the current -- pretty neat and
simple, I think -- implementation.

For example, a delete/modify conflict does not result in a nice
conflict-hunk formatted file, it just leaves 2 (out of 3) stages in
the index.  I can then not write a tree from that.

This could be fixed by adding a pass that turns a missing stage 2 or 3
into an empty file at that stage, so that the resulting file looks
like

  <<<<<<<
  =======
  ... stage 3 content goes here ...
  >>>>>>>

(analogously for stage 3 missing).

But that still doesn't handle at least directory/file conflicts.

So I would welcome comments, and/or better ideas, on the following
proposed resolution:

* Implement what I described last, to take care of delete/modify
  conflicts.

* Punt on more complex conflicts, by removing those files from the
  index, and emitting a warning about those files instead.

Note that even without delete/modify handling, this covers the vast
majority of merges: among some 8090 merges I sampled from git.git,
there are only 13 that trigger the "BUG: ..." message, and 34
octopuses.


Example run:

  $ git show --remerge-diff a39b15b4f6a3f08b67b17d968935d177821e680f
  [...]
  diff --git a/builtin/ls-files.c b/builtin/ls-files.c
  index 0f4473e..175e6e3 100644
  --- a/builtin/ls-files.c
  +++ b/builtin/ls-files.c
  @@ -490,16 +490,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
   		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
   			N_("show unmerged files in the output")),
   		OPT_BOOLEAN(0, "resolve-undo", &show_resolve_undo,
  -<<<<<<< f12e49ae877ad0644b9b9939b7cb742da98691d2
   			    N_("show resolve-undo information")),
  -		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], N_("pattern"),
  +		{ OPTION_CALLBACK, 'x', "exclude", &exclude_list, N_("pattern"),
   			N_("skip files matching pattern"),
  -=======
  -			    "show resolve-undo information"),
  -		{ OPTION_CALLBACK, 'x', "exclude",
  -			&exclude_list, "pattern",
  -			"skip files matching pattern",
  ->>>>>>> 72aeb18772deeb386da7dd8997b969877bd29e41
   			0, option_parse_exclude },
   		{ OPTION_CALLBACK, 'X', "exclude-from", &dir, N_("file"),
   			N_("exclude patterns are read from <file>"),
  diff --git a/dir.c b/dir.c
  index 8c58ce4..cf1e6b0 100644
  --- a/dir.c
  +++ b/dir.c
  @@ -1674,14 +1674,14 @@ void free_pathspec(struct pathspec *pathspec)
   	pathspec->items = NULL;
   }
   
  -<<<<<<< f12e49ae877ad0644b9b9939b7cb742da98691d2
   int limit_pathspec_to_literal(void)
   {
   	static int flag = -1;
   	if (flag < 0)
   		flag = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
   	return flag;
  -=======
  +}
  +
   /*
    * Frees memory within dir which was allocated for exclude lists and
    * the exclude_stack.  Does not free dir itself.
  @@ -1710,5 +1710,4 @@ void clear_directory(struct dir_struct *dir)
   		free(stk);
   		stk = prev;
   	}
  ->>>>>>> 72aeb18772deeb386da7dd8997b969877bd29e41
   }




 Documentation/merge-strategies.txt |   9 +++
 Documentation/rev-list-options.txt |  14 ++++
 builtin/diff-files.c               |   5 +-
 builtin/diff-tree.c                |   2 +-
 builtin/diff.c                     |  12 +--
 builtin/fmt-merge-msg.c            |   2 +-
 builtin/log.c                      |   9 +--
 builtin/merge.c                    |   1 -
 combine-diff.c                     |  13 ++--
 commit.h                           |   1 +
 diff-lib.c                         |  13 ++--
 diff.h                             |   6 +-
 log-tree.c                         |  69 +++++++++++++++++-
 merge-recursive.c                  |  52 +++++++------
 merge-recursive.h                  |   3 +
 pretty.c                           |  41 +++++++----
 revision.c                         |  17 ++---
 revision.h                         |  26 ++++++-
 submodule.c                        |   3 +-
 t/t3030-merge-recursive.sh         |  33 +++++++++
 t/t4202-log.sh                     |  34 +++++++++
 t/t4213-log-remerge-diff.sh        | 145 +++++++++++++++++++++++++++++++++++++
 22 files changed, 426 insertions(+), 84 deletions(-)
 create mode 100755 t/t4213-log-remerge-diff.sh

-- 
1.9.rc2.232.gdd31389

             reply	other threads:[~2014-02-04 22:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-04 22:17 Thomas Rast [this message]
2014-02-04 22:17 ` [PATCH 1/9] merge-recursive: remove dead conditional in update_stages() Thomas Rast
2014-02-04 22:17 ` [PATCH 2/9] merge-recursive: internal flag to avoid touching the worktree Thomas Rast
2014-02-04 22:17 ` [PATCH 3/9] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
2014-02-04 22:17 ` [POC PATCH 4/9] pretty: refactor add_merge_info() into parts Thomas Rast
2014-02-04 22:17 ` [POC PATCH 5/9] log: add a merge base inspection option Thomas Rast
2014-02-04 22:17 ` [PATCH 6/9] combine-diff: do not pass revs->dense_combined_merges redundantly Thomas Rast
2014-02-04 22:17 ` [PATCH 7/9] Fold all merge diff variants into an enum Thomas Rast
2014-02-04 22:17 ` [PATCH 8/9] merge-recursive: allow storing conflict hunks in index Thomas Rast
2014-02-04 22:17 ` [RFC PATCH 9/9] log --remerge-diff: show what the conflict resolution changed Thomas Rast
2014-02-04 23:04 ` [PATCH 0/9] remerge diff proof of concept/RFC Junio C Hamano

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1391549294.git.tr@thomasrast.ch \
    --to=tr@thomasrast.ch \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.