threads / patch / 10030

patchgit-commit --amend: respect grafted parents.

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

## tl;dr

8 messages between Sep 26, 2007 and Sep 27, 2007. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Johannes Sixt· Sep 26, 2007, 12:11 UTC · lore
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
Show changes to 2 files +11 −2

git-commit.sh, 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
Johannes Schindelin· Sep 26, 2007, 12:24 UTC · re: Johannes Sixt · lore

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

Hi,
On Wed, 26 Sep 2007, Johannes Sixt wrote:
Show 18 quoted lines
> 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

Johannes Sixt· Sep 26, 2007, 12:52 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin schrieb:
Show 28 quoted lines
> 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
Johannes Schindelin· Sep 26, 2007, 14:03 UTC · re: Johannes Sixt · lore

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

Hi,
On Wed, 26 Sep 2007, Johannes Sixt wrote:
Show 33 quoted lines
> 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

Johannes Sixt· Sep 26, 2007, 14:29 UTC · re: Johannes Schindelin · lore

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

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
Junio C Hamano· Sep 26, 2007, 18:50 UTC · re: Johannes Sixt · lore

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

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

Johannes Schindelin· Sep 26, 2007, 19:47 UTC · re: Junio C Hamano · lore

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

Hi,
On Wed, 26 Sep 2007, Junio C Hamano wrote:
Show 10 quoted lines
> 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

David Kastrup· Sep 27, 2007, 08:39 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 18 quoted lines
> 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

← back to recent threads