I spent a day trying to understand what the heck is going on in the
transport code, with the intent of adding an option like
--sign-if-possible to git push. This has come up twice on the mailing
list in the past couple weeks, and I also think it's important for
$DAY_JOB.
I'm giving up in despair, but I decided to leave some comments that
will hopefully help a future, more enterprising refactorer. Please
don't read this (entirely) as a rant; I understand that the transport
code is old and hairy and grew organically to get to this point. I
really do just hope this makes the next guy's job easier in
understanding the code. (But then again, this hope may be in vain:
it's not like I searched the mailing list myself before diving in :)
One of the biggest issues IMO is that the idea of "options" for
transports is grossly overloaded:
-struct git_transport_options contains mostly boolean options that are
read in fetch.c to enable certain options.
-In push.c, we for the most part ignore the smart_options field in
transport, and instead rely on the flags bitfield. However, some (not
all) flags in this bitfield have seemingly-equivalent fields in
git_transport_options. In some cases (particularly push_cert), the
field in git_transport_options is ignored.
-Similarly, one might think the executable arg to the connect callback
might bear some relation to the uploadpack/receivepack field in
smart_options. It does not.
-Some (but by no means all) options are set via transport_set_option
by passing in one of the TRANS_OPT_* string constants. This a)
switches on these values and populates the appropriate smart_options
field, and b) calls the set_option callback.
-The end result is that the TRANS_OPT_* constants are a mixture of
things that can be set on the command line and things that are
documented in the remote helper protocol. But there are also options
used in the remote helper protocol that are hard-coded and have no
constant, or can't be set on the command line, etc.
-The helper protocol's set_helper_option synchronously sends the
option to the helper process and reads the response. Naturally, the
return code from transport_set_option is almost always ignored.
In my ideal world:
-smart_options would never be NULL, and would instead be called
"options" with a "smart" bit which is unset for dumb protocols.
-Command line option processing code in {fetch,clone,push}.c would set
fields in options (formerly known as smart_options) rather than
passing around string constants.
-TRANS_OPT_* string constants would only be used for remote helper
protocol option names, and no more hard-coding these names.
-The flags arg to the push* callbacks would go away, and callbacks
would respect options instead.
-The helper code would not send options immediately, but instead send
just the relevant options immediately before a particular command
requires them. Hopefully we could then eliminate the set_option
callback entirely. (Two things I ran into that complicated this: 1)
backfill_tags mutates just a couple of options before reusing the
transport, and 2) the handling of push_cas_option is very
special-cased.)
There are other confusing things but I think that would make option
handling in particular less of a head-scratcher.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html