Stefan Beller <sbel...@google.com> writes:
>> Hmph, so everybody else thinks it is interacting with 'upload-pack',
>> and this is the only function that knows it is actually talking with
>> 'upload-pack-2'?
>
> Yes.
>>
>> I am wondering why there isn't a separate helper function that
>> munges data->options.{uploadpack,receivepack} fields based on
>> the value of transport_version that is called _before_ this function
>> is called.
>
> That makes sense.
Hmph, but then everybody would know that it is now interacting with
upload-pack-2; I think it probably is a better way to go.
In any case, all codepaths other than what actually runs exec()
should not be basing their decision on the program names---that is
what you added transport_version field for, and they should look
at that field if they want to switch behaviour between protocol
versions anyway, so I think we can live with either way.
>> Also, how does this interact with the name of the program the end
>> user can specify via "fetch --upload-pack=<program name>" option?
>
> You'd specify --upload-pack=foo-frotz and --transport-version=2
> and it would look for foo-frotz-2 instead.
Hmm, that's an unfortunate interaction. If you wrote a new protocol
driver that talks v2, it may be natural to say
git fetch --upload-pack=a.out --transport-version=2
when you want to test it, and we do not want to invoke a.out-2 in
that case.
> The problem IMHO is we have quite a few places where the
> upload-pack binary path can be configured. Either as a command line
> option or as a repository configuration.
At least shouldn't you be able to tell if we are using compiled-in
default or user specified value (whether configuration/command line)?
Does the code that initialise data->options.{upload,receive}pack
fields with the compiled-in default values know what protocol
version is going to be used at that point? I think that is where
the appending of "-2" should be done; in other words, I think
addition of "-2" should be done _ONLY_ for compiled-in default
values, and it should be done not just for exec() but should be
visible to everybody, including those who are debugging and inspect
data->options.uploadpack field. What we spawn must match what is
stored there.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
