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

Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode

From
Dmitry V. Levin <ldv@altlinux.org>
Date
Sep 6, 2007, 10:16 UTC
Message-ID
<20070906101648.GD6665@basalt.office.altlinux.org>
In-Reply-To
<20070906022539.GG18160@spearce.org>
On Wed, Sep 05, 2007 at 10:25:39PM -0400, Shawn O. Pearce wrote:
Show 13 quoted lines
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> > Do not commit an unchanged tree in non-merge mode.
> 
> A laudable goal.  git-gui also does this.  Turns out the other
> checks within git-gui prevent the user from ever getting that far.
> I probably should remove the empty tree check as it costs CPU time
> to get the old tree.  But I'd rather have the safety check.
>  
> > The idea is that amend should not commit an unchanged tree,
> > one should just remove the top commit using git-reset instead.
> 
> NO.  `git commit --amend` is *often* used for fixing the commit
> message.
You see, my proposed change does not affect this usage case at all.
> Or adding additional detail.

If that "additional" detail just undoes the latest commit, why should "git commit --amend" welcome such thing? I did not get your pint here.

Show 5 quoted lines
> Forcing the user to do
> a `git reset --soft HEAD^ && git commit --amend` just because
> you don't want git-commit to make an "empty commit" (which it
> doesn't usually like to do now anyway!) is a major step back
> in functionality.

I suppose that helping users to avoid doing really stupid things does not look as a major step back in functionality, just otherwise.

Show 10 quoted lines
> > +		current_tree="$(git cat-file commit "$current${amend:+^}" 2>/dev/null |
> > +				sed -e '/^tree \+/!d' -e 's///' -e q)"
> 
> The better way to get the old tree would be this:
> 
> 		current_tree="$(git rev-parse "$current${amend:+^}^{tree}" 2>/dev/null
> 
> as it avoids the tool from needing to know about the internal
> representation of a commit object.  It also avoids an entire
> fork+exec of a sed process.
Agreed.
Show 11 quoted lines
> > +		if test "$tree" = "$current_tree"
> > +		then
> > +			echo >&2 "nothing to commit${amend:+ (use \"git reset HEAD^\" to remove the top commit)}"
> 
> That message is a bad idea.  Doing a mixed mode reset will also
> reset the index, causing the user to lose any changes that had
> already been staged.  This may actually be difficult for him/her to
> recover from if they have used `git add -i` or git-gui to stage only
> certain hunks of files, or if their working tree has been further
> modified after the commit but they want to go back and amend the
> message only of the prior commit.

Would "git reset --soft HEAD^" advice be better than first one? Could you suggest a better message, please?

-- 
ldv
Previous: Shawn O. PearceNext: Linus Torvalds
Message 3 of 13 in “git-commit: Disallow unchanged tree in non-merge mode”
  1. 1/2 git-commit: Disallow unchanged tree in non-merge modeDmitry V. Levin, Sep 5, 2007
  2. Shawn O. PearceSep 6, 2007
  3. Dmitry V. LevinSep 6, 2007
  4. Linus TorvaldsSep 14, 2007
  5. 1/2 Fix "git diff" setup codeLinus Torvalds, Sep 14, 2007
  6. 2/2 Fix the rename detection limit checkingLinus Torvalds, Sep 14, 2007
  7. Linus TorvaldsSep 14, 2007
  8. Linus TorvaldsSep 14, 2007
  9. Junio C HamanoSep 14, 2007
  10. Linus TorvaldsSep 14, 2007
  11. Junio C HamanoSep 14, 2007
  12. Linus TorvaldsSep 14, 2007
  13. Junio C HamanoSep 14, 2007

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.