Re: [PATCH] index-pack: teach --promisor to require --stdin

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> But I think that makes the --stdin check redundant. I.e., here:
>
>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 08b340552f..c46b6e4061 100644
>> --- a/builtin/index-pack.c
>> +++ b/builtin/index-pack.c
>> @@ -1970,6 +1970,10 @@ int cmd_index_pack(int argc,
>>  		usage(index_pack_usage);
>>  	if (fix_thin_pack && !from_stdin)
>>  		die(_("the option '%s' requires '%s'"), "--fix-thin", "--stdin");
>> +	if (promisor_msg && !from_stdin)
>> +		die(_("the option '%s' requires '%s'"), "--promisor", "--stdin");
>> +	if (promisor_msg && pack_name)
>> +		die(_("--promisor cannot be used with a pack name"));
>
> ...just the second one would be sufficient, because the context just
> above this has:
>
> 	if (!pack_name && !from_stdin)
> 		usage(index_pack_usage);
>
> So if there isn't a pack name then from_stdin must be set anyway.

Nice findings that leads to ... 

> What you've written won't behave incorrectly, but I wonder if this means
> we can explain the rule in a more simple way:
>
>   - the --promisor option requires that we be indexing a pack in the
>     object database
>
>   - when not given a pack name on the command line, we know this is true
>     (because we generate the name ourselves internally)
>
>   - when given a pack name on the command line, we _could_ check that it
>     is inside the object directory, but we don't currently do so and
>     just bail. That could be changed in the future.
>
> And then there is no mention of --stdin at all (though of course it is
> an implication of the second point, since we have to get input somehow).

... a good simplification.  Not of the implementation---as it is
already simple enough---but of the concept, and simplification of
the latter counts a lot more ;-)

Thanks, both, for working on this.