Re: Guided merge with override

Previous thread: [RFC/PATCH] git-instaweb: added logo and favicon by Flavio Poletti on Sunday, June 15, 2008 - 7:46 pm. (2 messages)

Next thread: How to omit/alter the sigend-off line in a patch set? by Ian Brown on Monday, June 16, 2008 - 3:24 am. (2 messages)
From: Geoff Russell
Date: Sunday, June 15, 2008 - 9:16 pm

I have a two repositories A and B.  B is a tiny subset of the files in
A and all have been
modified.   If I do a "git pull B" into A, I get conflicts. I always
want to resolve these
by accepting the version from B. Is there a magic "override" switch to
let me do this?

Cheers,
Geoff Russell
--

From: Miklos Vajna
Date: Monday, June 16, 2008 - 2:25 am

There was a thread about this:

http://thread.gmane.org/gmane.comp.version-control.git/84047

and then you can do a git pull -s theirs B.
From: Johannes Sixt
Date: Monday, June 16, 2008 - 3:16 am

I don't think that's what Geoff needs. The 'theirs' strategy replaces the
entire tree by 'their' - B's - tree. But IIUC, only the subset of files
that are contained in B should be replaced by B's version, the rest of the
files should remain unchanged. This is quite different from 'theirs' strategy.

The solution depends on whether *all* files in B should be taken, or only
those files in B where there's a merge conflict. I don't know an easy way
to do the former, but the latter I'd do like this:

	$ git diff --name-only | xargs git checkout B --

-- Hannes

--

From: Miklos Vajna
Date: Monday, June 16, 2008 - 2:16 pm

Thanks, I missed that difference, and sorry for the wrong suggestion.
From: Sverre Rabbelier
Date: Monday, June 16, 2008 - 3:21 pm

Wouldn't something similar work but do a 'git ls-files' and filter it
on files that have a merge conflict?

-- 
Cheers,

Sverre Rabbelier
--

From: Geoff Russell
Date: Monday, June 16, 2008 - 3:45 pm

Thanks everybody,


This looks like a manageable approach and better than the scripting I was
thinking about -- ie. scan the conflict files with perl and fix them!

Cheers,


-- 
6 Fifth Ave,
St Morris, S.A. 5068
Australia
Ph: 041 8805 184 / 08 8332 5069
--

From: Junio C Hamano
Date: Tuesday, June 17, 2008 - 1:04 pm

Careful.

The above pipeline gives quite different result from "scan the conflict
files with perl and fix them".

If the result you want from the "scan and fix the conflicts" approach is
to take as much automerge as possible, and punt only on conflicting parts
by discarding your work (side note: I hear this wish often and still I
have not heard satisfactory explanation why people think that could be a
sane result, though), the above pipeline is not what you want.  It not
only discards your work in conflicting parts but also all your work from
even cleanly automerged parts of a path that has conflicts.

For example, suppose the original, our version and their version are like
these respectively:

        (orig)    (ours)     (theirs)
        1         one        uno
        2         2          2
        3         3          3
        4         4          4
        5         5          5
        6         6          6
        7         7          7
        8         eight      8

If you merge these with natural 3-way merge, you would get this:

        $ git-merge-file -p ours orig theirs
        <<<<<<< ours
        one
        =======
        uno
        >>>>>>> theirs
        2
        3
        4
        5
        6
        7
        eight

Note: git-merge-file mimicks the "merge" program from RCS suite.  The
      three file parameters are mine, old and yours (in alphabetical order
      as easy-to-remember mnemonic) and means "update mine taking the
      change that you made to old to reach yours".

That's a conflict.  The above suggested pipeline would give "their"
version.  That means the result does not have your change s/8/eight/.

It all depends on what you really mean by take "theirs", but you might
have wanted to have this instead, with the "scan and fix the conflicts"
approach you hinted in your message:

        $ git-merge-file -p --theirs ours orig theirs
        uno
        2
        3
        4
        5
        6
        7
    ...
From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 8:19 am

Hi,


Looks good, thanks!  Just to be safe, this should be accompanied by tet 
cases.  But I did not see anything wrong with the patch.

Ciao,
Dscho
--

From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 8:28 am

Hi,


Thinking about this again, there could be a problem: in case of complex 
merges, it is possible that the sides are switched around for an 
intermediate merge.  IOW you'd expect it to take "theirs", but it really 
takes "ours".

Hrm.

Ciao,
Dscho

--

From: Junio C Hamano
Date: Wednesday, June 18, 2008 - 9:31 am

Are you thinking about using this in merge-recursive?

I do not think there is any reason to use this during intermediate merges
done inside merge-recursive.  The point of recursive merge is to create a
neutral intermediate merge result, with conflicts and all.  Do this only
during the final round and you are fine (for some definition of "fine" ---
I still have not heard a convincing argument as to why it is even a good
thing to be able to take one side for only parts that did conflict, while
taking the change from the other side in places that did not).
--

From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 12:29 pm

Hi,



That is right, but for some stupid reason I did not realize that 
git-merge-file is not even called by merge-recursive.

So the accompanying patch for merge-recursive would use the --theirs or 
--ours logic only in the !index_only case, i.e. the final merge.

And I guess we'd have similar logic as for merge-subtree, introducing 
merge-recursive-ours and merge-recursive-theirs.

Very nice.

Ciao,
Dscho
--

From: Junio C Hamano
Date: Friday, June 20, 2008 - 12:38 am

Often people want their conflicting merges autoresolved by favouring
upstream changes (or their own --- it's the same thing), and hinted to run
"git diff --name-only | xargs git checkout MERGE_HEAD --".  This is
essentially to accept automerge results for the paths that are fully
resolved automatically while taking their version of the file in full for
paths that have conflicts.

This is problematic on two counts.

One problem is that this is not exactly what these people want.  They
usually want to salvage as much automerge result as possible.  In
particular, they want to keep autoresolved parts in conflicting paths, as
well as the paths that are fully autoresolved.

This patch teaches two new modes of operation to the lowest-lever merge
machinery, xdl_merge().  Instead of leaving the conflicted lines from both
sides enclosed in <<<, ===, and >>> markers, you can tell the conflicts to
be resolved favouring your side or their side of changes.

A larger problem is that this tends to encourage a bad workflow by
allowing them to record such a mixed up half-merge result as a full commit
without auditing.  This commit does not tackle this latter issue.  In git,
we usually give long enough rope to users with strange wishes as long as
the risky features is not on by default.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Essentially the same patch but with documentation.

 Documentation/git-merge-file.txt |   12 ++++++++++--
 builtin-merge-file.c             |   10 ++++++++--
 xdiff/xdiff.h                    |    8 +++++++-
 xdiff/xmerge.c                   |   24 ++++++++++++++++--------
 4 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 149f131..87e07d3 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git-merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
-	[-p|--stdout] ...
From: Junio C Hamano
Date: Friday, June 20, 2008 - 12:48 am

This uses the low-level mechanism for "ours" and "theirs" autoresolution
introduced by the previous commit to introduce two additional merge
strategies, merge-recursive-ours and merge-recursive-theirs.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile                     |    3 ++
 builtin-merge-recursive.c    |   37 ++++++++++++++++++++++++---
 git-merge.sh                 |    3 +-
 git.c                        |    2 +
 ll-merge.c                   |   24 +++++++++++------
 ll-merge.h                   |    4 ++-
 t/t6034-merge-ours-theirs.sh |   56 ++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 114 insertions(+), 15 deletions(-)
 create mode 100755 t/t6034-merge-ours-theirs.sh

diff --git a/Makefile b/Makefile
index b003e3e..82d2892 100644
--- a/Makefile
+++ b/Makefile
@@ -304,6 +304,8 @@ BUILT_INS += git-format-patch$X
 BUILT_INS += git-fsck-objects$X
 BUILT_INS += git-get-tar-commit-id$X
 BUILT_INS += git-init$X
+BUILT_INS += git-merge-recursive-ours$X
+BUILT_INS += git-merge-recursive-theirs$X
 BUILT_INS += git-merge-subtree$X
 BUILT_INS += git-peek-remote$X
 BUILT_INS += git-repo-config$X
@@ -1381,6 +1383,7 @@ check-docs::
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
+		git-merge-recursive-ours | git-merge-recursive-theirs | \
 		git-merge-resolve | git-merge-stupid | git-merge-subtree | \
 		git-fsck-objects | git-init-db | \
 		git-?*--?* ) continue ;; \
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 4aa28a1..a355e7a 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -20,7 +20,11 @@
 #include "attr.h"
 #include "merge-recursive.h"
 
-static int subtree_merge;
+static enum {
+	MERGE_RECURSIVE_SUBTREE = 1,
+	MERGE_RECURSIVE_OURS,
+	MERGE_RECURSIVE_THEIRS,
+} merge_recursive_variants;
 
 static struct tree *shift_tree_object(struct tree *one, struct tree *two)
 {
@@ -642,6 +646,7 @@ static int merge_3way(mmbuffer_t ...
From: Johannes Schindelin
Date: Friday, June 20, 2008 - 5:58 am

Hi,


Hrm.  I would have preferred something like this:

	if (!index_only && merge_recursive_variants == MERGE_RECURSIVE_OURS)
		favor = XDL_MERGE_FAVOR_OURS;
	if (!index_only && merge_recursive_variants == MERGE_RECURSIVE_THEIRS)
		favor = XDL_MERGE_FAVOR_THEIRS;
	else

Sorry, but in my opinion this flag mangling makes the whole code uglier.  
Why not just add another parameter?

Or if you are really concerned about future enhancements to ll_merge(), 

This just cries out loud for a new function suffixcmp().

I will not say anything about the long lines in git-merge.sh, since I 
fully expect builtin-merge to happen Real Soon Now.

Anyhow, your comments about this driving the wrong workflow still apply.  
Maybe we want to display them really, really prominently in 
Documentation/merge-strategies.txt.

Ciao,
Dscho
--

From: Johannes Sixt
Date: Monday, June 16, 2008 - 11:16 pm

Well, you could 'git ls-files --unmerged', but that prints the whole index
entry, not just the name, and then you need a more complicated pipeline.

-- Hannes

--

From: Sverre Rabbelier
Date: Tuesday, June 17, 2008 - 1:53 am

How about 'git ls-files -t | grep "^M " | xargs git checkout B --',
that would list all files that are unmerged and check them out?

-- 
Cheers,

Sverre Rabbelier
--

From: Johannes Schindelin
Date: Tuesday, June 17, 2008 - 2:48 am

Hi,


You probably meant 'sed -n "s/^M //p"' instead of 'grep "^M"', right?

Of course, I think that a better way would be to ask "git diff 
--name-only --diff-filter=U".

Ciao,
Dscho

--

From: Sverre Rabbelier
Date: Tuesday, June 17, 2008 - 2:53 am

On Tue, Jun 17, 2008 at 11:48 AM, Johannes Schindelin


Hmmm, not having an unfinished merge at hand, would that require the
sed on top too?

-- 
Cheers,

Sverre Rabbelier
--

From: Johannes Schindelin
Date: Tuesday, June 17, 2008 - 3:17 am

Hi,


Nope.  The "--name-only" says it will print only the name.

Ciao,
Dscho

--

Previous thread: [RFC/PATCH] git-instaweb: added logo and favicon by Flavio Poletti on Sunday, June 15, 2008 - 7:46 pm. (2 messages)

Next thread: How to omit/alter the sigend-off line in a patch set? by Ian Brown on Monday, June 16, 2008 - 3:24 am. (2 messages)