git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Peter Hunkeler <phunsoft@gmx.net>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Git Mailinglist <git@vger.kernel.org>
Subject: Re: Git not commiting anything if file created and "git add"ed in pre-commit hook is the only file in the staging area
Date: Sat, 30 Oct 2021 14:25:09 -0700	[thread overview]
Message-ID: <xmqqlf2adx3e.fsf@gitster.g> (raw)
In-Reply-To: <4c88e2ae-e42f-491a-2b97-aa1f92c392d5@kdbg.org> (Johannes Sixt's message of "Sat, 30 Oct 2021 18:44:54 +0200")

Johannes Sixt <j6t@kdbg.org> writes:

> Even though the documentation does not say explicitly that the commit
> must not be changed, it is implicit in the stated intent (that the
> commit is only checked). Depending on that some particular behavior
> works for you sometimes is then your own business, and when it breaks
> you get to keep both parts.

The above matches my _intention_ for the pre-commit hook when I
added it in 2005, but there were enough people who wanted to abuse
the interface that it no longer exactly matches the reality.  It has
been made usable to inspect the changes (which is the original
purpose of the hook) and in addition, apply mechanical fixes on top,
before making the commit.

The story so far, however, is that the scenario that started this
thread is not even that, if I understand it correctly.  The question
is: what should happen when *nothing* is different between the HEAD
and the index, the user types "$ git commit" (no pathspec, no
nothing, commit the index as-is, no --allow-empty option), and the
pre-commit mucks with the index to "fix" the content in the index.

Because we check if there is anything to commit before we invoke the
pre-commit hook and then reject an empty commit based on that, we
successfully reject the attempt to commit.  This is in line even
with the modern intention, as the mucking done by the hook cannot be
"fixes" based on the observation of the changes made to the index by
the user (e.g. "The user tries to add changes, with whitespace
breakages, and then pre-commit hook notices.  Instead of rejecting,
it fixes the whitespace issues for the user" is the justifying use
case behind the looser than the original "check only, no touching"
definition).  So ...

> In conclusion, the pre-commit hook behaves as designed and nothing has
> to be changed.

... in conclusion, pre-commit is *not* a place to make such a change
that may be created by a script even when there is no human
initiated change, and "git commit" is behaving as designed.

But there are two things that are not solved yet.

 * It is *not* the ultimate goal of the OP to "use" pre-commit
   hook.  The goal of OP is to find a workflow ingredient where
   changes other than human initiated ones are committed at the same
   time human user tries to commit changes created by human.  So if
   pre-commit is the wrong tool to use for that purpose, what is it?

   I suspect that there is no built-in way to do this, and I am not
   sure if it is a good idea to add such a feature to the tool---as
   some have already noted in the discussion, it may encourage a bad
   workflow to include such non-human-created artifacts to human
   initiated commit.  I don't know.

 * If we do not consider changes made by pre-commit hook to count as
   "without --allow-empty, an empty commit is rejected" logic, why
   do we even call the hook in the first place in such a case?  I
   think there is a room for improvement on our side---perhaps we
   can make "git commit" fail much earlier in such a case without
   calling the pre-commit hook.

Thanks for the discussion so far.


  reply	other threads:[~2021-10-30 21:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-27 19:03 Git not commiting anything if file created and "git add"ed in pre-commit hook is the only file in the staging area Peter Hunkeler
2021-10-27 22:07 ` brian m. carlson
2021-10-27 22:42   ` Jeff King
2021-10-28  1:32     ` Junio C Hamano
2021-10-28 12:08   ` Peter Hunkeler
2021-10-30 13:21     ` Peter Hunkeler
2021-10-30 16:44       ` Johannes Sixt
2021-10-30 21:25         ` Junio C Hamano [this message]
2021-11-02 13:13           ` Peter Hunkeler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlf2adx3e.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=phunsoft@gmx.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).