Thread (26 messages) flat view 26 messages, 3 authors, 6d ago

Re: [PATCH v5 1/2] pack-objects: give fetch_if_missing call sites access to 'repo'

From: Junio C Hamano <hidden>
Date: 2026-08-14 14:38:43

Tian Yuchen [off-list ref] writes:
'option_parse_missing_action()' is registered as an OPT_CALLBACK, so
its signature is fixed and cannot easily gain a 'repo' parameter of
its own.
Is that true?  OPT_CALLBACK takes (short, long, value, arghelp,
help, callback), but the value parameter, which typically receives
the location to store the parsed value in, is NULL for
"--missing=<...>" option, like so:

		OPT_CALLBACK_F(0, "missing", NULL, N_("action"),
		  N_("handling for missing objects"), PARSE_OPT_NONEG,
		  option_parse_missing_action),

And "value" does not have to be the exact location; we can pass
the address of the repository instance there, and the callback can
dereference it to reach repo->fetch_if_missing member to make an
assignment, if it wants to.

Am I missing something?
Let it only record 'arg_missing_action'. Instead, apply the
side effect right after 'parse_options()' returns in
'cmd_pack_objects()', where 'repo' is available.
When I review a new iteration, I first apply and look at the
difference from the previous iteration before reading the new
iteration of patches or their proposed commit log messages, and I
did notice this business about "arg_missing_action" and wondering
what it was about (which by the way is precisely I read the patch
before the log message to ensure that the log message explains what
I find unclear in the code).  Thanks for describing this.  But if we
use OPT_CALLBACK() to pass the location of the variable, this may no
longer be necessary, no?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help