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

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

Junio C Hamano [off-list ref] writes:

quoted

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