Short "git commit $file" syntax fails in the face of a resolved conflict

Short "git commit $file" syntax fails in the face of a resolved conflict

[Asheesh Laroia]

I have found what seems to be a bug in the short "git commit $file" mode 
of interaction with git. To reproduce it, you can:

1. Create a repository with some content.

 	$ (mkdir a ; cd a ; git init ; echo hi > file ; git add file ; git commit -m 'initial commit')
 	Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/a/.git/
 	Created initial commit 276d6eb: initial commit
 	 1 files changed, 1 insertions(+), 0 deletions(-)
 	 create mode 100644 file

2. Clone that repository.

 	$ git clone a b
 	Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/b/.git/

3. Create changes in "a" that are not yet cloned into "b".

 	$ (cd a ; echo ho > file ; git add file ; git commit -m update)
 	Created commit 91deff9: update
 	 1 files changed, 1 insertions(+), 1 deletions(-)

4. Make changes in "b", the clone.

 	$ echo lol > file
 	$ git add file ; git commit -m 'Some changes'
 	Created commit 5d74b5b: Some changes
 	 1 files changed, 1 insertions(+), 1 deletions(-)

5. Fetch and merge (AKA pull) from the first repo.

 	$ git pull
 	remote: Counting objects: 5, done.
 	remote: Total 3 (delta 0), reused 0 (delta 0)
 	Unpacking objects: 100% (3/3), done.
 	From /tmp/playground.2009-01-21.w15613/a/
 	   276d6eb..91deff9  master     -> origin/master
 	Auto-merged file
 	CONFLICT (content): Merge conflict in file
 	Automatic merge failed; fix conflicts and then commit the result.

6. Resolve the conflict (in our case, by discarding the changes in the "b" 
clone).

 	$ echo ho > file

7. Commit the resolved conflict.

NOTE: The normal way to do step 6 is to "git add file ; git commit -m 
yay". But I will now try to use the "git commit file" shorthand:

 	$ git commit file -m 'Resolved conflict'
 	fatal: cannot do a partial commit during a merge.

8. Declare a bug.

I believe that the "git commit file" command issued in step 6 should have 
worked as well as the "git add file ; git commit" that us old-time git 
users do.

9. Discuss on the git list.

Do y'all agree that the git behavior is strange and unnecessarily 
user-impeding here?

Cheers!

-- Asheesh.

P.S. I'm not the one who ran into the bad behavior here; Nathan (CC:d) is 
the one who did. You don't have to keep him CC:d, though.

-- 
Avoid gunfire in the bathroom tonight.

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Michael J Gruber]

Asheesh Laroia venit, vidit, dixit 01/21/09 22:00:
> I have found what seems to be a bug in the short "git commit $file" mode 
> of interaction with git. To reproduce it, you can:
> 
> 1. Create a repository with some content.
> 
>  	$ (mkdir a ; cd a ; git init ; echo hi > file ; git add file ; git commit -m 'initial commit')
>  	Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/a/.git/
>  	Created initial commit 276d6eb: initial commit
>  	 1 files changed, 1 insertions(+), 0 deletions(-)
>  	 create mode 100644 file
> 
> 2. Clone that repository.
> 
>  	$ git clone a b
>  	Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/b/.git/
> 
> 3. Create changes in "a" that are not yet cloned into "b".
> 
>  	$ (cd a ; echo ho > file ; git add file ; git commit -m update)
>  	Created commit 91deff9: update
>  	 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 4. Make changes in "b", the clone.
> 
>  	$ echo lol > file
>  	$ git add file ; git commit -m 'Some changes'
>  	Created commit 5d74b5b: Some changes
>  	 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 5. Fetch and merge (AKA pull) from the first repo.
> 
>  	$ git pull
>  	remote: Counting objects: 5, done.
>  	remote: Total 3 (delta 0), reused 0 (delta 0)
>  	Unpacking objects: 100% (3/3), done.
>  	From /tmp/playground.2009-01-21.w15613/a/
>  	   276d6eb..91deff9  master     -> origin/master
>  	Auto-merged file
>  	CONFLICT (content): Merge conflict in file
>  	Automatic merge failed; fix conflicts and then commit the result.
> 
> 6. Resolve the conflict (in our case, by discarding the changes in the "b" 
> clone).
> 
>  	$ echo ho > file
> 
> 7. Commit the resolved conflict.
> 
> NOTE: The normal way to do step 6 is to "git add file ; git commit -m 
> yay". But I will now try to use the "git commit file" shorthand:
> 
>  	$ git commit file -m 'Resolved conflict'
>  	fatal: cannot do a partial commit during a merge.
> 
> 8. Declare a bug.
> 
> I believe that the "git commit file" command issued in step 6 should have 
> worked as well as the "git add file ; git commit" that us old-time git 
> users do.
> 
> 9. Discuss on the git list.
> 
> Do y'all agree that the git behavior is strange and unnecessarily 
> user-impeding here?
> 
> Cheers!
> 
> -- Asheesh.
> 
> P.S. I'm not the one who ran into the bad behavior here; Nathan (CC:d) is 
> the one who did. You don't have to keep him CC:d, though.
> 

You want git commit -i:

       -i, --include
           Before making a commit out of staged contents so far, stage
the contents of paths given on the command line as well.
           This is usually not what you want unless you are concluding a
conflicted merge.

Without -i, git commit path ignores the index, which would be bad in the
middle of a merge, which is why git refuses to do so. You may argue for
git commit to use -i automatically here, but I don't think it's a good idea.

So, out of
1) git add path && git commit
2) git commit path
3) git commit -i path
only 1) and 3) are always equivalent.

Michael

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Nathan Yergler]

Can you elaborate on why doing -i automatically is a bad idea in this
case?  [It may really be, I don't pretend to have enough knowledge
about git's internals to make a reasoned argument.]  This was
unexpected behavior for me as I'd always experienced "git add path &&
git commit" and "git commit path" as being equivalent and so I assumed
they would work equivalently in this situation.

Nathan

On Wed, Jan 21, 2009 at 1:35 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Asheesh Laroia venit, vidit, dixit 01/21/09 22:00:
>> I have found what seems to be a bug in the short "git commit $file" mode
>> of interaction with git. To reproduce it, you can:
>>
>> 1. Create a repository with some content.
>>
>>       $ (mkdir a ; cd a ; git init ; echo hi > file ; git add file ; git commit -m 'initial commit')
>>       Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/a/.git/
>>       Created initial commit 276d6eb: initial commit
>>        1 files changed, 1 insertions(+), 0 deletions(-)
>>        create mode 100644 file
>>
>> 2. Clone that repository.
>>
>>       $ git clone a b
>>       Initialized empty Git repository in /tmp/playground.2009-01-21.w15613/b/.git/
>>
>> 3. Create changes in "a" that are not yet cloned into "b".
>>
>>       $ (cd a ; echo ho > file ; git add file ; git commit -m update)
>>       Created commit 91deff9: update
>>        1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> 4. Make changes in "b", the clone.
>>
>>       $ echo lol > file
>>       $ git add file ; git commit -m 'Some changes'
>>       Created commit 5d74b5b: Some changes
>>        1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> 5. Fetch and merge (AKA pull) from the first repo.
>>
>>       $ git pull
>>       remote: Counting objects: 5, done.
>>       remote: Total 3 (delta 0), reused 0 (delta 0)
>>       Unpacking objects: 100% (3/3), done.
>>       From /tmp/playground.2009-01-21.w15613/a/
>>          276d6eb..91deff9  master     -> origin/master
>>       Auto-merged file
>>       CONFLICT (content): Merge conflict in file
>>       Automatic merge failed; fix conflicts and then commit the result.
>>
>> 6. Resolve the conflict (in our case, by discarding the changes in the "b"
>> clone).
>>
>>       $ echo ho > file
>>
>> 7. Commit the resolved conflict.
>>
>> NOTE: The normal way to do step 6 is to "git add file ; git commit -m
>> yay". But I will now try to use the "git commit file" shorthand:
>>
>>       $ git commit file -m 'Resolved conflict'
>>       fatal: cannot do a partial commit during a merge.
>>
>> 8. Declare a bug.
>>
>> I believe that the "git commit file" command issued in step 6 should have
>> worked as well as the "git add file ; git commit" that us old-time git
>> users do.
>>
>> 9. Discuss on the git list.
>>
>> Do y'all agree that the git behavior is strange and unnecessarily
>> user-impeding here?
>>
>> Cheers!
>>
>> -- Asheesh.
>>
>> P.S. I'm not the one who ran into the bad behavior here; Nathan (CC:d) is
>> the one who did. You don't have to keep him CC:d, though.
>>
>
> You want git commit -i:
>
>       -i, --include
>           Before making a commit out of staged contents so far, stage
> the contents of paths given on the command line as well.
>           This is usually not what you want unless you are concluding a
> conflicted merge.
>
> Without -i, git commit path ignores the index, which would be bad in the
> middle of a merge, which is why git refuses to do so. You may argue for
> git commit to use -i automatically here, but I don't think it's a good idea.
>
> So, out of
> 1) git add path && git commit
> 2) git commit path
> 3) git commit -i path
> only 1) and 3) are always equivalent.
>
> Michael
>

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Johannes Sixt]

Please don't top-post.

Nathan Yergler schrieb:
> Can you elaborate on why doing -i automatically is a bad idea in this
> case?  [It may really be, I don't pretend to have enough knowledge
> about git's internals to make a reasoned argument.]  This was
> unexpected behavior for me as I'd always experienced "git add path &&
> git commit" and "git commit path" as being equivalent and so I assumed
> they would work equivalently in this situation.

They are not equivalent. 'git add path && git commit' commits changes to
path *in addition* to what is already staged before you run this command
sequence. But 'git commit path' commits *only* changes to path, leaving
other changes that might be staged uncommitted.

It may become obvious why the latter behavior is unwanted if a merge is in
progress: The merge left changes (and conflicts) in the index; but with
'git commit path' you say that you are not interested in what the index has.

-- Hannes

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Michael J Gruber]

Nathan Yergler venit, vidit, dixit 21.01.2009 22:46:
> Can you elaborate on why doing -i automatically is a bad idea in this
> case?  [It may really be, I don't pretend to have enough knowledge
> about git's internals to make a reasoned argument.]  This was
> unexpected behavior for me as I'd always experienced "git add path &&
> git commit" and "git commit path" as being equivalent and so I assumed
> they would work equivalently in this situation.

Because it makes it hard to follow the discussion.

Why shouldn't I?

Fist of all: Please don't top post.

;)

That being said: As Johannes 6t explained (in agreement with git help
commit), "git commit path" - which is synonymous with "git commit -o
path" is a way of bypassing the index. Think of "Oh wait, I wanted to
commit that before I commit what I'm preparing right now.". Now,
bypassing the index is no big deal, but bypassing a merge in progress
is, because a merge in progress leaves more traces than just the index
state (e.g. MERGE_HEAD). That's also why this use case is mentioned
explicitly in the man page... In fact, rereading that man page (and
testing things to be on the safe side) I have to correct myself: Out of

1) git add path && git commit
2) git commit path
3) git commit -i path

none are equivalent! 1) and 3) are equivalent if and only if "path" is
known to git already: git commit -i does not add new paths.
2) and 3) are equivalent if and only if the index is empty (no changes
staged). The question "When are 1) and 2)" equivalent is left as an
exercise in elementary logic. ;)

Cheers,
Michael

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Nanako Shiraishi]

Quoting Johannes Sixt <j.sixt@viscovery.net>:

> Please don't top-post.
>
> Nathan Yergler schrieb:
>> Can you elaborate on why doing -i automatically is a bad idea in this
>> case?  [It may really be, I don't pretend to have enough knowledge
>> about git's internals to make a reasoned argument.]  This was
>> unexpected behavior for me as I'd always experienced "git add path &&
>> git commit" and "git commit path" as being equivalent and so I assumed
>> they would work equivalently in this situation.
>
> They are not equivalent. 'git add path && git commit' commits changes to
> path *in addition* to what is already staged before you run this command
> sequence. But 'git commit path' commits *only* changes to path, leaving
> other changes that might be staged uncommitted.
>
> It may become obvious why the latter behavior is unwanted if a merge is in
> progress: The merge left changes (and conflicts) in the index; but with
> 'git commit path' you say that you are not interested in what the index has.

Your explanation is a good answer to Nathan's misunderstanding; "git add path && git commit" and "git commit path" are different.

But Nathan's first sentence is a different matter. I do not think it is coming from the same confusion, and I think the question is a valid one. Your answer does not explain why it is a bad idea to change the behavior of "git commit path" to what "git commit -i path" does during a merge.

The answer of course can be "because it changes the behavior people are very much used to."

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Asheesh Laroia]

On Fri, 23 Jan 2009, Nanako Shiraishi wrote:

> Your explanation is a good answer to Nathan's misunderstanding; "git add 
> path && git commit" and "git commit path" are different.
>
> But Nathan's first sentence is a different matter.

Thanks for seeing this!

> I do not think it is coming from the same confusion, and I think the 
> question is a valid one. Your answer does not explain why it is a bad 
> idea to change the behavior of "git commit path" to what "git commit -i 
> path" does during a merge.

During a merge where the file called "file" is in conflict, I don't see 
why the internal mechanism of how a merge gets resolved is important to 
users like Nathan.

Sure, the index is nice, but let's look at the choices here.  When he runs
$ git commit file -m 'fixed conflict'
git can do one of two things:

(a) Fail with an obscure (or less obscure) error message, or
(b) Succeed.

The way in which it can suceed is unambiguous. Now, in the case of more 
than one file being in conflict, it makes sense to abort; success isn't 
possible. But in this case, no one really benefits from the user having to 
type something else to have the command actually succeed.

Those are my thoughts.

> The answer of course can be "because it changes the behavior people are 
> very much used to."

I don't think anyone is "very much used to" this error message, or that 
making something succeed in the only possible way is going to confuse 
anyone. If you're worried about confusing people, git could print a note 
like:

 	$ git commit file -m "Fixed conflict"
 	NOTE: Merge was in progress. If you have more than one file in conflict
 	in a future merge, be sure to "git add" each file separately and then
 	commit them all at once.
 	Created commit 12ede36: Fixed conflict
 	 0 files changed, 0 insertions(+), 0 deletions(-)
 	 create mode 100644 file
 	$

-- Asheesh.

-- 
In the Spring, I have counted 136 different kinds of weather inside of
24 hours.
 		-- Mark Twain, on New England weather

Re: Short "git commit $file" syntax fails in the face of a resolved conflict

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> But Nathan's first sentence is a different matter. I do not think it is
> coming from the same confusion, and I think the question is a valid
> one. Your answer does not explain why it is a bad idea to change the
> behavior of "git commit path" to what "git commit -i path" does during a
> merge.
>
> The answer of course can be "because it changes the behavior people are
> very much used to."

[jc: how many times do I have to ask you to wrap your lines, by the way?]

I tend to agree that "very much used to" argument carries much weight, but
I'll be sending a three-patch series to weatherbaloon the idea.

[PATCH 1/3] Add "partial commit" tests during a conflicted merge

[Junio C Hamano]

We are supposed to reject "--only path..." aka "a partial commit" during a
conflicted merge resolution, and accept "--include path..." aka "an also
commit" in such a case.

Recent git (since v1.3.0) always assumes that "git commit" with paths but
without --only nor --include requests the "--only" semantics, but there is
a discussion that it might be a good idea to assume "--include" semantics
during a merge.

The last test this commit adds expects such a behaviour and marked as
"expect_failure".  It will be changed by the third patch in the series.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7501-commit.sh |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b4e2b4d..0c10105 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -365,4 +365,48 @@ test_expect_success 'amend using the message from a commit named with tag' '
 
 '
 
+test_expect_success 'setup merge commit with paths test' '
+	git reset --hard &&
+	git checkout HEAD^0 &&
+	echo frotz >file &&
+	test_tick &&
+	git add file &&
+	git commit -a -m "one side says frotz" &&
+	git tag one-side-says-frotz &&
+	git reset --hard HEAD^ &&
+	echo nitfol >file &&
+	test_tick &&
+	git add file &&
+	git commit -a -m "the other side says nitfol" &&
+	git tag the-other-side-says-nitfol
+'
+
+test_expect_success 'reject --only during a merge' '
+	git checkout HEAD^0 &&
+	git reset --hard the-other-side-says-nitfol &&
+	test_must_fail git merge one-side-says-frotz &&
+	echo yomin-only >file &&
+	test_must_fail git commit -m merge --only file &&
+	git reset --hard
+'
+
+test_expect_success 'allow --include during a merge' '
+	git checkout HEAD^0 &&
+	git reset --hard the-other-side-says-nitfol &&
+	test_must_fail git merge one-side-says-frotz &&
+	echo yomin-include >file &&
+	git commit -m merge --include file &&
+	git reset --hard
+'
+
+test_expect_failure 'assume --include during a merge' '
+	git checkout HEAD^0 &&
+	git reset --hard the-other-side-says-nitfol &&
+	test_must_fail git merge one-side-says-frotz &&
+	echo yomin-assumed >file &&
+	git add file &&
+	git commit -m merge file &&
+	git reset --hard
+'
+
 test_done
-- 
1.6.1.265.g9a013

[PATCH 2/3] builtin-commit: shorten eye-sore overlong lines

[Junio C Hamano]

This does not change anything other than the way the variable to hold
an informative message thrown in the commit log buffer is assigned.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This does not really belong to the series in the sense that it is
   needed to implement the new semantics, but these long lines have always
   bothered me.

 builtin-commit.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 7aaa530..d861263 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -71,6 +71,29 @@ static int use_editor = 1, initial_commit, in_merge;
 static const char *only_include_assumed;
 static struct strbuf message;
 
+enum {
+	MSG_AMEND_CLEVER,
+	MSG_ASSUME_PARTIAL,
+};
+
+static void set_partial_commit_message(int msgnum)
+{
+	const char *msg;
+
+	switch (msgnum) {
+	case MSG_AMEND_CLEVER:
+		msg = "Clever... amending the last one with dirty index.";
+		break;
+	case MSG_ASSUME_PARTIAL:
+		msg = "Explicit paths specified without -i nor -o; assuming --only paths...";
+		break;
+	default:
+		die("Oops (%d) is not a valid message number", msgnum);
+		break;
+	}
+	only_include_assumed = msg;
+}
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
 	struct strbuf *buf = opt->value;
@@ -788,9 +811,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (argc == 0 && (also || (only && !amend)))
 		die("No paths with --include/--only does not make sense.");
 	if (argc == 0 && only && amend)
-		only_include_assumed = "Clever... amending the last one with dirty index.";
+		set_partial_commit_message(MSG_AMEND_CLEVER);
 	if (argc > 0 && !also && !only)
-		only_include_assumed = "Explicit paths specified without -i nor -o; assuming --only paths...";
+		set_partial_commit_message(MSG_ASSUME_PARTIAL);
 	if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
 		cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
 	else if (!strcmp(cleanup_arg, "verbatim"))
-- 
1.6.1.265.g9a013

[PATCH 3/3] git commit: pathspec without -i/-o implies -i semantics during a merge

[Junio C Hamano]

"git commit paths..." has been a short-hand for "git commit -o paths..."
since v1.3.0 and let you create a commit skipping any other updated state
staged in the index.  The "--only" semantics (aka "partial commit") is
always wrong during a conflicted merge resolution, and we rejected both
"git commit paths..."  and "git commit -o paths..."  forms during a merge.

On the other hand, "git commit -i paths..." (aka "an also commit", which
asks to commit what you staged in the index, and also the paths you may or
may not have git-add'ed) is accepted, as it is a way to register the
contents you fixed up to the index and commit the result.

This makes "git commit paths..." form default to "git commit -i paths"
semantics only during a merge, restoring the pre-v1.3.0 behaviour.  The
codepath to create a non-merge commit is not affected and still defaults
to the "--only" semantics.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-commit.c  |   14 ++++++++++++--
 t/t7501-commit.sh |    2 +-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index d861263..4cb1985 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -74,6 +74,7 @@ static struct strbuf message;
 enum {
 	MSG_AMEND_CLEVER,
 	MSG_ASSUME_PARTIAL,
+	MSG_ASSUME_ALSO_DURING_MERGE
 };
 
 static void set_partial_commit_message(int msgnum)
@@ -87,6 +88,9 @@ static void set_partial_commit_message(int msgnum)
 	case MSG_ASSUME_PARTIAL:
 		msg = "Explicit paths specified without -i nor -o; assuming --only paths...";
 		break;
+	case MSG_ASSUME_ALSO_DURING_MERGE:
+		msg = "Paths specified without -i nor -o during a merge; assuming -i";
+		break;
 	default:
 		die("Oops (%d) is not a valid message number", msgnum);
 		break;
@@ -812,8 +816,14 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		die("No paths with --include/--only does not make sense.");
 	if (argc == 0 && only && amend)
 		set_partial_commit_message(MSG_AMEND_CLEVER);
-	if (argc > 0 && !also && !only)
-		set_partial_commit_message(MSG_ASSUME_PARTIAL);
+	if (argc > 0 && !also && !only) {
+		if (!in_merge)
+			set_partial_commit_message(MSG_ASSUME_PARTIAL);
+		else {
+			set_partial_commit_message(MSG_ASSUME_ALSO_DURING_MERGE);
+			also = 1;
+		}
+	}
 	if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
 		cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
 	else if (!strcmp(cleanup_arg, "verbatim"))
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 0c10105..68892de 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -399,7 +399,7 @@ test_expect_success 'allow --include during a merge' '
 	git reset --hard
 '
 
-test_expect_failure 'assume --include during a merge' '
+test_expect_success 'assume --include during a merge' '
 	git checkout HEAD^0 &&
 	git reset --hard the-other-side-says-nitfol &&
 	test_must_fail git merge one-side-says-frotz &&
-- 
1.6.1.265.g9a013

Re: [PATCH 1/3] Add "partial commit" tests during a conflicted merge

[Johannes Sixt]

Junio C Hamano schrieb:
> +test_expect_success 'setup merge commit with paths test' '
> +	git reset --hard &&
> +	git checkout HEAD^0 &&
> +	echo frotz >file &&
> +	test_tick &&
> +	git add file &&
> +	git commit -a -m "one side says frotz" &&
> +	git tag one-side-says-frotz &&
> +	git reset --hard HEAD^ &&
> +	echo nitfol >file &&
> +	test_tick &&
> +	git add file &&
> +	git commit -a -m "the other side says nitfol" &&
> +	git tag the-other-side-says-nitfol
> +'
> +
> +test_expect_success 'reject --only during a merge' '
> +	git checkout HEAD^0 &&
> +	git reset --hard the-other-side-says-nitfol &&
> +	test_must_fail git merge one-side-says-frotz &&
> +	echo yomin-only >file &&
> +	test_must_fail git commit -m merge --only file &&

I don't see why this must fail: 'file' is the only file that is different
from HEAD. Yes, currently we fail; but if something is about to be
changed, then this can change as well.

> +	git reset --hard
> +'
> +
> +test_expect_success 'allow --include during a merge' '
> +	git checkout HEAD^0 &&
> +	git reset --hard the-other-side-says-nitfol &&
> +	test_must_fail git merge one-side-says-frotz &&
> +	echo yomin-include >file &&
> +	git commit -m merge --include file &&
> +	git reset --hard
> +'
> +
> +test_expect_failure 'assume --include during a merge' '
> +	git checkout HEAD^0 &&
> +	git reset --hard the-other-side-says-nitfol &&
> +	test_must_fail git merge one-side-says-frotz &&
> +	echo yomin-assumed >file &&
> +	git add file &&
> +	git commit -m merge file &&
> +	git reset --hard
> +'

If I read the test case correctly, there is only 'file' that is different
from HEAD, and it had a conflict. But IMO, the test should stress the
point that after the conflicted merge there are at least two files that
are different from HEAD, one was trivially merged, and the other had a
conflict.

-- Hannes

Re: [PATCH 1/3] Add "partial commit" tests during a conflicted merge

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

>> +test_expect_success 'reject --only during a merge' '
>> +	git checkout HEAD^0 &&
>> +	git reset --hard the-other-side-says-nitfol &&
>> +	test_must_fail git merge one-side-says-frotz &&
>> +	echo yomin-only >file &&
>> +	test_must_fail git commit -m merge --only file &&
>
> I don't see why this must fail: 'file' is the only file that is different
> from HEAD. Yes, currently we fail; but if something is about to be
> changed, then this can change as well.

Not at all.

Avoiding --only is to prevent a much more dangerous glitch.

Suppose you and the other have two paths diverged, and one merges cleanly
and the other results in conflict.  When "git merge" gives control back to
you, the cleanly merged result is ALREADY IN THE INDEX.

Now you futz with the other path, and say

	git commit --only other

What --only tells git is "I do not care what I've staged in the index.
Start from the contents of HEAD commit, and update the index entry at these
paths (and these path _ONLY_), and commit the contents registered in the
index.

That is why --include is the only sane semantics during a conflicted
merge.  I thought you should know better, as you were the one who gave the
explanation to Nathan, which triggered Nana's response, which resulted in
this series.

Re: [PATCH 1/3] Add "partial commit" tests during a conflicted merge

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>>> +test_expect_success 'reject --only during a merge' '
>>> +	git checkout HEAD^0 &&
>>> +	git reset --hard the-other-side-says-nitfol &&
>>> +	test_must_fail git merge one-side-says-frotz &&
>>> +	echo yomin-only >file &&
>>> +	test_must_fail git commit -m merge --only file &&
>> I don't see why this must fail: 'file' is the only file that is different
>> from HEAD. Yes, currently we fail; but if something is about to be
>> changed, then this can change as well.
> 
> Not at all.

Read again what I said: 'file' is the *ONLY* file that is different from
HEAD. Why should an explicit --only not work in this case?

> Avoiding --only is to prevent a much more dangerous glitch.
[...]

We are in total agreement about what you said in the rest of the message.

I'm proposing that, during a merge, if --only was given (or remains the
implicit choice), then we compare the index with HEAD, and if nothing
outside the given pathspec differs from HEAD, then allow the commit.

-- Hannes

Re: [PATCH 1/3] Add "partial commit" tests during a conflicted merge

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Read again what I said: 'file' is the *ONLY* file that is different from
> HEAD. Why should an explicit --only not work in this case?

I know what you said.

If you study the codepath, the code does not know nor care if 'file' is
the only one or if there are other changed paths.

Too much additional code is needed and for too little gain.

Re: [PATCH 3/3] git commit: pathspec without -i/-o implies -i semantics during a merge

[Pieter de Bie]

On 23 jan 2009, at 06:21, Junio C Hamano wrote:

> This makes "git commit paths..." form default to "git commit -i paths"
> semantics only during a merge, restoring the pre-v1.3.0 behaviour.   
> The
> codepath to create a non-merge commit is not affected and still  
> defaults
> to the "--only" semantics.

Do you really want to do this? I think this is a pretty large change
that can bite users if they don't know about this -- for example,  
because
they forgot that they are in a merge (it happens..).

FWIW, I'd much rather see a useful error message than this change. If
this change does get in, I think it should be well-documented in the
man pages as well as in the release notes.

- Pieter

Re: [PATCH 3/3] git commit: pathspec without -i/-o implies -i semantics during a merge

[Junio C Hamano]

Pieter de Bie <pdebie@ai.rug.nl> writes:

> On 23 jan 2009, at 06:21, Junio C Hamano wrote:
>
>> This makes "git commit paths..." form default to "git commit -i paths"
>> semantics only during a merge, restoring the pre-v1.3.0 behaviour.
>> The
>> codepath to create a non-merge commit is not affected and still
>> defaults
>> to the "--only" semantics.
>
> Do you really want to do this? I think this is a pretty large change
> that can bite users if they don't know about this -- for example,
> because
> they forgot that they are in a merge (it happens..).
>
> FWIW, I'd much rather see a useful error message than this change. If
> this change does get in, I think it should be well-documented in the
> man pages as well as in the release notes.

As I said already in an earlier message in this thread, this is only a
weatherballoon series to help facilitate the discussion, and I am not
strongly in favor of this.  In fact, if I were, I would have done that
long time ago around v1.3.0, because there was a discussion about doing
this and the concensus back then was that the command changing the default
behaviour between -i and -o was too confusing, even though it may be
dwimming better.

The onus is upon those who argued that "commit paths" should default to
the --include semantics during a merge resolution in this thread to
improve the documentation, if they want this to go forward.