git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH 04/18] merge-octopus: Abort if index not clean

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 8, 2016, 19:31 UTC
Message-ID
<xmqq4mbbx5go.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<1460098726-5958-5-git-send-email-newren@gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 15 quoted lines
> 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.

Show 57 quoted lines
>  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' '
Previous: Elijah NewrenNext: Elijah Newren
Message 7 of 30 in “Add --index-only option to git merge”
  1. 00/18 Add --index-only option to git mergeElijah Newren, Apr 8, 2016
  2. 01/18 Remove duplicate codeElijah Newren, Apr 8, 2016
  3. Junio C HamanoApr 8, 2016
  4. 02/18 Avoid checking working copy when creating a virtual merge baseElijah Newren, Apr 8, 2016
  5. 03/18 Document weird bug in octopus merges via testcasesElijah Newren, Apr 8, 2016
  6. 04/18 merge-octopus: Abort if index not cleanElijah Newren, Apr 8, 2016
  7. Junio C HamanoApr 8, 2016
  8. 05/18 Add testcase for --index-only merges needing the recursive strategyElijah Newren, Apr 8, 2016
  9. Junio C HamanoApr 8, 2016
  10. Junio C HamanoApr 8, 2016
  11. 06/18 Add testcase for --index-only merges needing an ff updateElijah Newren, Apr 8, 2016
  12. 07/18 Add testcase for --index-only merges with the resolve strategyElijah Newren, Apr 8, 2016
  13. 08/18 Add testcase for --index-only merges with the octopus strategyElijah Newren, Apr 8, 2016
  14. 09/18 Add testcase for --index-only merges with the ours strategyElijah Newren, Apr 8, 2016
  15. 10/18 Add testcase for --index-only merges with the subtree strategyElijah Newren, Apr 8, 2016
  16. 11/18 merge: Add a new --index-only option, not yet implementedElijah Newren, Apr 8, 2016
  17. Junio C HamanoApr 8, 2016
  18. 12/18 Add --index-only support for recursive mergesElijah Newren, Apr 8, 2016
  19. 13/18 Add --index-only support with read_tree_trivial merges, kind ofElijah Newren, Apr 8, 2016
  20. 14/18 Add --index-only support for ff_only mergesElijah Newren, Apr 8, 2016
  21. 15/18 merge: Pass --index-only along to external merge strategy programsElijah Newren, Apr 8, 2016
  22. 16/18 git-merge-one-file.sh: support --index-only optionElijah Newren, Apr 8, 2016
  23. 17/18 git-merge-resolve.sh: support --index-only optionElijah Newren, Apr 8, 2016
  24. 18/18 git-merge-octopus.sh: support --index-only optionElijah Newren, Apr 8, 2016
  25. Michael J GruberApr 8, 2016
  26. Elijah NewrenApr 9, 2016
  27. Junio C HamanoApr 8, 2016
  28. Elijah NewrenApr 9, 2016
  29. Junio C HamanoApr 9, 2016
  30. Elijah NewrenApr 10, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.