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

Jeff King [off-list ref] writes:

But I think that makes the --stdin check redundant. I.e., here:

quoted

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.