threads / discuss / 17293

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

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

## tl;dr

17 messages between Jan 21, 2009 and Jan 23, 2009.

replies: 16people: 7as markdown or json

Asheesh Laroia· Jan 21, 2009, 21:00 UTC · lore

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.
Michael J Gruber· Jan 21, 2009, 21:35 UTC · re: Asheesh Laroia · lore

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

Asheesh Laroia venit, vidit, dixit 01/21/09 22:00:
Show 72 quoted lines
> 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
Nathan Yergler· Jan 21, 2009, 21:46 UTC · re: Michael J Gruber · lore

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

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:

Show 94 quoted lines
> 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
>
Johannes Sixt· Jan 22, 2009, 07:28 UTC · re: Nathan Yergler · lore

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

Please don't top-post.
Nathan Yergler schrieb:
Show 6 quoted lines
> 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
Nanako Shiraishi· Jan 23, 2009, 00:45 UTC · re: Johannes Sixt · lore

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

Quoting Johannes Sixt <j.sixt@viscovery.net>:
Show 18 quoted lines
> 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/
Asheesh Laroia· Jan 23, 2009, 02:55 UTC · re: Nanako Shiraishi · lore

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

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
Junio C Hamano· Jan 23, 2009, 06:15 UTC · re: Nanako Shiraishi · lore

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

Nanako Shiraishi <nanako3@lavabit.com> writes:
Show 8 quoted lines
> 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.

Junio C Hamano· Jan 23, 2009, 06:17 UTC · re: Junio C Hamano · lore

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

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
Johannes Sixt· Jan 23, 2009, 07:09 UTC · re: Junio C Hamano · lore

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

Junio C Hamano schrieb:
Show 22 quoted lines
> +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.

Show 21 quoted lines
> +	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
Junio C Hamano· Jan 23, 2009, 07:16 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <j.sixt@viscovery.net> writes:
Show 10 quoted lines
>> +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.

Johannes Sixt· Jan 23, 2009, 07:32 UTC · re: Junio C Hamano · lore

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

Junio C Hamano schrieb:
Show 13 quoted lines
> 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
Junio C Hamano· Jan 23, 2009, 07:39 UTC · re: Johannes Sixt · lore

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

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.
Junio C Hamano· Jan 23, 2009, 06:19 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Jan 23, 2009, 06:21 UTC · re: Junio C Hamano · lore

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

"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
Pieter de Bie· Jan 23, 2009, 09:51 UTC · re: Junio C Hamano · lore

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

On 23 jan 2009, at 06:21, Junio C Hamano wrote:
Show 6 quoted lines
> 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
Junio C Hamano· Jan 23, 2009, 17:01 UTC · re: Pieter de Bie · lore

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

Pieter de Bie <pdebie@ai.rug.nl> writes:
Show 17 quoted lines
> 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.

Michael J Gruber· Jan 22, 2009, 09:17 UTC · re: Nathan Yergler · lore

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

Nathan Yergler venit, vidit, dixit 21.01.2009 22:46:
Show 6 quoted lines
> 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

← back to recent threads