* [RFC/PATCH 01/18] Remove duplicate code
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 23:34 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 02/18] Avoid checking working copy when creating a virtual merge base Elijah Newren
` (19 subsequent siblings)
20 siblings, 1 reply; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
In commit 51931bf (merge-recursive: Improve handling of rename target vs.
directory addition, 2011-08-11) I apparently added two lines of code that
were immediately duplicated a few lines later. No idea why, other than
it seems pretty clear this was a mistake: there is no need to remove the
same file twice; removing it once is sufficient...especially since the
intervening line was working with a different file entirely.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
merge-recursive.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index b880ae5..d4292de 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1773,8 +1773,6 @@ static int process_entry(struct merge_options *o,
output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s"),
conf, path, other_branch, path, new_path);
- if (o->call_depth)
- remove_file_from_cache(path);
update_file(o, 0, sha, mode, new_path);
if (o->call_depth)
remove_file_from_cache(path);
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 01/18] Remove duplicate code
2016-04-08 6:58 ` [RFC/PATCH 01/18] Remove duplicate code Elijah Newren
@ 2016-04-08 23:34 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 23:34 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Elijah Newren <newren@gmail.com> writes:
> Subject: Re: [RFC/PATCH 01/18] Remove duplicate code
Subject: [RFC/PATCH 01/18] merge-recursive: remove duplicate code
> In commit 51931bf (merge-recursive: Improve handling of rename target vs.
> directory addition, 2011-08-11) I apparently added two lines of code that
> were immediately duplicated a few lines later. No idea why, other than
> it seems pretty clear this was a mistake: there is no need to remove the
> same file twice; removing it once is sufficient...especially since the
> intervening line was working with a different file entirely.
Interesting.
I briefly wondered if the update_files() in the middle may be
prevented by D/F conflict if you didn't remove the path beforehand,
but path and new_path are both in the same directory, so that would
not be the reason. Checking out 51931bf and running two tests (6022
and 6042) that were modified to protect the fix after applying this
partial revert does not seem to break, either, so...
Reviewed-by: Junio C Hamano <gitster@pobox.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> merge-recursive.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index b880ae5..d4292de 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1773,8 +1773,6 @@ static int process_entry(struct merge_options *o,
> output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
> "Adding %s as %s"),
> conf, path, other_branch, path, new_path);
> - if (o->call_depth)
> - remove_file_from_cache(path);
> update_file(o, 0, sha, mode, new_path);
> if (o->call_depth)
> remove_file_from_cache(path);
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC/PATCH 02/18] Avoid checking working copy when creating a virtual merge base
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 01/18] Remove duplicate code Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 03/18] Document weird bug in octopus merges via testcases Elijah Newren
` (18 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
There were a few cases in merge-recursive that could result in a check for
the presence of files in the working copy while trying to create a virtual
merge base. These were rare and innocuous, but somewhat illogical. The
two cases were:
* When there was naming conflicts (e.g. a D/F conflict) and we had to
pick a new unique name for a file. Since the new name is somewhat
arbitrary, it didn't matter that we consulted the working copy to
avoid picking a filename it has, but since the virtual merge base is
never checked out, it's a waste of time and slightly odd to do so.
* When two different files get renamed to the same name (on opposite
sides of the merge), we needed to delete the original filenames from
the cache and possibly also the working directory. The caller's check
for determining whether to delete from the working directory was a
call to would_lose_untracked(). It turns out this didn't matter
because remove_file() had logic to avoid modifying the working
directory when creating a virtual merge base, but there is no reason
for the caller to check the working directory in such circumstances.
It's a waste of time, if not also a bit weird.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
merge-recursive.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index d4292de..06d31ed 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -622,7 +622,7 @@ static char *unique_path(struct merge_options *o, const char *path, const char *
base_len = newpath.len;
while (string_list_has_string(&o->current_file_set, newpath.buf) ||
string_list_has_string(&o->current_directory_set, newpath.buf) ||
- file_exists(newpath.buf)) {
+ (!o->call_depth && file_exists(newpath.buf))) {
strbuf_setlen(&newpath, base_len);
strbuf_addf(&newpath, "_%d", suffix++);
}
@@ -1234,8 +1234,8 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
a->path, c1->path, ci->branch1,
b->path, c2->path, ci->branch2);
- remove_file(o, 1, a->path, would_lose_untracked(a->path));
- remove_file(o, 1, b->path, would_lose_untracked(b->path));
+ remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path));
+ remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path));
mfi_c1 = merge_file_special_markers(o, a, c1, &ci->ren1_other,
o->branch1, c1->path,
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 03/18] Document weird bug in octopus merges via testcases
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 01/18] Remove duplicate code Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 02/18] Avoid checking working copy when creating a virtual merge base Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 04/18] merge-octopus: Abort if index not clean Elijah Newren
` (17 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
...and check all other merge strategies with testcases while we're at it.
When the index has a staged change before running git merge, most the time
git merge will error out telling the user that the merge operation would
toss their merged changes unless they commit them first. There are two
exceptions:
* ff updates
* octopus merges
ff updates actually will error out if the staged change is to a path
modified between HEAD and the commit being merged. If the path(s) that
are staged are files unrelated to the changes between these two commits,
though, then an ff update will just keep these staged changes around after
the merge.
For octopus merges, the staged changes seem to take the place of HEAD just
before the merge is performed. So if the staged changes can be cleanly
merged with all the other heads, then the staged changes will just be
incorported into the resulting commit. If the staged changes cannot be
cleanly merged with all the other heads, the merge is not aborted -- merge
conflicts are simply reported as if HEAD had originally contained whatever
the index did.
Somewhat amusingly, though, an octopus merge will abort if the changes are
not staged. So, for example, if you do a 'git rm --cached somefile' right
before attempting an octopus merge then you'll get an error, but if you do
a 'git rm somefile' right before attempting an octopus merge git will
happily proceed. Similarly if you just modify a tracked file without
adding it.
...makes me wonder if I should add a testcase for unstaged and untracked
changes for each merge strategy while I'm at it. Hmmm...
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6044-merge-unrelated-index-changes.sh | 165 +++++++++++++++++++++++++++++++
1 file changed, 165 insertions(+)
create mode 100755 t/t6044-merge-unrelated-index-changes.sh
diff --git a/t/t6044-merge-unrelated-index-changes.sh b/t/t6044-merge-unrelated-index-changes.sh
new file mode 100755
index 0000000..726c898
--- /dev/null
+++ b/t/t6044-merge-unrelated-index-changes.sh
@@ -0,0 +1,165 @@
+#!/bin/sh
+
+test_description="merges with unrelated index changes"
+
+. ./test-lib.sh
+
+# Testcase for some simple merges
+# A
+# o-----o B
+# \
+# \---o C
+# \
+# \-o D
+# \
+# o E
+# Commit A: some file a
+# Commit B: adds file b, modifies end of a
+# Commit C: adds file c
+# Commit D: adds file d, modifies beginning of a
+# Commit E: renames a->subdir/a, adds subdir/e
+
+test_expect_success 'setup trivial merges' '
+ seq 1 10 >a &&
+ git add a &&
+ test_tick && git commit -m A &&
+
+ git branch A &&
+ git branch B &&
+ git branch C &&
+ git branch D &&
+ git branch E &&
+
+ git checkout B &&
+ echo b >b &&
+ echo 11 >>a &&
+ git add a b &&
+ test_tick && git commit -m B &&
+
+ git checkout C &&
+ echo c >c &&
+ git add c &&
+ test_tick && git commit -m C &&
+
+ git checkout D &&
+ seq 2 10 >a &&
+ echo d >d &&
+ git add a d &&
+ test_tick && git commit -m D &&
+
+ git checkout E &&
+ mkdir subdir &&
+ git mv a subdir/a &&
+ echo e >subdir/e &&
+ git add subdir &&
+ test_tick && git commit -m E
+'
+
+test_expect_success 'ff update' '
+ git reset --hard &&
+ git checkout A^0 &&
+
+ touch random_file && git add random_file &&
+
+ git merge E^0 &&
+
+ test_must_fail git rev-parse HEAD:random_file &&
+ test "$(git diff --name-only --cached E)" = "random_file"
+'
+
+test_expect_success 'ff update, important file modified' '
+ git reset --hard &&
+ git checkout A^0 &&
+
+ mkdir subdir &&
+ touch subdir/e &&
+ git add subdir/e &&
+
+ test_must_fail git merge E^0
+'
+
+test_expect_success 'resolve, trivial' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ test_must_fail git merge -s resolve C^0
+'
+
+test_expect_success 'resolve, non-trivial' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ test_must_fail git merge -s resolve D^0
+'
+
+test_expect_success 'recursive' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ test_must_fail git merge -s recursive C^0
+'
+
+test_expect_failure 'octopus, unrelated file touched' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ echo "I think the next line should fail, but maybe it was intended..." &&
+ test_might_fail git merge C^0 D^0 &&
+
+ echo "No matter what, random_file should NOT be part of HEAD" &&
+ test_must_fail git rev-parse HEAD:random_file
+'
+
+test_expect_failure 'octopus, related file removed' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git rm b &&
+
+ echo "I think the next line should fail, but maybe it was intended..." &&
+ test_might_fail git merge C^0 D^0 &&
+
+ echo "No matter what, b should still be in HEAD" &&
+ git cat-file -p HEAD:b | grep b$
+'
+
+test_expect_failure 'octopus, related file modified' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ echo 12 >>a && git add a &&
+
+ echo "I think the next line should fail, but maybe it was intended..." &&
+ test_might_fail git merge C^0 D^0 &&
+
+ echo "No matter what, 12 should NOT be in the copy of a from HEAD" &&
+ git cat-file -p HEAD:a | test_must_fail grep 12
+'
+
+test_expect_success 'ours' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ test_must_fail git merge -s ours C^0
+'
+
+test_expect_success 'subtree' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ touch random_file && git add random_file &&
+
+ test_must_fail git merge -s subtree E^0
+'
+
+test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 04/18] merge-octopus: Abort if index not clean
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (2 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 03/18] Document weird bug in octopus merges via testcases Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 19:31 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy Elijah Newren
` (16 subsequent siblings)
20 siblings, 1 reply; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
git-merge-octopus.sh | 5 +++++
t/t6044-merge-unrelated-index-changes.sh | 21 ++++++---------------
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 8643f74..a1d7702 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -44,6 +44,11 @@ esac
# MRC is the current "merge reference commit"
# MRT is the current "merge result tree"
+if ! git diff-index --quiet --cached HEAD --; then
+ echo "Error: Your local changes to the following files need to be resolved before merge"
+ git diff-index --cached --name-only HEAD -- | sed -e 's/^/ /'
+ exit 2
+fi
MRC=$(git rev-parse --verify -q $head)
MRT=$(git write-tree)
NON_FF_MERGE=0
diff --git a/t/t6044-merge-unrelated-index-changes.sh b/t/t6044-merge-unrelated-index-changes.sh
index 726c898..4c1dc5a 100755
--- a/t/t6044-merge-unrelated-index-changes.sh
+++ b/t/t6044-merge-unrelated-index-changes.sh
@@ -105,43 +105,34 @@ test_expect_success 'recursive' '
test_must_fail git merge -s recursive C^0
'
-test_expect_failure 'octopus, unrelated file touched' '
+test_expect_success 'octopus, unrelated file touched' '
git reset --hard &&
git checkout B^0 &&
touch random_file && git add random_file &&
echo "I think the next line should fail, but maybe it was intended..." &&
- test_might_fail git merge C^0 D^0 &&
-
- echo "No matter what, random_file should NOT be part of HEAD" &&
- test_must_fail git rev-parse HEAD:random_file
+ test_must_fail git merge C^0 D^0
'
-test_expect_failure 'octopus, related file removed' '
+test_expect_success 'octopus, related file removed' '
git reset --hard &&
git checkout B^0 &&
git rm b &&
echo "I think the next line should fail, but maybe it was intended..." &&
- test_might_fail git merge C^0 D^0 &&
-
- echo "No matter what, b should still be in HEAD" &&
- git cat-file -p HEAD:b | grep b$
+ test_must_fail git merge C^0 D^0
'
-test_expect_failure 'octopus, related file modified' '
+test_expect_success 'octopus, related file modified' '
git reset --hard &&
git checkout B^0 &&
echo 12 >>a && git add a &&
echo "I think the next line should fail, but maybe it was intended..." &&
- test_might_fail git merge C^0 D^0 &&
-
- echo "No matter what, 12 should NOT be in the copy of a from HEAD" &&
- git cat-file -p HEAD:a | test_must_fail grep 12
+ test_must_fail git merge C^0 D^0
'
test_expect_success 'ours' '
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 04/18] merge-octopus: Abort if index not clean
2016-04-08 6:58 ` [RFC/PATCH 04/18] merge-octopus: Abort if index not clean Elijah Newren
@ 2016-04-08 19:31 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 19:31 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Elijah Newren <newren@gmail.com> writes:
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> git-merge-octopus.sh | 5 +++++
> t/t6044-merge-unrelated-index-changes.sh | 21 ++++++---------------
> 2 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
> index 8643f74..a1d7702 100755
> --- a/git-merge-octopus.sh
> +++ b/git-merge-octopus.sh
> @@ -44,6 +44,11 @@ esac
> # MRC is the current "merge reference commit"
> # MRT is the current "merge result tree"
>
> +if ! git diff-index --quiet --cached HEAD --; then
Style.
> + echo "Error: Your local changes to the following files need to be resolved before merge"
> + git diff-index --cached --name-only HEAD -- | sed -e 's/^/ /'
> + exit 2
> +fi
Unless they are existing conflicts, the word "resolve" is misleading.
Erroring it out may not strictly be necessary (i.e. as Octopus is
meant to be used only when changes from the branches do not overlap,
it is likely that all of them are small simple changes that do not
touch any path that you currently have local modifications in the
index), but I do not think it is worth the effort to identify safe
cases that can continue, so I agree that this change to abort the
merge is a sensible one.
> MRC=$(git rev-parse --verify -q $head)
> MRT=$(git write-tree)
> NON_FF_MERGE=0
> diff --git a/t/t6044-merge-unrelated-index-changes.sh b/t/t6044-merge-unrelated-index-changes.sh
> index 726c898..4c1dc5a 100755
> --- a/t/t6044-merge-unrelated-index-changes.sh
> +++ b/t/t6044-merge-unrelated-index-changes.sh
> @@ -105,43 +105,34 @@ test_expect_success 'recursive' '
> test_must_fail git merge -s recursive C^0
> '
>
> -test_expect_failure 'octopus, unrelated file touched' '
> +test_expect_success 'octopus, unrelated file touched' '
> git reset --hard &&
> git checkout B^0 &&
>
> touch random_file && git add random_file &&
>
> echo "I think the next line should fail, but maybe it was intended..." &&
> - test_might_fail git merge C^0 D^0 &&
> -
> - echo "No matter what, random_file should NOT be part of HEAD" &&
> - test_must_fail git rev-parse HEAD:random_file
> + test_must_fail git merge C^0 D^0
> '
>
> -test_expect_failure 'octopus, related file removed' '
> +test_expect_success 'octopus, related file removed' '
> git reset --hard &&
> git checkout B^0 &&
>
> git rm b &&
>
> echo "I think the next line should fail, but maybe it was intended..." &&
> - test_might_fail git merge C^0 D^0 &&
> -
> - echo "No matter what, b should still be in HEAD" &&
> - git cat-file -p HEAD:b | grep b$
> + test_must_fail git merge C^0 D^0
> '
>
> -test_expect_failure 'octopus, related file modified' '
> +test_expect_success 'octopus, related file modified' '
> git reset --hard &&
> git checkout B^0 &&
>
> echo 12 >>a && git add a &&
>
> echo "I think the next line should fail, but maybe it was intended..." &&
> - test_might_fail git merge C^0 D^0 &&
> -
> - echo "No matter what, 12 should NOT be in the copy of a from HEAD" &&
> - git cat-file -p HEAD:a | test_must_fail grep 12
> + test_must_fail git merge C^0 D^0
> '
>
> test_expect_success 'ours' '
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (3 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 04/18] merge-octopus: Abort if index not clean Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 19:37 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 06/18] Add testcase for --index-only merges needing an ff update Elijah Newren
` (15 subsequent siblings)
20 siblings, 1 reply; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 170 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 170 insertions(+)
create mode 100755 t/t6043-merge-index-only.sh
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
new file mode 100755
index 0000000..b8b22ab
--- /dev/null
+++ b/t/t6043-merge-index-only.sh
@@ -0,0 +1,170 @@
+#!/bin/sh
+
+test_description="index-only merges"
+
+. ./test-lib.sh
+
+# Testcase setup for rename/modify:
+# Commit A: new file: a
+# Commit B: modify a slightly
+# Commit C: rename a->b
+# We should be able to merge B & C cleanly
+
+test_expect_success 'setup rename/modify merge' '
+ git init &&
+
+ printf "1\n2\n3\n4\n5\n6\n7\n" >a &&
+ git add a &&
+ git commit -m A &&
+ git tag A &&
+
+ git checkout -b B A &&
+ echo 8 >>a &&
+ git add a &&
+ git commit -m B &&
+
+ git checkout -b C A &&
+ git mv a b &&
+ git commit -m C
+'
+
+test_expect_failure '--index-only with rename/modify works in non-bare-clone' '
+ git checkout B^0 &&
+
+ git merge --index-only -s recursive C^0 &&
+
+ echo "Making sure the working copy was not updated" &&
+ test ! -f b &&
+ test -f a &&
+ test $(git rev-parse B:a) = $(git hash-object a) &&
+
+ echo "Making sure the index was updated" &&
+ test 1 -eq $(git ls-files -s | wc -l) &&
+ test $(git rev-parse B:a) = $(git rev-parse :b)
+'
+
+test_expect_failure '--index-only with rename/modify works in a bare clone' '
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s recursive C^0 &&
+
+ echo "Making sure the working copy was not updated" &&
+ test ! -f b &&
+ test ! -f a &&
+
+ echo "Making sure the index was updated" &&
+ test 1 -eq $(git ls-files -s | wc -l) &&
+ test $(git rev-parse B:a) = $(git rev-parse :b)
+ )
+'
+
+# Testcase requiring recursion to handle:
+#
+# L1 L2
+# o---o--o
+# / \ / \
+# B o X o C
+# \ / \ /
+# o---o--o
+# R1 R2
+#
+# B : 1..20
+# L1: 1..3 a..c 4..20
+# R1: 1..13n..p14..20
+# L2: 1..3 a..i 4..13 n..p 14..20
+# R2: 1..3 a..c 4..13 n..v 14..20
+# C: 1..3 a..i 4..13 n..v 14..20
+# needs 'recursive' strategy to get correct solution; 'resolve' would fail
+#
+
+test_expect_success 'setup single-file criss-cross resolvable with recursive strategy' '
+ git reset --hard &&
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ seq 1 20 >contents &&
+ git add contents &&
+ test_tick && git commit -m initial &&
+
+ git branch R1 &&
+ git checkout -b L1 &&
+ seq 1 3 >contents &&
+ seq 1 3 | tr 1-3 a-c >>contents &&
+ seq 4 20 >>contents &&
+ git add contents &&
+ test_tick && git commit -m L1 &&
+
+ git checkout R1 &&
+ seq 1 13 >contents &&
+ seq 1 3 | tr 1-3 n-p >>contents &&
+ seq 14 20 >>contents &&
+ git add contents &&
+ test_tick && git commit -m R1 &&
+
+ git checkout -b L2 L1^0 &&
+ test_tick && git merge R1 &&
+ seq 1 3 >contents &&
+ seq 1 9 | tr 1-9 a-i >>contents &&
+ seq 4 13 >>contents &&
+ seq 1 3 | tr 1-3 n-p >>contents &&
+ seq 14 20 >>contents &&
+ git add contents &&
+ test_tick && git commit -m L2 &&
+
+ git checkout -b R2 R1^0 &&
+ test_tick && git merge L1 &&
+ seq 1 3 >contents &&
+ seq 1 3 | tr 1-3 a-c >>contents &&
+ seq 4 13 >>contents &&
+ seq 1 9 | tr 1-9 n-v >>contents &&
+ seq 14 20 >>contents &&
+ git add contents &&
+ test_tick && git commit -m R2 &&
+
+ seq 1 3 >answer &&
+ seq 1 9 | tr 1-9 a-i >>answer &&
+ seq 4 13 >>answer &&
+ seq 1 9 | tr 1-9 n-v >>answer &&
+ seq 14 20 >>answer &&
+ git tag answer $(git hash-object -w answer) &&
+ rm -f answer
+'
+
+test_expect_failure 'recursive --index-only in non-bare repo' '
+ git reset --hard &&
+ git checkout L2^0 &&
+
+ git merge --index-only -s recursive R2^0 &&
+
+ test 1 = $(git ls-files -s | wc -l) &&
+ test 0 = $(git ls-files -u | wc -l) &&
+ test 0 = $(git ls-files -o | wc -l) &&
+
+ test $(git rev-parse :contents) = $(git rev-parse answer) &&
+ test $(git rev-parse L2:contents) = $(git hash-object contents)
+'
+
+test_expect_failure 'recursive --index-only in bare repo' '
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD L2 &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s recursive R2^0 &&
+
+ test 1 = $(git ls-files -s | wc -l) &&
+ test 0 = $(git ls-files -u | wc -l) &&
+
+ test $(git rev-parse :contents) = $(git rev-parse answer) &&
+ test ! -f contents
+ )
+'
+
+test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy
2016-04-08 6:58 ` [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy Elijah Newren
@ 2016-04-08 19:37 ` Junio C Hamano
2016-04-08 20:14 ` Junio C Hamano
0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 19:37 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Elijah Newren <newren@gmail.com> writes:
> +test_expect_failure '--index-only with rename/modify works in non-bare-clone' '
> + git checkout B^0 &&
> +
> + git merge --index-only -s recursive C^0 &&
> +
> + echo "Making sure the working copy was not updated" &&
> + test ! -f b &&
> + test -f a &&
> + test $(git rev-parse B:a) = $(git hash-object a) &&
> +
> + echo "Making sure the index was updated" &&
> + test 1 -eq $(git ls-files -s | wc -l) &&
> + test $(git rev-parse B:a) = $(git rev-parse :b)
The most crucial test that is missing (hence prevents reviewers from
judging if --index-only is a good idea at all) is the test on HEAD.
Does it record a merge between B and C and move HEAD there, i.e.
test $(git rev-parse HEAD^1) = $(git rev-parse B) &&
test $(git rev-parse HEAD^2) = $(git rev-parse C)
or does it make a merge but does not advance HEAD, i.e.
test $(git rev-parse HEAD) = $(git rev-parse B)
I fear that it may give a great headache to end users if you move
HEAD in a repository with a working tree to point at the merge
result--how do they reconcile the difference between the working
tree (which was based on B) and the index and HEAD (which is now
based on the result of the merge)? The next "git commit -a" would
appear that it would revert the changes brought in by this merge,
wouldn't it?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy
2016-04-08 19:37 ` Junio C Hamano
@ 2016-04-08 20:14 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 20:14 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Elijah Newren <newren@gmail.com> writes:
>
>> +test_expect_failure '--index-only with rename/modify works in non-bare-clone' '
>> + git checkout B^0 &&
>> +
>> + git merge --index-only -s recursive C^0 &&
>> +
>> + echo "Making sure the working copy was not updated" &&
>> + test ! -f b &&
>> + test -f a &&
>> + test $(git rev-parse B:a) = $(git hash-object a) &&
>> +
>> + echo "Making sure the index was updated" &&
>> + test 1 -eq $(git ls-files -s | wc -l) &&
>> + test $(git rev-parse B:a) = $(git rev-parse :b)
>
> The most crucial test that is missing (hence prevents reviewers from
> judging if --index-only is a good idea at all) is the test on HEAD.
> Does it record a merge between B and C and move HEAD there, i.e.
>
> test $(git rev-parse HEAD^1) = $(git rev-parse B) &&
> test $(git rev-parse HEAD^2) = $(git rev-parse C)
>
> or does it make a merge but does not advance HEAD, i.e.
>
> test $(git rev-parse HEAD) = $(git rev-parse B)
>
> I fear that it may give a great headache to end users if you move
> HEAD in a repository with a working tree to point at the merge
> result--how do they reconcile the difference between the working
> tree (which was based on B) and the index and HEAD (which is now
> based on the result of the merge)? The next "git commit -a" would
> appear that it would revert the changes brought in by this merge,
> wouldn't it?
And if the expectation that is not spelled out by this new test is
that the merge does not advance HEAD, the command does not make much
sense either, because there is no way for you to find out the merge
result.
Let's step back a bit. This may or may not be what you are aiming
at, but I think I can buy a series whose title is "merge without
working tree" if the big-picture objective of the topic were to
support this workflow:
0. I am working on my own branch that is not 'master'.
1. I see somebody else worked on and finished a good topic
'en/topic'. My work is not yet in a good shape, and I do not
want to switch to 'master' only to merge 'en/topic' to
'master', but 'en/topic' is so good and my urge to merge it
to 'master' is strong.
2. Hence I want a way to merge 'en/topic' to 'master' and
advance the tip of 'master', without using my working tree
(or the index or my HEAD for that matter), without disrupting
my current state.
3. For simplicity, I can live with it if the early version of
that "merge en/topic to master" operation is incapable of
handling conflicts. Then I'd say "huh, I'd need to manually
inspect the merge result so I'll do that later" and continue
what I was doing, or I'd say "OK, let's really have a look",
save what I was doing in a temporary commit and switch to
'master' to do the usual "git merge en/topic".
4. But it would be even better if I could give a temporary
directory to it as a scratch-pad area, and if I was asked to
help it resolving conflicts by editing conflicted files in
the scratch-pad area.
In any case, what is in my current index (or HEAD) is no use for
merging en/topic into master, so the command should not care or
touch not just what is in my working tree (which gives the operation
the same requirement as your series, i.e. "do not touch or require a
working tree") but also what is in my index or where my HEAD is.
The low-level ingredients that support "git merge", namely, "git
read-tree -m COMMON OURS THEIRS", actually has a support to make
this kind of "merge without fully populated working tree" possible,
by considering the lack of working tree file an equivalent to having
an unmodified working tree file (I suspect that merge-recursive is
probably broken with this regard, as it was mostly done without
knowing that we anticipated such a need in early life of Git long
before merge-recursive was invented). We can give an empty
"scratch-pad area" as a temporary working tree, create a temporary
index out of 'master' (in the above example), perform the usual
three-way merge with
GIT_DIR=... point at the real thing ...
GIT_INDEX_FILE=... point at a temporary file ...
export GIT_DIR GIT_INDEX_FILE
rm -fr /var/tmp/scratchpad && mkdir /var/tmp/scratchpad
cd /var/tmp/scratchpad
base=$(git merge-base master en/topic)
git read-tree -m $base master en/topic
and run "git merge -s resolve" to drive "git merge-one-file". This
would populate /var/tmp/scratchpad with files that are involved in
the merge, without wasting diskspace for files in 'master' that does
not change, and you can edit them, "git add" these paths, and then
do a "git write-tree" to create the tree that represents the merge
result. You obviously need to avoid touching HEAD when recording
that tree as a merge commit and advance the tip of 'master' (as HEAD
is still on my own unrelated branch), but that part is trivial
(i.e. we have written "git commit" at least twice).
Of course, if you are not going to support merges that need manual
conflict resolution, you do not need the scratchpad and the end user
experience would be simpler (i.e. you do not have to think about how
they resolve, record the resolution, etc.) And your "merge only in
index" topic would fit well in that picture.
But then, using the current index and the HEAD as the place to merge
into is still wrong. I think this is better done as a separate
command, not "git merge" but perhaps something like
$ git merge-to en/topic master
or something. "git merge --into=master en/topic" is also a
possibility.
But as I said in the early part of this message, this may or may not
be what you are aiming at. The above is the most sensible tangent
that I can think of that may be related to "merge only in index".
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC/PATCH 06/18] Add testcase for --index-only merges needing an ff update
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (4 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 05/18] Add testcase for --index-only merges needing the recursive strategy Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 07/18] Add testcase for --index-only merges with the resolve strategy Elijah Newren
` (14 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index b8b22ab..67c8e92 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -167,4 +167,74 @@ test_expect_failure 'recursive --index-only in bare repo' '
)
'
+# Testcase for a simple ff update
+# A
+# o-----o E
+#
+# Commit A: some file a
+# Commit E: renames a->subdir/a, adds subdir/e
+
+test_expect_success 'setup simple ff update' '
+ git reset --hard &&
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ seq 1 10 >a &&
+ git add a &&
+ test_tick && git commit -m A &&
+
+ git branch A &&
+ git branch E &&
+
+ git checkout E &&
+ mkdir subdir &&
+ git mv a subdir/a &&
+ echo e >subdir/e &&
+ git add subdir &&
+ test_tick && git commit -m E
+'
+
+test_expect_failure '--index-only ff update, non-bare' '
+ git reset --hard &&
+ git checkout A^0 &&
+
+ git merge --index-only --ff-only E^0 &&
+
+ git diff --staged --exit-code E &&
+ test $(git hash-object a) = $(git rev-parse A:a) &&
+ test ! -d subdir
+'
+
+test_expect_failure '--index-only ff update, bare' '
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD A &&
+ git read-tree HEAD &&
+
+ git merge --index-only --ff-only E^0 &&
+
+ git diff --staged --exit-code E &&
+ test ! -f a &&
+ test ! -d subdir
+ )
+'
+
+test_expect_failure '--index-only ff update, non-bare with uncommitted changes' '
+ git clean -fdx &&
+ git reset --hard &&
+ git checkout A^0 &&
+
+ touch random_file && git add random_file &&
+
+ git merge --index-only --ff-only E^0 &&
+
+ test_must_fail git rev-parse HEAD:random_file &&
+ test "$(git diff --name-only --cached E)" = "random_file" &&
+ test $(git hash-object a) = $(git rev-parse A:a) &&
+ test ! -d subdir
+'
+
test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 07/18] Add testcase for --index-only merges with the resolve strategy
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (5 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 06/18] Add testcase for --index-only merges needing an ff update Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 08/18] Add testcase for --index-only merges with the octopus strategy Elijah Newren
` (13 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Including both tests which can be handled without calling the actual
git-merge-resolve program (because the merge is trivially resolvable
without worrying about 3-way file merges), and ones that do need it to be
invoked.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 104 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 100 insertions(+), 4 deletions(-)
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 67c8e92..4c3c40a 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -167,14 +167,22 @@ test_expect_failure 'recursive --index-only in bare repo' '
)
'
-# Testcase for a simple ff update
+# Testcase for some simple merges
# A
-# o-----o E
-#
+# o-----o B
+# \
+# \---o C
+# \
+# \-o D
+# \
+# o E
# Commit A: some file a
+# Commit B: adds file b, modifies end of a
+# Commit C: adds file c
+# Commit D: adds file d, modifies beginning of a
# Commit E: renames a->subdir/a, adds subdir/e
-test_expect_success 'setup simple ff update' '
+test_expect_success 'setup simple merges' '
git reset --hard &&
git rm -rf . &&
git clean -fdqx &&
@@ -186,8 +194,28 @@ test_expect_success 'setup simple ff update' '
test_tick && git commit -m A &&
git branch A &&
+ git branch B &&
+ git branch C &&
+ git branch D &&
git branch E &&
+ git checkout B &&
+ echo b >b &&
+ echo 11 >>a &&
+ git add a b &&
+ test_tick && git commit -m B &&
+
+ git checkout C &&
+ echo c >c &&
+ git add c &&
+ test_tick && git commit -m C &&
+
+ git checkout D &&
+ seq 2 10 >a &&
+ echo d >d &&
+ git add a d &&
+ test_tick && git commit -m D &&
+
git checkout E &&
mkdir subdir &&
git mv a subdir/a &&
@@ -237,4 +265,72 @@ test_expect_failure '--index-only ff update, non-bare with uncommitted changes'
test ! -d subdir
'
+test_expect_failure '--index-only w/ resolve, trivial, non-bare' '
+ git clean -fdx &&
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git merge --index-only -s resolve C^0 | grep Wonderful &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :c) = $(git rev-parse C:c) &&
+ test ! -f c
+'
+
+test_expect_failure '--index-only w/ resolve, trivial, bare' '
+ rm -rf bare.clone &&
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s resolve C^0 | grep Wonderful &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :c) = $(git rev-parse C:c) &&
+ test ! -f a &&
+ test ! -f b &&
+ test ! -f c
+ )
+'
+
+test_expect_failure '--index-only w/ resolve, non-trivial, non-bare' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git merge --index-only -s resolve D^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) != $(git rev-parse B:a) &&
+ test $(git rev-parse :a) != $(git rev-parse D:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :d) = $(git rev-parse D:d) &&
+ test $(git hash-object a) = $(git rev-parse B:a) &&
+ test ! -f d
+'
+
+test_expect_failure '--index-only w/ resolve, non-trivial, bare' '
+ rm -rf bare.clone &&
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s resolve D^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) != $(git rev-parse B:a) &&
+ test $(git rev-parse :a) != $(git rev-parse D:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :d) = $(git rev-parse D:d) &&
+ test ! -f a
+ )
+'
+
test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 08/18] Add testcase for --index-only merges with the octopus strategy
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (6 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 07/18] Add testcase for --index-only merges with the resolve strategy Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 09/18] Add testcase for --index-only merges with the ours strategy Elijah Newren
` (12 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 4c3c40a..b01bf79 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -333,4 +333,45 @@ test_expect_failure '--index-only w/ resolve, non-trivial, bare' '
)
'
+test_expect_failure '--index-only octopus, non-bare' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git merge --index-only -s octopus C^0 D^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 5 &&
+ test $(git rev-parse :a) != $(git rev-parse B:a) &&
+ test $(git rev-parse :a) != $(git rev-parse C:a) &&
+ test $(git rev-parse :a) != $(git rev-parse D:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :c) = $(git rev-parse C:c) &&
+ test $(git rev-parse :d) = $(git rev-parse D:d) &&
+ test $(git hash-object a) = $(git rev-parse B:a) &&
+ test ! -f c &&
+ test ! -f d
+'
+
+test_expect_failure '--index-only octopus, bare' '
+ rm -rf bare.clone &&
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s octopus C^0 D^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 5 &&
+ test $(git rev-parse :a) != $(git rev-parse B:a) &&
+ test $(git rev-parse :a) != $(git rev-parse C:a) &&
+ test $(git rev-parse :a) != $(git rev-parse D:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :c) = $(git rev-parse C:c) &&
+ test $(git rev-parse :d) = $(git rev-parse D:d) &&
+ test ! -f a &&
+ test ! -f c &&
+ test ! -f d
+ )
+'
+
test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 09/18] Add testcase for --index-only merges with the ours strategy
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (7 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 08/18] Add testcase for --index-only merges with the octopus strategy Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 10/18] Add testcase for --index-only merges with the subtree strategy Elijah Newren
` (11 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
This is almost trivial to make work, because we already know that the
working tree will need no modifications, but lets test it for completeness
anyway.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index b01bf79..84c6640 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -374,4 +374,36 @@ test_expect_failure '--index-only octopus, bare' '
)
'
+test_expect_failure '--index-only ours, non-bare' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git merge --index-only -s ours C^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test_must_fail git rev-parse :c &&
+ test ! -f c
+'
+
+test_expect_failure '--index-only ours, bare' '
+ rm -rf bare.clone &&
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s ours C^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test_must_fail git rev-parse :c &&
+ test ! -f a &&
+ test ! -f c
+ )
+'
+
test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 10/18] Add testcase for --index-only merges with the subtree strategy
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (8 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 09/18] Add testcase for --index-only merges with the ours strategy Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented Elijah Newren
` (10 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-index-only.sh | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 84c6640..5eda6b3 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -406,4 +406,38 @@ test_expect_failure '--index-only ours, bare' '
)
'
+test_expect_failure '--index-only subtree, non-bare' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git merge --index-only -s subtree E^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :e) = $(git rev-parse E:subdir/e) &&
+ test ! -d subdir &&
+ test ! -f e
+'
+
+test_expect_failure '--index-only subtree, bare' '
+ rm -rf bare.clone &&
+ git clone --bare . bare.clone &&
+ (cd bare.clone &&
+
+ git update-ref --no-deref HEAD B &&
+ git read-tree HEAD &&
+
+ git merge --index-only -s subtree E^0 &&
+
+ test "$(git rev-list --count HEAD)" -eq 4 &&
+ test $(git rev-parse :a) = $(git rev-parse B:a) &&
+ test $(git rev-parse :b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :e) = $(git rev-parse E:subdir/e) &&
+ test ! -d subdir &&
+ test ! -f a &&
+ test ! -f e
+ )
+'
+
test_done
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (9 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 10/18] Add testcase for --index-only merges with the subtree strategy Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 22:33 ` Junio C Hamano
2016-04-08 6:58 ` [RFC/PATCH 12/18] Add --index-only support for recursive merges Elijah Newren
` (9 subsequent siblings)
20 siblings, 1 reply; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Subsequent commits will add implementation for each relevant merge strategy
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-merge.txt | 14 ++++++++++++++
builtin/merge.c | 7 +++++++
git.c | 2 +-
merge-recursive.c | 1 +
merge-recursive.h | 1 +
t/t6043-merge-index-only.sh | 4 ++--
6 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 07f7295..82a6d43 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -86,6 +86,20 @@ invocations. The automated message can include the branch description.
Allow the rerere mechanism to update the index with the
result of auto-conflict resolution if possible.
+--index-only::
+ Perform merge on index only, leaving working tree alone. Most
+ users do NOT want to use this flag, as it will leave the
+ working tree and the index completely out of sync, which is
+ very likely to confuse users and prevent a subsequent 'git
+ merge --abort' from working. It is intended for script
+ writers to have a way to easily check whether a merge would
+ succeed and which files would conflict, typically from bare
+ clones.
++
+Note that files other than the index in the git-dir may also be
+modified when using this flag (e.g. MERGE_HEAD, MERGE_MSG, MERGE_RR,
+HEAD, whatever HEAD points at, and new objects under objects/).
+
--abort::
Abort the current conflict resolution process, and
try to reconstruct the pre-merge state.
diff --git a/builtin/merge.c b/builtin/merge.c
index 101ffef..ce5be0a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -63,6 +63,7 @@ static char *branch_mergeoptions;
static int option_renormalize;
static int verbosity;
static int allow_rerere_auto;
+static int index_only = 0;
static int abort_current_merge;
static int show_progress = -1;
static int default_to_upstream = 1;
@@ -221,6 +222,8 @@ static struct option builtin_merge_options[] = {
OPT__VERBOSITY(&verbosity),
OPT_BOOL(0, "abort", &abort_current_merge,
N_("abort the current in-progress merge")),
+ OPT_BOOL(0, "index-only", &index_only,
+ N_("perform merge on index only, leaving working tree alone")),
OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
@@ -663,6 +666,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
o.subtree_shift = "";
o.renormalize = option_renormalize;
+ o.index_only = index_only;
o.show_rename_progress =
show_progress == -1 ? isatty(2) : show_progress;
@@ -1193,6 +1197,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
parse_branch_merge_options(branch_mergeoptions);
argc = parse_options(argc, argv, prefix, builtin_merge_options,
builtin_merge_usage, 0);
+ if (!index_only)
+ setup_work_tree();
+
if (shortlog_len < 0)
shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
diff --git a/git.c b/git.c
index 6cc0c07..b07485a 100644
--- a/git.c
+++ b/git.c
@@ -427,7 +427,7 @@ static struct cmd_struct commands[] = {
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo },
{ "mailsplit", cmd_mailsplit },
- { "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
+ { "merge", cmd_merge, RUN_SETUP },
{ "merge-base", cmd_merge_base, RUN_SETUP },
{ "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
{ "merge-index", cmd_merge_index, RUN_SETUP },
diff --git a/merge-recursive.c b/merge-recursive.c
index 06d31ed..b346ed6 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2041,6 +2041,7 @@ void init_merge_options(struct merge_options *o)
o->merge_rename_limit = -1;
o->renormalize = 0;
o->detect_rename = 1;
+ o->index_only = 0;
merge_recursive_config(o);
if (getenv("GIT_MERGE_VERBOSITY"))
o->verbosity =
diff --git a/merge-recursive.h b/merge-recursive.h
index 52f0201..7e9955f 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -15,6 +15,7 @@ struct merge_options {
const char *subtree_shift;
unsigned buffer_output : 1;
unsigned renormalize : 1;
+ unsigned index_only : 1;
long xdl_opts;
int verbosity;
int detect_rename;
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 5eda6b3..9bb64d8 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -374,7 +374,7 @@ test_expect_failure '--index-only octopus, bare' '
)
'
-test_expect_failure '--index-only ours, non-bare' '
+test_expect_success '--index-only ours, non-bare' '
git reset --hard &&
git checkout B^0 &&
@@ -387,7 +387,7 @@ test_expect_failure '--index-only ours, non-bare' '
test ! -f c
'
-test_expect_failure '--index-only ours, bare' '
+test_expect_success '--index-only ours, bare' '
rm -rf bare.clone &&
git clone --bare . bare.clone &&
(cd bare.clone &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented
2016-04-08 6:58 ` [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented Elijah Newren
@ 2016-04-08 22:33 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 22:33 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Elijah Newren <newren@gmail.com> writes:
> +--index-only::
> + Perform merge on index only, leaving working tree alone. Most
> + users do NOT want to use this flag, as it will leave the
> + working tree and the index completely out of sync, which is
> + very likely to confuse users and prevent a subsequent 'git
> + merge --abort' from working.
This whole paragraph is a strong sign that this feature as-is is not
a good addition.
On the other hand ...
> + It is intended for script
> + writers to have a way to easily check whether a merge would
> + succeed and which files would conflict, typically from bare
> + clones.
... this may be a good goal to aim for, but I think you are
understating. You seem to be wanting a lot more than "easily check
whether..."; isn't this more like "It is for scripts to create a
merge and advance the branch when no need for manual conflict
resolution in a bare repository, and to learn which paths would
require manual conflicts when it fails", no?
I'd think either (1) limit this to bare repository only, or (2) do
the "git merge --into master en/topic" I outlined in the other
message, (or both) would be a sensible alternative for a feature
whose description has to begin with "Most users do not want this".
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC/PATCH 12/18] Add --index-only support for recursive merges
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (10 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 11/18] merge: Add a new --index-only option, not yet implemented Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 13/18] Add --index-only support with read_tree_trivial merges, kind of Elijah Newren
` (8 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
The merge-recursive logic already had the ability to ignore the working
directory and operate entirely on the index -- it needed to do this when
creating a virtual merge base, i.e. when o->call_depth > 0.
The only trick here is that o->call_depth > 0 was also checked to
determine whether all merges conflicts should be forcibly immediately
resolved (typically by treating a copy of the code with conflict markers
still in it as the resolution) in order to get us some actual base tree
to work with.
Introduce a new merge option o->index_only which is true whenver either
--index-only is passed or o->call_depth > 0, and make the portions of
merge-recursive that are about operating only on the index look at
o->index_only, and the portions that are about forcibly immediately
resolving conflicts check o->call_depth.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
merge-recursive.c | 37 ++++++++++++++++++++++---------------
t/t6043-merge-index-only.sh | 12 ++++++------
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index b346ed6..6af37ed 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -406,10 +406,11 @@ static void record_df_conflict_files(struct merge_options *o,
int i;
/*
- * If we're merging merge-bases, we don't want to bother with
- * any working directory changes.
+ * If we're doing an index-only merge, we don't need to check for
+ * which files to remove from the working copy to make room for
+ * paths below directories of D/F conflicts; we can just exit early.
*/
- if (o->call_depth)
+ if (o->index_only)
return;
/* Ensure D/F conflicts are adjacent in the entries list. */
@@ -581,7 +582,7 @@ static int remove_file(struct merge_options *o, int clean,
const char *path, int no_wd)
{
int update_cache = o->call_depth || clean;
- int update_working_directory = !o->call_depth && !no_wd;
+ int update_working_directory = !o->index_only && !no_wd;
if (update_cache) {
if (remove_file_from_cache(path))
@@ -622,7 +623,7 @@ static char *unique_path(struct merge_options *o, const char *path, const char *
base_len = newpath.len;
while (string_list_has_string(&o->current_file_set, newpath.buf) ||
string_list_has_string(&o->current_directory_set, newpath.buf) ||
- (!o->call_depth && file_exists(newpath.buf))) {
+ (!o->index_only && file_exists(newpath.buf))) {
strbuf_setlen(&newpath, base_len);
strbuf_addf(&newpath, "_%d", suffix++);
}
@@ -742,7 +743,7 @@ static void update_file_flags(struct merge_options *o,
int update_cache,
int update_wd)
{
- if (o->call_depth)
+ if (o->index_only)
update_wd = 0;
if (update_wd) {
@@ -813,7 +814,7 @@ static void update_file(struct merge_options *o,
unsigned mode,
const char *path)
{
- update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->call_depth);
+ update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->index_only);
}
/* Low level file merging, update and removal */
@@ -1017,7 +1018,7 @@ static void handle_change_delete(struct merge_options *o,
const char *change, const char *change_past)
{
char *renamed = NULL;
- if (dir_in_way(path, !o->call_depth)) {
+ if (dir_in_way(path, !o->index_only)) {
renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
}
@@ -1145,7 +1146,7 @@ static void handle_file(struct merge_options *o,
remove_file(o, 0, rename->path, 0);
dst_name = unique_path(o, rename->path, cur_branch);
} else {
- if (dir_in_way(rename->path, !o->call_depth)) {
+ if (dir_in_way(rename->path, !o->index_only)) {
dst_name = unique_path(o, rename->path, cur_branch);
output(o, 1, _("%s is a directory in %s adding as %s instead"),
rename->path, other_branch, dst_name);
@@ -1234,8 +1235,8 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
a->path, c1->path, ci->branch1,
b->path, c2->path, ci->branch2);
- remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path));
- remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path));
+ remove_file(o, 1, a->path, o->index_only || would_lose_untracked(a->path));
+ remove_file(o, 1, b->path, o->index_only || would_lose_untracked(b->path));
mfi_c1 = merge_file_special_markers(o, a, c1, &ci->ren1_other,
o->branch1, c1->path,
@@ -1619,7 +1620,7 @@ static int merge_content(struct merge_options *o,
o->branch2 == rename_conflict_info->branch1) ?
pair1->two->path : pair1->one->path;
- if (dir_in_way(path, !o->call_depth))
+ if (dir_in_way(path, !o->index_only))
df_conflict_remains = 1;
}
mfi = merge_file_special_markers(o, &one, &a, &b,
@@ -1639,7 +1640,7 @@ static int merge_content(struct merge_options *o,
path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
if (!path_renamed_outside_HEAD) {
add_cacheinfo(mfi.mode, mfi.sha, path,
- 0, (!o->call_depth), 0);
+ 0, (!o->index_only), 0);
return mfi.clean;
}
} else
@@ -1767,7 +1768,7 @@ static int process_entry(struct merge_options *o,
sha = b_sha;
conf = _("directory/file");
}
- if (dir_in_way(path, !o->call_depth)) {
+ if (dir_in_way(path, !o->index_only)) {
char *new_path = unique_path(o, path, add_branch);
clean_merge = 0;
output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
@@ -1819,7 +1820,7 @@ int merge_trees(struct merge_options *o,
return 1;
}
- code = git_merge_trees(o->call_depth, common, head, merge);
+ code = git_merge_trees(o->index_only, common, head, merge);
if (code != 0) {
if (show(o, 4) || o->call_depth)
@@ -1899,6 +1900,7 @@ int merge_recursive(struct merge_options *o,
struct commit *merged_common_ancestors;
struct tree *mrtree = mrtree;
int clean;
+ int prev_index_only_setting;
if (show(o, 4)) {
output(o, 4, _("Merging:"));
@@ -1929,9 +1931,12 @@ int merge_recursive(struct merge_options *o,
merged_common_ancestors = make_virtual_commit(tree, "ancestor");
}
+ prev_index_only_setting = o->index_only;
+
for (iter = ca; iter; iter = iter->next) {
const char *saved_b1, *saved_b2;
o->call_depth++;
+ o->index_only = 1;
/*
* When the merge fails, the result contains files
* with conflict markers. The cleanness flag is
@@ -1954,6 +1959,8 @@ int merge_recursive(struct merge_options *o,
die(_("merge returned no commit"));
}
+ o->index_only = prev_index_only_setting;
+
discard_cache();
if (!o->call_depth)
read_cache();
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 9bb64d8..2e1d953 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -28,7 +28,7 @@ test_expect_success 'setup rename/modify merge' '
git commit -m C
'
-test_expect_failure '--index-only with rename/modify works in non-bare-clone' '
+test_expect_success '--index-only with rename/modify works in non-bare-clone' '
git checkout B^0 &&
git merge --index-only -s recursive C^0 &&
@@ -43,7 +43,7 @@ test_expect_failure '--index-only with rename/modify works in non-bare-clone' '
test $(git rev-parse B:a) = $(git rev-parse :b)
'
-test_expect_failure '--index-only with rename/modify works in a bare clone' '
+test_expect_success '--index-only with rename/modify works in a bare clone' '
git clone --bare . bare.clone &&
(cd bare.clone &&
@@ -136,7 +136,7 @@ test_expect_success 'setup single-file criss-cross resolvable with recursive str
rm -f answer
'
-test_expect_failure 'recursive --index-only in non-bare repo' '
+test_expect_success 'recursive --index-only in non-bare repo' '
git reset --hard &&
git checkout L2^0 &&
@@ -150,7 +150,7 @@ test_expect_failure 'recursive --index-only in non-bare repo' '
test $(git rev-parse L2:contents) = $(git hash-object contents)
'
-test_expect_failure 'recursive --index-only in bare repo' '
+test_expect_success 'recursive --index-only in bare repo' '
git clone --bare . bare.clone &&
(cd bare.clone &&
@@ -406,7 +406,7 @@ test_expect_success '--index-only ours, bare' '
)
'
-test_expect_failure '--index-only subtree, non-bare' '
+test_expect_success '--index-only subtree, non-bare' '
git reset --hard &&
git checkout B^0 &&
@@ -420,7 +420,7 @@ test_expect_failure '--index-only subtree, non-bare' '
test ! -f e
'
-test_expect_failure '--index-only subtree, bare' '
+test_expect_success '--index-only subtree, bare' '
rm -rf bare.clone &&
git clone --bare . bare.clone &&
(cd bare.clone &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 13/18] Add --index-only support with read_tree_trivial merges, kind of
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (11 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 12/18] Add --index-only support for recursive merges Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 14/18] Add --index-only support for ff_only merges Elijah Newren
` (7 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@palantir.com>
This almost works. It creates the correct merge commit, updates the
branch, but then if the repo is bare it leaves the index as it was before
the merge (and if the repo is non-bare it updates the index). I'm
totally lost as to why the testcase behaves differently in the bare and
non-bare cases.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/merge.c | 3 ++-
t/t6043-merge-index-only.sh | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index ce5be0a..5f463ad 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -606,7 +606,8 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
opts.head_idx = 2;
opts.src_index = &the_index;
opts.dst_index = &the_index;
- opts.update = 1;
+ opts.update = !index_only;
+ opts.index_only = index_only;
opts.verbose_update = 1;
opts.trivial_merges_only = 1;
opts.merge = 1;
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index 2e1d953..f79782c 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -265,7 +265,7 @@ test_expect_failure '--index-only ff update, non-bare with uncommitted changes'
test ! -d subdir
'
-test_expect_failure '--index-only w/ resolve, trivial, non-bare' '
+test_expect_success '--index-only w/ resolve, trivial, non-bare' '
git clean -fdx &&
git reset --hard &&
git checkout B^0 &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 14/18] Add --index-only support for ff_only merges
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (12 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 13/18] Add --index-only support with read_tree_trivial merges, kind of Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 15/18] merge: Pass --index-only along to external merge strategy programs Elijah Newren
` (6 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@palantir.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/merge.c | 1 +
builtin/pull.c | 4 ++--
cache.h | 1 +
merge.c | 4 +++-
sequencer.c | 2 +-
t/t6043-merge-index-only.sh | 6 +++---
6 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 5f463ad..b791702 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1443,6 +1443,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (checkout_fast_forward(head_commit->object.oid.hash,
commit->object.oid.hash,
+ index_only,
overwrite_ignore)) {
ret = 1;
goto done;
diff --git a/builtin/pull.c b/builtin/pull.c
index 10eff03..4f33a89 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -569,7 +569,7 @@ static int pull_into_void(const unsigned char *merge_head,
* index/worktree changes that the user already made on the unborn
* branch.
*/
- if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))
+ if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0, 0))
return 1;
if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))
@@ -871,7 +871,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
"fast-forwarding your working tree from\n"
"commit %s."), sha1_to_hex(orig_head));
- if (checkout_fast_forward(orig_head, curr_head, 0))
+ if (checkout_fast_forward(orig_head, curr_head, 0, 0))
die(_("Cannot fast-forward your working tree.\n"
"After making sure that you saved anything precious from\n"
"$ git diff %s\n"
diff --git a/cache.h b/cache.h
index b829410..d51fcbc 100644
--- a/cache.h
+++ b/cache.h
@@ -1785,6 +1785,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
const char *head_arg, struct commit_list *remotes);
int checkout_fast_forward(const unsigned char *from,
const unsigned char *to,
+ int index_only,
int overwrite_ignore);
diff --git a/merge.c b/merge.c
index 5db7d56..a40307c 100644
--- a/merge.c
+++ b/merge.c
@@ -46,6 +46,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
int checkout_fast_forward(const unsigned char *head,
const unsigned char *remote,
+ int index_only,
int overwrite_ignore)
{
struct tree *trees[MAX_UNPACK_TREES];
@@ -72,7 +73,8 @@ int checkout_fast_forward(const unsigned char *head,
opts.head_idx = 1;
opts.src_index = &the_index;
opts.dst_index = &the_index;
- opts.update = 1;
+ opts.update = !index_only;
+ opts.index_only = index_only;
opts.verbose_update = 1;
opts.merge = 1;
opts.fn = twoway_merge;
diff --git a/sequencer.c b/sequencer.c
index e66f2fe..1b772fb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -222,7 +222,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
struct strbuf err = STRBUF_INIT;
read_cache();
- if (checkout_fast_forward(from, to, 1))
+ if (checkout_fast_forward(from, to, 0, 1))
exit(128); /* the callee should have complained already */
strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index f79782c..cb860f2 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -224,7 +224,7 @@ test_expect_success 'setup simple merges' '
test_tick && git commit -m E
'
-test_expect_failure '--index-only ff update, non-bare' '
+test_expect_success '--index-only ff update, non-bare' '
git reset --hard &&
git checkout A^0 &&
@@ -235,7 +235,7 @@ test_expect_failure '--index-only ff update, non-bare' '
test ! -d subdir
'
-test_expect_failure '--index-only ff update, bare' '
+test_expect_success '--index-only ff update, bare' '
git clone --bare . bare.clone &&
(cd bare.clone &&
@@ -250,7 +250,7 @@ test_expect_failure '--index-only ff update, bare' '
)
'
-test_expect_failure '--index-only ff update, non-bare with uncommitted changes' '
+test_expect_success '--index-only ff update, non-bare with uncommitted changes' '
git clean -fdx &&
git reset --hard &&
git checkout A^0 &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 15/18] merge: Pass --index-only along to external merge strategy programs
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (13 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 14/18] Add --index-only support for ff_only merges Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 16/18] git-merge-one-file.sh: support --index-only option Elijah Newren
` (5 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/merge-index.c | 9 ++++++++-
builtin/merge.c | 2 +-
cache.h | 2 +-
merge.c | 4 +++-
sequencer.c | 2 +-
5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 1c3427c..53cc285 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -4,14 +4,17 @@
static const char *pgm;
static int one_shot, quiet;
static int err;
+static int index_only;
static int merge_entry(int pos, const char *path)
{
int found;
- const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
+ const char *arguments[] = { pgm, "", "", "", path, "", "", "", "--index-only", NULL };
char hexbuf[4][GIT_SHA1_HEXSZ + 1];
char ownbuf[4][60];
+ if (!index_only)
+ arguments[8] = NULL;
if (pos >= active_nr)
die("git merge-index: %s not in the cache", path);
found = 0;
@@ -96,6 +99,10 @@ int cmd_merge_index(int argc, const char **argv, const char *prefix)
force_file = 1;
continue;
}
+ if (!strcmp(arg, "--index-only")) {
+ index_only = 1;
+ continue;
+ }
if (!strcmp(arg, "-a")) {
merge_all();
continue;
diff --git a/builtin/merge.c b/builtin/merge.c
index b791702..d1fe3f9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -690,7 +690,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
rollback_lock_file(&lock);
return clean ? 0 : 1;
} else {
- return try_merge_command(strategy, xopts_nr, xopts,
+ return try_merge_command(strategy, index_only, xopts_nr, xopts,
common, head_arg, remoteheads);
}
}
diff --git a/cache.h b/cache.h
index d51fcbc..d017155 100644
--- a/cache.h
+++ b/cache.h
@@ -1780,7 +1780,7 @@ extern struct startup_info *startup_info;
/* merge.c */
struct commit_list;
-int try_merge_command(const char *strategy, size_t xopts_nr,
+int try_merge_command(const char *strategy, int index_only, size_t xopts_nr,
const char **xopts, struct commit_list *common,
const char *head_arg, struct commit_list *remotes);
int checkout_fast_forward(const unsigned char *from,
diff --git a/merge.c b/merge.c
index a40307c..705a8d7 100644
--- a/merge.c
+++ b/merge.c
@@ -15,7 +15,7 @@ static const char *merge_argument(struct commit *commit)
return EMPTY_TREE_SHA1_HEX;
}
-int try_merge_command(const char *strategy, size_t xopts_nr,
+int try_merge_command(const char *strategy, int index_only, size_t xopts_nr,
const char **xopts, struct commit_list *common,
const char *head_arg, struct commit_list *remotes)
{
@@ -24,6 +24,8 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
struct commit_list *j;
argv_array_pushf(&args, "merge-%s", strategy);
+ if (index_only)
+ argv_array_pushf(&args, "--index-only");
for (i = 0; i < xopts_nr; i++)
argv_array_pushf(&args, "--%s", xopts[i]);
for (j = common; j; j = j->next)
diff --git a/sequencer.c b/sequencer.c
index 1b772fb..2fa1101 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -570,7 +570,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ res = try_merge_command(opts->strategy, 0, opts->xopts_nr, opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 16/18] git-merge-one-file.sh: support --index-only option
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (14 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 15/18] merge: Pass --index-only along to external merge strategy programs Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 17/18] git-merge-resolve.sh: " Elijah Newren
` (4 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
git-merge-one-file.sh | 44 +++++++++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 424b034..78efa00 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -18,16 +18,25 @@
USAGE='<orig blob> <our blob> <their blob> <path>'
USAGE="$USAGE <orig mode> <our mode> <their mode>"
-LONG_USAGE="usage: git merge-one-file $USAGE
+LONG_USAGE="usage: git merge-one-file $USAGE [--index-only]
Blob ids and modes should be empty for missing files."
+update_worktree=t
+expected_argcount=7
+if test "$8" = "--index-only"; then
+ update_worktree=
+ expected_argcount=8
+fi
+
SUBDIRECTORY_OK=Yes
. git-sh-setup
cd_to_toplevel
-require_work_tree
+if test -n "$update_worktree"; then
+ require_work_tree
+fi
-if test $# != 7
+if test $# != $expected_argcount
then
echo "$LONG_USAGE"
exit 1
@@ -59,8 +68,10 @@ case "${1:-.}${2:-.}${3:-.}" in
fi
if test -f "$4"
then
- rm -f -- "$4" &&
- rmdir -p "$(expr "z$4" : 'z\(.*\)/')" 2>/dev/null || :
+ if test -n "$update_worktree"; then
+ rm -f -- "$4" &&
+ rmdir -p "$(expr "z$4" : 'z\(.*\)/')" 2>/dev/null || :
+ fi
fi &&
exec git update-index --remove -- "$4"
;;
@@ -80,8 +91,11 @@ case "${1:-.}${2:-.}${3:-.}" in
echo "ERROR: untracked $4 is overwritten by the merge." >&2
exit 1
fi
- git update-index --add --cacheinfo "$7" "$3" "$4" &&
+ git update-index --add --cacheinfo "$7" "$3" "$4"
+ if test -n "$update_worktree"; then
exec git checkout-index -u -f -- "$4"
+ fi
+ exit 0
;;
#
@@ -95,8 +109,11 @@ case "${1:-.}${2:-.}${3:-.}" in
exit 1
fi
echo "Adding $4"
- git update-index --add --cacheinfo "$6" "$2" "$4" &&
+ git update-index --add --cacheinfo "$6" "$2" "$4"
+ if test -n "$update_worktree"; then
exec git checkout-index -u -f -- "$4"
+ fi
+ exit 0
;;
#
@@ -139,7 +156,11 @@ case "${1:-.}${2:-.}${3:-.}" in
# Create the working tree file, using "our tree" version from the
# index, and then store the result of the merge.
- git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1
+ if test -n "$update_worktree"; then
+ git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1
+ else
+ sha1=$(git hash-object -w -- "$src1")
+ fi
rm -f -- "$orig" "$src1" "$src2"
if test "$6" != "$7"
@@ -157,7 +178,12 @@ case "${1:-.}${2:-.}${3:-.}" in
echo "ERROR: $msg in $4" >&2
exit 1
fi
- exec git update-index -- "$4"
+ if test -n "$update_worktree"; then
+ exec git update-index -- "$4"
+ else
+ printf "$6 $sha1\t$4" | git update-index --index-info
+ exit $?
+ fi
;;
*)
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 17/18] git-merge-resolve.sh: support --index-only option
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (15 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 16/18] git-merge-one-file.sh: support --index-only option Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 6:58 ` [RFC/PATCH 18/18] git-merge-octopus.sh: " Elijah Newren
` (3 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
git-merge-resolve.sh | 12 ++++++++++--
t/t6043-merge-index-only.sh | 4 ++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh
index c9da747..ed4a25b 100755
--- a/git-merge-resolve.sh
+++ b/git-merge-resolve.sh
@@ -5,6 +5,14 @@
#
# Resolve two trees, using enhanced multi-base read-tree.
+read_tree_update="-u"
+index_only=
+if test "$1" = "--index-only"; then
+ read_tree_update="-i"
+ index_only="--index-only"
+ shift
+fi
+
# The first parameters up to -- are merge bases; the rest are heads.
bases= head= remotes= sep_seen=
for arg
@@ -38,14 +46,14 @@ then
fi
git update-index -q --refresh
-git read-tree -u -m --aggressive $bases $head $remotes || exit 2
+git read-tree $read_tree_update -m --aggressive $bases $head $remotes || exit 2
echo "Trying simple merge."
if result_tree=$(git write-tree 2>/dev/null)
then
exit 0
else
echo "Simple merge failed, trying Automatic merge."
- if git-merge-index -o git-merge-one-file -a
+ if git-merge-index -o git-merge-one-file $index_only -a
then
exit 0
else
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index cb860f2..c0a553b 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -299,7 +299,7 @@ test_expect_failure '--index-only w/ resolve, trivial, bare' '
)
'
-test_expect_failure '--index-only w/ resolve, non-trivial, non-bare' '
+test_expect_success '--index-only w/ resolve, non-trivial, non-bare' '
git reset --hard &&
git checkout B^0 &&
@@ -314,7 +314,7 @@ test_expect_failure '--index-only w/ resolve, non-trivial, non-bare' '
test ! -f d
'
-test_expect_failure '--index-only w/ resolve, non-trivial, bare' '
+test_expect_success '--index-only w/ resolve, non-trivial, bare' '
rm -rf bare.clone &&
git clone --bare . bare.clone &&
(cd bare.clone &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC/PATCH 18/18] git-merge-octopus.sh: support --index-only option
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (16 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 17/18] git-merge-resolve.sh: " Elijah Newren
@ 2016-04-08 6:58 ` Elijah Newren
2016-04-08 13:01 ` [RFC/PATCH 00/18] Add --index-only option to git merge Michael J Gruber
` (2 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-08 6:58 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com>
---
git-merge-octopus.sh | 14 +++++++++++---
t/t6043-merge-index-only.sh | 4 ++--
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index a1d7702..e7915dc 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -13,6 +13,14 @@ die () {
exit 1
}
+read_tree_update="-u"
+index_only=
+if test "$1" = "--index-only"; then
+ read_tree_update="-i"
+ index_only="--index-only"
+ shift
+fi
+
# The first parameters up to -- are merge bases; the rest are heads.
bases= head= remotes= sep_seen=
for arg
@@ -89,7 +97,7 @@ do
# We still need to count this as part of the parent set.
echo "Fast-forwarding to: $pretty_name"
- git read-tree -u -m $head $SHA1 || exit
+ git read-tree $read_tree_update -m $head $SHA1 || exit
MRC=$SHA1 MRT=$(git write-tree)
continue
fi
@@ -97,12 +105,12 @@ do
NON_FF_MERGE=1
echo "Trying simple merge with $pretty_name"
- git read-tree -u -m --aggressive $common $MRT $SHA1 || exit 2
+ git read-tree $read_tree_update -m --aggressive $common $MRT $SHA1 || exit 2
next=$(git write-tree 2>/dev/null)
if test $? -ne 0
then
echo "Simple merge did not work, trying automatic merge."
- git-merge-index -o git-merge-one-file -a ||
+ git-merge-index -o git-merge-one-file $index_only -a ||
OCTOPUS_FAILURE=1
next=$(git write-tree 2>/dev/null)
fi
diff --git a/t/t6043-merge-index-only.sh b/t/t6043-merge-index-only.sh
index c0a553b..080e03d 100755
--- a/t/t6043-merge-index-only.sh
+++ b/t/t6043-merge-index-only.sh
@@ -333,7 +333,7 @@ test_expect_success '--index-only w/ resolve, non-trivial, bare' '
)
'
-test_expect_failure '--index-only octopus, non-bare' '
+test_expect_success '--index-only octopus, non-bare' '
git reset --hard &&
git checkout B^0 &&
@@ -351,7 +351,7 @@ test_expect_failure '--index-only octopus, non-bare' '
test ! -f d
'
-test_expect_failure '--index-only octopus, bare' '
+test_expect_success '--index-only octopus, bare' '
rm -rf bare.clone &&
git clone --bare . bare.clone &&
(cd bare.clone &&
--
2.8.0.18.gc685494
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (17 preceding siblings ...)
2016-04-08 6:58 ` [RFC/PATCH 18/18] git-merge-octopus.sh: " Elijah Newren
@ 2016-04-08 13:01 ` Michael J Gruber
2016-04-09 3:09 ` Elijah Newren
2016-04-08 18:08 ` Junio C Hamano
2016-04-10 5:33 ` Elijah Newren
20 siblings, 1 reply; 30+ messages in thread
From: Michael J Gruber @ 2016-04-08 13:01 UTC (permalink / raw)
To: Elijah Newren, git
Elijah Newren venit, vidit, dixit 08.04.2016 08:58:
> This patch series adds an --index-only flag to git merge, the idea
> being to allow a merge to be performed entirely in the index without
> touching (or even needing) a working tree.
>
> The core fix, to merge-recursive, was actually quite easy. The
> recursive merge logic already had the ability to ignore the working
> directory and operate entirely on the index -- it needed to do this
> when creating a virtual merge base, i.e. when o->call_depth > 0. It's
> just that o->call_depth was also used for other purposes, so I just
> needed to introduce a new flag to disambiguate and switch all the
> necessary index-only-related call sites to use it. It actually seems
> to make the code slightly easier to read too, which is a nice bonus.
> That was all done in patch 12 of this series.
>
> Adding all the necessary testcases and switching over the other merge
> strategies turned out to be the harder part...and still has a problem,
> as I'll mention below.
I haven't looked at your series thoroughly but immediately had to think
of 'tr/remerge-diff' (on 'pu'), see
http://permalink.gmane.org/gmane.comp.version-control.git/256591
There, Thomas used index-only merge to reproduce an automatic merge as
the base for a useful "remerge-diff".
I've been rebasing (and using) that series on 'next' for a while now
without any problems; some reasons kept it from being merged on next,
see the thread.
So, it would be interesting whether you solve the same problem
differently, or face the same problems ;)
Michael
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-08 13:01 ` [RFC/PATCH 00/18] Add --index-only option to git merge Michael J Gruber
@ 2016-04-09 3:09 ` Elijah Newren
0 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-09 3:09 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
On Fri, Apr 8, 2016 at 6:01 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> I haven't looked at your series thoroughly but immediately had to think
> of 'tr/remerge-diff' (on 'pu'), see
> http://permalink.gmane.org/gmane.comp.version-control.git/256591
>
> There, Thomas used index-only merge to reproduce an automatic merge as
> the base for a useful "remerge-diff".
>
> I've been rebasing (and using) that series on 'next' for a while now
> without any problems; some reasons kept it from being merged on next,
> see the thread.
>
> So, it would be interesting whether you solve the same problem
> differently, or face the same problems ;)
Thanks for the link. Looks like a very interesting series, even if
we're solving slightly different problems (I don't want conflicts
auto-resolved; I want to be able to look at what conflicted and why
with commands like 'git ls-files -u' afterward.)
But the problem he's trying to solve is interesting to me too. We
have one patch from each of our series that does overlap, the
index-only modification to merge-recursive, though implemented
slightly differently. I think mine's a little clearer, and I have a
hunch that he might be able to use the idea in mine to dramatically
simplify some of the other stuff he's doing. In particular,
merge-recursive already has code to auto resolve conflicts that he
could just re-use instead of reimplementing, which I believe would
dramatically simplify his patch #8.
I didn't read all his code super closely, so I might be missing
something important, but I got the feeling that he didn't need
different behavior than what merge-recursive already implements for
virtual merge bases, and that even he wasn't certain whether he had
handled all cases (e.g. not only conflict markers and modify/delete,
but also rename/delete, rename/add, rename/rename (both 1to2 and
2to1), D/F conflicts, and perhaps others I'm not thinking of at the
moment). We already have well reviewed and tested code for all those
cases; it's just a subset of the things triggered by o->call_depth for
virtual merges bases. Also, like the index-only stuff triggered by
o->call_depth, the auto-resolve-conflict stuff is pretty well
separated so it should be easy to add a flag to trigger just this
portion without getting all the other stuff that o->call_depth
normally does.
As far as I could tell, his series stalled out both because of
concerns surrounding whether all the automatic-conflict-resolutions
were correct, and because it made git log no longer be a read-only
operation. Neither of those concerns are applicable to my patchset; I
invented totally new problems and concerns instead. :-) I don't have
a good solution to the second concern for his patchset, but I think
there's probably an easy solution to the first. Once I get my
patchset cleaned up, I may take a look at reviving his (if he doesn't
beat me to it.)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (18 preceding siblings ...)
2016-04-08 13:01 ` [RFC/PATCH 00/18] Add --index-only option to git merge Michael J Gruber
@ 2016-04-08 18:08 ` Junio C Hamano
2016-04-09 2:35 ` Elijah Newren
2016-04-10 5:33 ` Elijah Newren
20 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2016-04-08 18:08 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
Elijah Newren <newren@gmail.com> writes:
> This patch series adds an --index-only flag to git merge, the idea
> being to allow a merge to be performed entirely in the index without
> touching (or even needing) a working tree.
The goal is stated rather vaguely--when you have a working tree and
perform this "in index" merge, you would obviously update the index
with the merge result and write it out as a tree to record the merge,
and update HEAD to point at the new commit, but "without touching"
would imply that you would not be checking out the result to the
working tree. That sounds crazy and probably you didn't mean it
that way.
One worry immediately comes to mind is what should happen when
conflicts occur. A knee-jerk reaction only after reading the above
explanation is to error out; there could be other sensible
alternatives, but I do not think of any offhand.
Another thing that comes to mind is if that should be an option to
the end-user facing "git merge" command that can be run in a
repository _with_ a working tree. When used in a repository with a
working tree, I am not sure what the next step the end user wants to
make after the "only in index" merge succeeds. The index has been
updated with the merge result at that point and I am guessing that
the merge would also have created a new commit and updated the HEAD
to point at it. The contents of the working tree for paths that
were not changed when the user initiated the merge can be safely
checked out, but what should happen to the ones that were dirty wrt
the index (I am again guessing that you do not allow a "git merge
only in index" if the index already has changes wrt the HEAD)?
I can understand if the option only worked in a bare repository,
though.
> The core fix, to merge-recursive, was actually quite easy. The
> recursive merge logic already had the ability to ignore the working
> directory and operate entirely on the index -- it needed to do this
> when creating a virtual merge base, i.e. when o->call_depth > 0.
You need to be careful here, though. The creation of a virtual
parent accepts (actually it wants to have) conflicted blobs as if
that is a valid merge result--"git merge but only in index" probably
does not want that behaviour.
> A brief-ish summary of the series:
>
> * Patches 1 and 2 are unrelated cleanups, which could be submitted
> independently. However, submitting them as separate patches would
> result in a minor conflict, so I'm just including them together.
>
> * Patches 3 and 4 add some testcases and a fix for a separate issue
> found while testing this series which could be split off and
> submitted separately, but fixing this problem and enforcing the
> starting state I expected permitted me to reduce the range of
> testcases I needed to consider for the --index-only merges. So I
> thought it made sense to include in this series.
I'd prefer these as a separate series if they are truly unrelated
cleanups, so that we can quickly review and have them graduated
without waiting for the remainder. You can still submit the main
series with a comment that says it applies on that separate clean-up
series and the right things should happen ;-)
> In particular, I was worried about how git merge behaved when the
> index differed from HEAD prior to the merge starting. I discovered
> that it wasn't just allowed for fast-forward merges (where behavior
> is well-defined) but that it was also (accidentally I'm assuming)
> allowed for octopus merges with weird/surprising behavior.
I briefly looked at these tests and I agree with your expectation,
i.e. we want to allow a merge as long as there is no conflicts with
the change already in the index (i.e. the changed paths have the
same contents in ORIG_HEAD and HEAD).
> Some things I am concerned about:
>
> * The option name (--index-only) may be slightly misleading since the
> index isn't the only thing modified within the git directory, other
> normal things under there are still modified (e.g. writing MERGE_*
> files, sticking blobs/trees/commits in objects/*, and updating refs
> and reflogs). I personally prefer this name and think the confusion
> would be minor, but I'm a bit unsure and wanted some other opinions
> on this.
When you say "index only", I'd say people would understand that to
be saying "as opposed to using and updating both the index and the
working tree", so I do not think there is no confusion.
An established convention to spell "index only" found in "apply" and
"grep" is to say "--cached", though (cf. "git help cli").
> * I didn't add this option to the separate git-merge-recursive
> executable and make the worktree-optional modification to the git
> subcommands merge-recursive, merge-recursive-ours,
> merge-recursive-theirs, and merge-subtree in git.c. Should I, or
> are these separate binaries and subcommands just present for
> historical backward compatibility reasons with people expected to
> call e.g. "git merge -s recursive" these days?
If you have to assume, assume that there are people who use these
programs in their scripts and workflows, because it is a relatively
new development to make "git merge" directly calling into the
recursive machinery bypassing these commands.
> * Expanding on the last item, git-merge-one-file.sh is of particular
> concern; it seemed to strongly assume exactly seven arguments and
> that the position of each mattered. I didn't want to break that, so
> I added --index-only as an optional 8th argument, even though it
> seems slightly odd force an option argument to always come after
> positional ones (and it made the changes to merge_entry in
> merge-index.c slightly easier to implement). Does that seem okay?
I have a suspicion that this would become moot, as the conclusion
may become that "git merge" that works only in index does not make
sense unless you are in a bare repository---in which case, these
external merge drivers has to be given a temporary working tree
anyway, and they can keep writing their result out to what they
consider is the working tree (i.e. via GIT_WORK_TREE or something).
You read the result of them from that temporary working tree into
the index before cleaning the temporary working tree. That way,
you do not have to touch them at all, no? In fact, the temporary
working tree trick may be applicable even when you are working in a
repository with a tree working tree.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-08 18:08 ` Junio C Hamano
@ 2016-04-09 2:35 ` Elijah Newren
2016-04-09 4:44 ` Junio C Hamano
0 siblings, 1 reply; 30+ messages in thread
From: Elijah Newren @ 2016-04-09 2:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Hi,
On Fri, Apr 8, 2016 at 11:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> The goal is stated rather vaguely--when you have a working tree and
> perform this "in index" merge, you would obviously update the index
> with the merge result and ...
Yes, I think you hit the nail on the head with your suggestions
elsewhere to (1) limit this to bare repository only, and/or (2) do the
"git merge --into master en/topic"
I'll fix this.
>> The core fix, to merge-recursive, was actually quite easy. The
>> recursive merge logic already had the ability to ignore the working
>> directory and operate entirely on the index -- it needed to do this
>> when creating a virtual merge base, i.e. when o->call_depth > 0.
>
> You need to be careful here, though. The creation of a virtual
> parent accepts (actually it wants to have) conflicted blobs as if
> that is a valid merge result--"git merge but only in index" probably
> does not want that behaviour.
Yes, precisely. :-)
A close look at the merge-recursive code showed that the code to
handle the index-only behavior was already nicely separated from the
other things triggered by o->call_depth (e.g. the conflicted blobs
case you mention above, among others), making it really easy to modify
it so we could get the index-only behavior without the other stuff.
> I'd prefer these as a separate series if they are truly unrelated
> cleanups, so that we can quickly review and have them graduated
> without waiting for the remainder. You can still submit the main
> series with a comment that says it applies on that separate clean-up
> series and the right things should happen ;-)
Will do.
> When you say "index only", I'd say people would understand that to
> be saying "as opposed to using and updating both the index and the
> working tree", so I do not think there is no confusion.
>
> An established convention to spell "index only" found in "apply" and
> "grep" is to say "--cached", though (cf. "git help cli").
I'm assuming your double negative was unintentional, i.e. that you do
not think there is any confusion. Let me know if I misread.
The pointer is helpful, but I struggle a bit with the name '--cached'.
The past tense in that word suggests to me that it should be a
read-only operation on the cache (which works for grep), rather than a
write operation (such as for merge or apply). It could also be
misconstrued in merge's case to think that the index is one of the
things being merged (rather than erroring out if the index doesn't
match HEAD). I know apply already uses --cached, but if others don't
mind, I personally would rather not use it here. Is this something
you feel strongly about, or are you okay with --index-only?
> If you have to assume, assume that there are people who use these
> programs in their scripts and workflows, because it is a relatively
> new development to make "git merge" directly calling into the
> recursive machinery bypassing these commands.
So, my question wasn't so much about whether the git-merge-* programs
are used, as to whether they should support all the same options that
e.g. "git merge -s recursive" does. I'm not sure if having the old
merge-* programs support fewer options is considered good ("we want to
toss them eventually", or "they are less tested these days so we want
to take less risk with modifying them") or bad ("we don't want there
to be any difference in ability or behavior").
It should be easy to add the --index-only option to each of these, I'm
just unsure whether it matters or if it's wanted.
> I have a suspicion that this would become moot, as the conclusion
> may become that "git merge" that works only in index does not make
> sense unless you are in a bare repository---in which case, these
> external merge drivers has to be given a temporary working tree
> anyway, and they can keep writing their result out to what they
> consider is the working tree (i.e. via GIT_WORK_TREE or something).
> You read the result of them from that temporary working tree into
> the index before cleaning the temporary working tree. That way,
> you do not have to touch them at all, no? In fact, the temporary
> working tree trick may be applicable even when you are working in a
> repository with a tree working tree.
I'm a little confused; you seem to be suggesting git-merge-one-file
will always need to have a working tree of some sort, though I don't
see why.
git-merge-one-file doesn't currently read from the working tree,
except to check for untracked files in the way; it instead gets file
contents from git unpack-file, which pulls it from the object store.
git-merge-one-file currently records the merge resolution, if clean,
in both the index and the working copy, so my modifications are about
making it able to write to only the index. That's the only
modification I think it needs to avoid requiring a working tree at
all.
(git-merge-one-file does create some temporary files in the current
working directory via git unpack-file, but it doesn't need a full
working tree for that. I should also make it a little cleaner by
having it not check for the presence of untracked-and-in-the-way files
in the --index-only case.)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-09 2:35 ` Elijah Newren
@ 2016-04-09 4:44 ` Junio C Hamano
0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2016-04-09 4:44 UTC (permalink / raw)
To: Elijah Newren; +Cc: Git Mailing List
Elijah Newren <newren@gmail.com> writes:
> On Fri, Apr 8, 2016 at 11:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Elijah Newren <newren@gmail.com> writes:
>>
>> The goal is stated rather vaguely--when you have a working tree and
>> perform this "in index" merge, you would obviously update the index
>> with the merge result and ...
>
> Yes, I think you hit the nail on the head with your suggestions
> elsewhere to (1) limit this to bare repository only, and/or (2) do the
> "git merge --into master en/topic"
>
> I'll fix this.
> ...
> The pointer is helpful, but I struggle a bit with the name '--cached'.
> The past tense in that word suggests to me that it should be a
> read-only operation on the cache (which works for grep), rather than a
> write operation (such as for merge or apply). It could also be
> misconstrued in merge's case to think that the index is one of the
> things being merged (rather than erroring out if the index doesn't
> match HEAD). I know apply already uses --cached, but if others don't
> mind, I personally would rather not use it here. Is this something
> you feel strongly about, or are you okay with --index-only?
"cached" is not in past tense, but is used an adjective. You can
update what is in the index and that is to update the "cached"
information.
But I do not think we need to discuss this, because we do not need
such an option, as I understand that your updated direction is to do
one or both of (1) do the "merge and update HEAD only when no manual
conflict resolution is needed" thing in a bare repository, (2) "git
merge --into=master en/topic" does the "merge master and en/topic
and update master only when no manual conflict resolution is needed
and do so without touching HEAD, index or the working tree".
Because the current "git merge" refuses to work without the working
tree, (1) can be done without adding any new option---if you are run
in a bare repository, by definition, you are being asked to do that
new thing, and because you will not do the new thing in a repository
with a working tree, there is no need for an option to tell the
command, "no, don't do the usual thing but do this new thing". And
obviously (2) already has a new option --into and no other new option
is needed to tell the command that it needs to do that new thing.
>> I have a suspicion that this would become moot, as the conclusion
>> may become that "git merge" that works only in index does not make
>> sense unless you are in a bare repository---in which case, these
>> external merge drivers has to be given a temporary working tree
>> anyway, and they can keep writing their result out to what they
>> consider is the working tree (i.e. via GIT_WORK_TREE or something).
>> You read the result of them from that temporary working tree into
>> the index before cleaning the temporary working tree. That way,
>> you do not have to touch them at all, no? In fact, the temporary
>> working tree trick may be applicable even when you are working in a
>> repository with a tree working tree.
>
> I'm a little confused; you seem to be suggesting git-merge-one-file
> will always need to have a working tree of some sort, though I don't
> see why.
I was loosely referring to the working tree, which includes things
like having a place you can safely check out temporary files as if
they were one of the working tree files, i.e. the arrangement in
which "merge-one-file" is designed to operate. Even if they are
"tmpname" generated files, you do not want to clobber your $GIT_DIR
with these random files when driving these blob level merge drivers
in a bare repository.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC/PATCH 00/18] Add --index-only option to git merge
2016-04-08 6:58 [RFC/PATCH 00/18] Add --index-only option to git merge Elijah Newren
` (19 preceding siblings ...)
2016-04-08 18:08 ` Junio C Hamano
@ 2016-04-10 5:33 ` Elijah Newren
20 siblings, 0 replies; 30+ messages in thread
From: Elijah Newren @ 2016-04-10 5:33 UTC (permalink / raw)
To: Git Mailing List
On Thu, Apr 7, 2016 at 11:58 PM, Elijah Newren <newren@gmail.com> wrote:
> Luckily, I figured out that bug. So, that leaves just one case left
> that I can't seem to figure out: read_tree_trivial. So much better,
> right? Even it's name is sitting there, mocking me. "Ha ha, I'm
> read_tree_*trivial* and you can't figure me out." read_tree_trivial
> is a jerk.
It turns out this wasn't a bug in my index-only handling of the
trivial merge case; it was a pre-existing bug in git handling of
trivial merges that probably no one else had ever triggered before.
I'm submitting a fix with my other miscellaneous merge fixups...
^ permalink raw reply [flat|nested] 30+ messages in thread