Re: [PATCH GSoC v18 13/13] cat-file: make remote-object-info allow-list dynamic

[Junio C Hamano <gitster@pobox.com>]

"Pablo Sabater" <pabloosabaterr@gmail.com> writes:

>>> 2. Filters the request in fetch_object_info() dropping any option that
>>>    the server does not advertise.
>>>
>>> 3. After the fetching, the options that haven't been dropped are the ones
>>>    fetched and supported by the server, these supported options are
>>>    mapped and remote_allowed_atoms is populated with the placeholders.
>>>
>>> 4. expand_atom() checks remote_allowed_atoms with the same behaviour as
>>>    the static allow_list had.
>>
>> I am not sure I follow the above entirely.  Could you add a
>> concrete example to the commit message?
>>
>> For instance, if the client wants "%(objectsize) %(objectcolor)" and
>> the server only supports 'size' but not 'color', the filtering in
>> step (2) prevents the client from asking about the color, requesting
>> only the size instead.  When the server says the size is 42, step (3)
>> uses that to substitute '%(objectsize)'.  Would the end result then
>> be "42 %(objectcolor)"?
>
> You've gotten everything right until the last step, because we have only
> size from the server there is no data to match %(objectcolor) and the
> end result is an empty string for %(objeccolor):
>
> "42 "
>
> Note that %(objectcolor) doesn't exists and it would have die(), the
> empty string is only for known but unsupported placeholders.

It was not clear there is a distinction between "unknown" and
"known but unsupported".  The proposed log message needs to be
clarified to make this distinction obvious.

>> And if the request is only for "%(objectname)", an empty
>> object_info_options is given to get_remote_info().
>
> Right now 'name' is not part of the protocol as 'type' or 'size' are,
> 'objectname' is always allowed but only shown if it's present on the
> format.
> If the format is only "%(objectname)" then there's nothing to ask the
> server for.
>
> The current code avoids making the request if there's only objectname or
> nothing supported, but still goes through the connection work. I will
> add an early return to just output the oid back without any connection.

I think you are heading in the opposite direction.  Rather, when
only the object name is requested, I was hoping we would pick
something cheap to retrieve and ask the remote side for it, if
only to catch a bogus or missing object name.

Thanks.