Re: [PATCH] Add option to git-commit to allow empty log messages

[Jeff King <peff@peff.net>]

On Mon, Apr 05, 2010 at 02:10:58PM +0900, Miles Bader wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> > As a Porcelain, "git commit" has some leeway to enforce sensible policy on
> > the users, and "forbid commit that does not explain anything" is one such
> > policy.  It is not generally a good idea to expose the full capabilities
> > of plumbing to Porcelain if it leads to bad user behaviour, and such
> > "artificial" limitations are safety features we do not want to remove.
> 
> Isn't the requirement of using a longish option like
> "--allow-empty-message" enough of a warning to users though?
> 
> Although it seems reasonable for git _discourage_ bad practices, I think
> that should generally also be moderated with "... but if you _reallllly_
> want to, you can do this somewhat annoying thing....".  Forcing someone
> to use commit-tree, though, seems a bit much to me; an annoyingly long
> option seems about right.

Yes and no. There are other reasons not to use "git commit" in your
import script. You probably want to pass --allow-empty, too, and
--no-verify.  And you probably want to use --cleanup=none to keep
messages intact.

But most of all, even if you do everything right, we still don't promise
not to change it out from under you in a future version. Because it's
porcelain, and the plumbing method is to use commit-tree.  If
commit-tree is too hard to use, I would rather see the plumbing made
more friendly than encouraging people to build on top of porcelain.

All of that being said, I looked at the snerp-vortex source code (which
started this thread):

  http://github.com/rcaputo/snerp-vortex/blob/master/lib/SVN/Dump/Replayer/Git.pm

It uses several pieces of porcelain. Some in silly ways, like calling
"git status" to avoid calling git-commit when there are no changes and
getting an error code. Which is silly (if you are importing, you
probably want --allow-empty), wasteful (you just need the diff-index
part of status), and now broken (because status is no longer "commit
--dry-run", it always exits with status 0 whether there are changes or
not). Then there are things like calling "git add -f" with arguments,
and a "TODO: split arguments to handle larger filesets" comment. When he
should be using update-index, which takes updates on stdin.

He also notes in the README that it takes 250 seconds to convert his
test repo to git, but only 70 to make a flat filesystem, and that he
wants to move to using fast-import.

So yes, it sucks that his importer does not support empty comments, and
that the OP had to hack around it. But it already doesn't support many
things (like commits with a large number of files, and from what I can
see, files with spaces will break his `find` invocation). The right
answer is for him to move to fast-import, which will be way faster, more
robust, and is actually a supported plumbing interface.

I don't think it's worth adding new features to support a scripting
interface that we are trying to discourage. And I haven't seen another
argument in favor of empty commits besides importing.  Are people
really wanting to make empty commit messages while using git itself?

-Peff