Re: [PATCH 2/9] midx: allow marking a pack as preferred

On Thu, Feb 11, 2021 at 08:33:14PM +0100, SZEDER Gábor wrote:

quoted

+--preferred-pack=<pack>::
+	When using the `write` subcommand, optionally specify the
+	tie-breaking pack used when multiple packs contain the same
+	object. Incompatible with other subcommands, including `repack`,

I think this shouldn't be an option of the 'git multi-pack-index'
command but an option of its 'write' subcommand.

:-/. I wrote a lengthy response on Friday, but Gmail must have eaten it.

The gist of my response was that the intermingling of sub-commands with
options from other sub-commands goes deeper than just the documentation,
since command-line arguments are only parsed once in
builtin/multi-pack-index.c.

I explored whether or not it would be worth it to parse the common
options first, and then have separate options for each of the
sub-commands (as is done in the commit-graph builtin). But, this is
tricky, since we accept common options on either side of the sub-command
(i.e., we'd expect both 'git multi-pack-index --object-dir=... write' to
behave the same as 'git multi-pack-index write --object-dir=...').

So you could let the first call to parse_options() parse all of the
arguments, but then specialized arguments (e.g., 'repack --batch-size')
would cause the parse-options API to barf because the first call to
parse_options() doesn't recognize '--batch-size'.

I think the easiest way to do it would be to pass
PARSE_OPT_STOP_AT_NON_OPTION, and then let the subsequent calls to
parse_options() pass an array of option structs that also includes the
common options so they can be parsed on either side of the sub-command.

Obviously this leads to a lot of rather unfortunate duplication. So,
I'm content to leave it all as-is, and let the multi-pack-index
builtin check the disallowed combinations itself (e.g., if you passed
'--preferred-pack' but aren't in 'write' mode, then we should complain).

I can certainly move this piece of documentation into the 'write'
section, though, which should alleviate your immediate concern.

Thanks,
Taylor