Commit Graph

8 Commits

Author SHA1 Message Date
Elijah Newren
cf69f2af08 t6044: add a testcase for index matching head, when head doesn't match HEAD
The `git merge-recursive` command allows the user to directly specify
three commits to merge -- base, head, and remote.  (More than three can be
specified in the case of multiple merge bases.)  Note that since the user
is allowed to specify head, it need not match HEAD.

Virtually every test and script in the current git.git codebase calls `git
merge-recursive` with head=HEAD, and likely external callers do as well,
which is why this has gone unnoticed.  There is one notable
counter-example: git-stash.sh.  However, git-stash called `git
merge-recursive` with an index that matches the expected merge result,
which happens to be a currently allowed exception to the "index must match
head" rule, so this never triggered an error previously.

Since we would like to tighten up the "index must match head" rule, we
need to make sure we are comparing to the correct head.  Add a testcase
that demonstrates the failure when we check the wrong HEAD.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-11 09:38:36 -07:00
Elijah Newren
58f4d1b961 t6044: verify that merges expected to abort actually abort
t6044 has lots of tests for verifying that merge will abort as expected
when there are changes staged before the merge starts.  However, it only
checked for non-zero exit code, which could mean that the merge ran to
completion with conflicts.  Check that the merge was actually correctly
aborted, i.e. that .git/MERGE_HEAD is not present.

This changes one of the tests from expect_success to expect_failure.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03 13:13:18 -07:00
Elijah Newren
65170c07d4 merge-recursive: avoid incorporating uncommitted changes in a merge
builtin/merge.c contains this important requirement for merge strategies:
	/*
	 * At this point, we need a real merge.  No matter what strategy
	 * we use, it would operate on the index, possibly affecting the
	 * working tree, and when resolved cleanly, have the desired
	 * tree in the index -- this means that the index must be in
	 * sync with the head commit.  The strategies are responsible
	 * to ensure this.
	 */

merge-recursive does not do this check directly, instead it relies on
unpack_trees() to do it.  However, merge_trees() has a special check for
the merge branch exactly matching the merge base; when it detects that
situation, it returns early without calling unpack_trees(), because it
knows that the HEAD commit already has the correct result.  Unfortunately,
it didn't check that the index matched HEAD, so after it returned, the
outer logic ended up creating a merge commit that included something
other than HEAD.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22 12:20:38 -08:00
Elijah Newren
eab3f2850e t6044: recursive can silently incorporate dirty changes in a merge
The recursive merge strategy has some special handling when the tree for
the merge branch exactly matches the merge base, but that code path is
missing checks for the index having changes relative to HEAD.  Add a
testcase covering this scenario.

Reported-by: Andreas Krey <a.krey@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22 12:14:16 -08:00
Junio C Hamano
10184b2718 Merge branch 'js/t6044-use-test-seq'
Test portability fix.

* js/t6044-use-test-seq:
  t6044: replace seq by test_seq
2016-05-29 18:06:43 -07:00
Johannes Sixt
3f215b0328 t6044: replace seq by test_seq
seq is not available everywhere.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-18 08:48:06 -07:00
Elijah Newren
a6ee883b8e t6044: new merge testcases for when index doesn't match HEAD
With one exception, we require the index to exactly match the
current HEAD commit at the time git merge is invoked.  This
expectation was even documented in git-merge.txt until commit
ebef7e5 (Documentation: simplify How Merge Works, 2010-01-23).

Most merge strategies enforced this requirement, but it turns out
not all did.  The current exceptions were the following two:

  * 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.  This is the one exception we
expected to the abort-merge-if- index-doesn't-match-HEAD rule.

For octopus merges, the rule should be enforced.  Unfortunately, the
current behavior of the code is to ignore the difference and use the
staged changes in place of whatever is in HEAD as it proceeds to
perform the merge.  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.

Add testcases that check our expectations.  A subsequent commit will
correct the erroneous octopus merge behavior.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-12 18:39:43 -07:00
Elijah Newren
3ec62ad9ff merge-octopus: abort if index does not match HEAD
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-12 18:39:43 -07:00