On Thu, Sep 13, 2007 at 12:35:35PM -0700, Junio C Hamano wrote:
quoted text
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > Junio C Hamano <gitster@pobox.com> wrote:
> >> "Shawn O. Pearce" <spearce@spearce.org> writes:
> >>=20
> >> > But I have to also wonder, aside from the sick amend case I just
> >> > talked about, how does one get to this part of git-commit.sh with
> >> > nothing to commit?
> >>=20
> >> Yeah, I also have to wonder.  Existing run_status call should
> >> have taken care of things.
> >
> > By the next patch; the one that introduces the --fast flag and lets
> > the user bypass the run_status call.  Which then makes this new check
> > the only safety valve the user has to prevent them from making an
> > empty commit and not realize they have forgotten to add files.
>=20
> But isn't that quite a bogus reason?  Why does this piece of
> code need to spend extra cycles if already checked with
> run_status?

Yes, the check looks almost redundant if run_status is enabled, but
- the bug fixed by commit v1.5.3.1-15-g4fb5fd5 was found due to
  this check;
- the check itself is very cheap compared to run_status;
- it closes again a race condition which was already fixed once,
  by commit v1.2.4-212-gb831015.

quoted text
> I do not see anything that disables this redundant
> check from the normal codepath in patch [2/2].

That said, I'm OK to replace
	if test -n "$current" -a ! -f "$GIT_DIR/MERGE_HEAD"
with
	if test -n "$fast" -a -n "$current" -a ! -f "$GIT_DIR/MERGE_HEAD"
if you consider this extra check redundant.


--=20
ldv