[PATCH] git-commit --amend: respect grafted parents.

[PATCH] git-commit --amend: respect grafted parents.

[Johannes Sixt]

This allows a poor-mans-filter-branch of the top-most commit.

Specifically, I had this history:

   --o--o--A'    <-- amended version of A
         \
          A
           \
    --o--B--M

I now wanted to "amend" M to pull in A' instead of A as its second parent.
For various reasons I didn't want to redo the merge M again, in particular,
it already contained the changes that were amended into A'. So I figured
I would just install a graft that lists B and A' as parents of M and then
do a simple git commit --amend. Alas, git commit looks at the real parents
instead of the grafted ones, so the amended M' would still have A as its
second parent. Here is the fix that picks the grafted parents instead.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
  I know that git-commit is becoming a builtin. At the least, here is
  a test that makes sure that the behavior is remains.

  -- Hannes

 git-commit.sh     |    4 ++--
 t/t7501-commit.sh |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 t/t7501-commit.sh

diff --git a/git-commit.sh b/git-commit.sh
index 7a7a2cb..33e7503 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -529,8 +529,8 @@ then
 		PARENTS="-p HEAD "`sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD"`
 	elif test -n "$amend"; then
 		rloga='commit (amend)'
-		PARENTS=$(git cat-file commit HEAD |
-			sed -n -e '/^$/q' -e 's/^parent /-p /p')
+		PARENTS=$(git rev-list --parents -1 HEAD |
+			sed -e 's/^[^ ]*//' -e 's/ / -p /g')
 	fi
 	current="$(git rev-parse --verify HEAD)"
 else
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
old mode 100644
new mode 100755
index b151b51..09c0d98
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -163,4 +163,13 @@ test_expect_success 'partial commit that involves removal (3)' '
 
 '
 
+test_expect_success '--amend respects grafts' '
+
+	grandparent=$(git rev-parse HEAD~2) &&
+	echo $(git rev-parse HEAD) $grandparent > .git/info/grafts &&
+	git commit --amend -C HEAD &&
+	test "parent $grandparent" = "$(git cat-file commit HEAD | grep "^parent")"
+
+'
+
 test_done
-- 
1.5.3.3.gcc9e

Re: [PATCH] git-commit --amend: respect grafted parents.

[Johannes Schindelin]

Hi,

On Wed, 26 Sep 2007, Johannes Sixt wrote:

> This allows a poor-mans-filter-branch of the top-most commit.
> 
> Specifically, I had this history:
> 
>    --o--o--A'    <-- amended version of A
>          \
>           A
>            \
>     --o--B--M
> 
> I now wanted to "amend" M to pull in A' instead of A as its second 
> parent. For various reasons I didn't want to redo the merge M again, in 
> particular, it already contained the changes that were amended into A'. 
> So I figured I would just install a graft that lists B and A' as parents 
> of M and then do a simple git commit --amend. Alas, git commit looks at 
> the real parents instead of the grafted ones, so the amended M' would 
> still have A as its second parent. Here is the fix that picks the 
> grafted parents instead.

I do not think that this is a "fix".

I'd really suggest that you fix it up (before the amend!) by installing 
the graft, and running git-filter-branch -1 <branch>.  (Note: I did not 
test this, but it _should_ work.  If it does not, please let me know.)

Ciao,
Dscho

Re: [PATCH] git-commit --amend: respect grafted parents.

[Johannes Sixt]

Johannes Schindelin schrieb:
> Hi,
> 
> On Wed, 26 Sep 2007, Johannes Sixt wrote:
> 
>> This allows a poor-mans-filter-branch of the top-most commit.
>>
>> Specifically, I had this history:
>>
>>    --o--o--A'    <-- amended version of A
>>          \
>>           A
>>            \
>>     --o--B--M
>>
>> I now wanted to "amend" M to pull in A' instead of A as its second 
>> parent. For various reasons I didn't want to redo the merge M again, in 
>> particular, it already contained the changes that were amended into A'. 
>> So I figured I would just install a graft that lists B and A' as parents 
>> of M and then do a simple git commit --amend. Alas, git commit looks at 
>> the real parents instead of the grafted ones, so the amended M' would 
>> still have A as its second parent. Here is the fix that picks the 
>> grafted parents instead.
> 
> I do not think that this is a "fix".
> 
> I'd really suggest that you fix it up (before the amend!) by installing 
> the graft, and running git-filter-branch -1 <branch>.  (Note: I did not 
> test this, but it _should_ work.  If it does not, please let me know.)

The problem with filter-branch is that it leaves a committer date in M that 
is before A', and for this reason is not the right tool for this task. So I 
do think that this fix is necessary.

-- Hannes

Re: [PATCH] git-commit --amend: respect grafted parents.

[Johannes Schindelin]

Hi,

On Wed, 26 Sep 2007, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> 
> > On Wed, 26 Sep 2007, Johannes Sixt wrote:
> > 
> > > This allows a poor-mans-filter-branch of the top-most commit.
> > > 
> > > Specifically, I had this history:
> > > 
> > >    --o--o--A'    <-- amended version of A
> > >          \
> > >           A
> > >            \
> > >     --o--B--M
> > > 
> > > I now wanted to "amend" M to pull in A' instead of A as its second 
> > > parent. For various reasons I didn't want to redo the merge M again, 
> > > in particular, it already contained the changes that were amended 
> > > into A'. So I figured I would just install a graft that lists B and 
> > > A' as parents of M and then do a simple git commit --amend. Alas, 
> > > git commit looks at the real parents instead of the grafted ones, so 
> > > the amended M' would still have A as its second parent. Here is the 
> > > fix that picks the grafted parents instead.
> > 
> > I do not think that this is a "fix".
> > 
> > I'd really suggest that you fix it up (before the amend!) by 
> > installing the graft, and running git-filter-branch -1 <branch>.  
> > (Note: I did not test this, but it _should_ work.  If it does not, 
> > please let me know.)
> 
> The problem with filter-branch is that it leaves a committer date in M 
> that is before A', and for this reason is not the right tool for this 
> task. So I do think that this fix is necessary.

But that's easily fixed with "--env-filter 'export 
GIT_COMMITTER_DATE=$(date +%s)'" or something similar.

The reason why I insist on not putting this into --amend is that I think 
this is not really an amend, but actively a rewrite of the merge commit.  

Ciao,
Dscho

Re: [PATCH] git-commit --amend: respect grafted parents.

[Johannes Sixt]

Johannes Schindelin schrieb:
> The reason why I insist on not putting this into --amend is that I think 
> this is not really an amend, but actively a rewrite of the merge commit.  

You have a point here. I'm convinced. Scrap the patch.

-- Hannes

Re: [PATCH] git-commit --amend: respect grafted parents.

[Junio C Hamano]

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

> Johannes Schindelin schrieb:
>> The reason why I insist on not putting this into --amend is that I
>> think this is not really an amend, but actively a rewrite of the
>> merge commit.
>
> You have a point here. I'm convinced. Scrap the patch.

I am slow today.  Since when --amend is not about "a rewrite of
commit"?

Re: [PATCH] git-commit --amend: respect grafted parents.

[Johannes Schindelin]

Hi,

On Wed, 26 Sep 2007, Junio C Hamano wrote:

> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
> > Johannes Schindelin schrieb:
> >> The reason why I insist on not putting this into --amend is that I 
> >> think this is not really an amend, but actively a rewrite of the 
> >> merge commit.
> >
> > You have a point here. I'm convinced. Scrap the patch.
> 
> I am slow today.  Since when --amend is not about "a rewrite of commit"?

Technically, you are right, of course.  Commit objects are immutable.  But 
from a _porcelain_ view I maintain that "amending" is about changes _to_ a 
commit.  It is not about redefining the (a) parent.

Ciao,
Dscho

Re: [PATCH] git-commit --amend: respect grafted parents.

[David Kastrup]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Wed, 26 Sep 2007, Junio C Hamano wrote:
>
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>> 
>> > Johannes Schindelin schrieb:
>> >> The reason why I insist on not putting this into --amend is that I 
>> >> think this is not really an amend, but actively a rewrite of the 
>> >> merge commit.
>> >
>> > You have a point here. I'm convinced. Scrap the patch.
>> 
>> I am slow today.  Since when --amend is not about "a rewrite of commit"?
>
> Technically, you are right, of course.  Commit objects are immutable.  But 
> from a _porcelain_ view I maintain that "amending" is about changes _to_ a 
> commit.  It is not about redefining the (a) parent.

Well, if you already branched off the commit, the changes "to the
commit" will not register on the branch.  So my view is that amending
is about changes to HEAD, not to the HEAD commit.  And since branching
is certainly a porcelain operation and is clearly not affected by
amending commits, I think that the "rewrite of a commit" wording
strikes a good balance between "the original commit remains" and "it
is functionally replaced in the HEAD".

-- 
David Kastrup