git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 20, 2024, 01:34 UTC
Message-ID
<xmqqcyiqadtf.fsf@gitster.g>
In-Reply-To
<20241119185345.GB15723@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> 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 ... 
Show 15 quoted lines
> 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.
Previous: Jeff KingNext: Jonathan Tan
Message 31 of 37 in “When fetching from a promisor remote, repack local objects referenced”
  1. 0/5 When fetching from a promisor remote, repack local objects referencedJonathan Tan, Oct 24, 2024
  2. 1/5 pack-objects: make variable non-staticJonathan Tan, Oct 24, 2024
  3. Taylor BlauOct 28, 2024
  4. Jonathan TanOct 28, 2024
  5. Taylor BlauOct 28, 2024
  6. Jonathan TanOct 28, 2024
  7. 2/5 t0410: make test description clearerJonathan Tan, Oct 24, 2024
  8. 3/5 t0410: use from-scratch serverJonathan Tan, Oct 24, 2024
  9. 4/5 t5300: move --window clamp test next to unclampedJonathan Tan, Oct 24, 2024
  10. 5/5 index-pack: repack local links into promisor packsJonathan Tan, Oct 24, 2024
  11. Josh SteadmonOct 30, 2024
  12. Jonathan TanNov 1, 2024
  13. Han YoungOct 25, 2024
  14. Taylor BlauOct 25, 2024
  15. Junio C HamanoNov 2, 2024
  16. Taylor BlauOct 25, 2024
  17. 0/4 When fetching from a promisor remote, repack local objects referencedJonathan Tan, Nov 1, 2024
  18. 1/4 t0410: make test description clearerJonathan Tan, Nov 1, 2024
  19. 2/4 t0410: use from-scratch serverJonathan Tan, Nov 1, 2024
  20. 3/4 t5300: move --window clamp test next to unclampedJonathan Tan, Nov 1, 2024
  21. Jeff KingNov 13, 2024
  22. Jonathan TanNov 13, 2024
  23. Jeff KingNov 14, 2024
  24. Junio C HamanoNov 14, 2024
  25. Jeff KingNov 15, 2024
  26. Jonathan TanNov 15, 2024
  27. Jeff KingNov 16, 2024
  28. index-pack: teach --promisor to require --stdinJonathan Tan, Nov 18, 2024
  29. Junio C HamanoNov 19, 2024
  30. Jeff KingNov 19, 2024
  31. Junio C HamanoNov 20, 2024
  32. index-pack: teach --promisor to forbid pack nameJonathan Tan, Nov 19, 2024
  33. Jeff KingNov 20, 2024
  34. Jeff KingNov 14, 2024
  35. 4/4 index-pack: repack local links into promisor packsJonathan Tan, Nov 1, 2024
  36. Junio C HamanoNov 4, 2024
  37. Junio C HamanoNov 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.