From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 13 Feb 2008 23:25:11 GMT
Subject: Re: [PATCH/selftest] hash-object: don't rely on order of --stdin, -w arguments
Message-ID: <7v8x1oo2w8.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080213224941.18121.qmail@c0fdbb95c1b5f1.315fe32.mid.smarden.org>

Gerrit Pape <pape@smarden.org> writes:

> Fix 'git hash-object --stdin -w' to actually write the object, just as
> 'git hash-object -w --stdin' does.
>
> Reported by Josh Triplett through
>  http://bugs.debian.org/464432
>
> Signed-off-by: Gerrit Pape <pape@smarden.org>

Thanks.  I think the patch itself is a good fix to the old
design mistake we made.

However, I would _really_ like to see something like the
following mentioned in the proposed commit log message for
discussion:

    This regresses the use case of running:

        $ git hash-object --stdin Makefile <cache.h

    to obtain hash values for cache.h and then Makefile.  It
    used to report the object names in order, but now it always
    processes --stdin at the end.  It is not an issue if
    everything is file (i.e. "git hash-object cache.h Makefile"
    is an easy replacement), but if existing scripts used the
    option to read from a pipe, this might become problematic.

    Similarly, when typing from terminal:

        $ git hash-object --stdin --stdin
        foo
        ^D
        bar
        ^D

    used to work, but not anymore.  The latter however should
    not be something we need to worry about.  It is an insane
    usage.

Granted, we _should have_ prevented such insane usages (the
second one is definitely insane and not so useful, and the first
one is but to a much lessor degree).  We _should have_ forced
scripted users to do these with two separate invocations of "git
hash-object", by refusing more than one --stdin and --stdin with
filename on the command line.  But we didn't, and this earlier
design mistake has already been in the field for a long time.
People may have been depending on the existing misbehaviour.

So I am Ok with the patch, and I strongly suspect that we should
even detect more than one --stdin and --stdin with filename on
the command line to reject the usage your saner version behaves
differently from before, instead of accepting and silently doing
an unexpected thing.

But at the same time I would really like our commit messages,
and Release Notes that is based on these commit messages, to be
_candid_ about our previous mistakes and the incompatibility we
are incurring to the existing users.

> +test_expect_success \
> +    'git hash-object -w --stdin saves the object' \
> +    'echo foo | git hash-object -w --stdin &&
> +    test -r .git/objects/25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99 &&
> +    rm -f .git/objects/25/7cc5642cb1a054f08cc83f2d943e56fd3ebe99'

I'd feel better if tests outside t0000 did not hardcode the
actual values, like this:

	obname=$(echo foo | git hash-object -w --stdin) &&
        ob=$(expr "$obname" : "\(..\)") &&
        name=$(expr "$obname" : "..\(.*\)") &&
        test -f ".git/objects/$ob/$name"