Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

["Pablo Sabater" <pabloosabaterr@gmail.com>]

On Wed Oct 28, 2026 at 6:37 AM WET, Junio C Hamano wrote:
> "Pablo Sabater" <pabloosabaterr@gmail.com> writes:
>
>>>> +	if (!transport->smart_options)
>>>> +		die(_("smart options required"));
>>>
>>> Is this an end-user facing error message?  I am not sure a user who
>>> saw this message can figure out what went wrong or how to correct
>>> it.
>>
>> I thought it would be better to say why it fails instead of a generic
>> error. If we drop the check it will die() later with "remote does not
>> support object-info".
>>
>> Since the smart options check only die()s when the remote is a bundle,
>> I could keep it and change the message to "object-info is not supported
>> on bundles".
>
> I am somewhat torn on this.  It is certainly much better than the
> "smart options required" message, which the end user would not know
> how to act upon.  However, that improvement only holds until we gain
> another transport other than bundle that also lacks support for
> "smart options".  Saying something like "object-info is not
> supported by this remote." would avoid blindly mentioning "bundle"
> when the remote that lacks object-info support is actually not a
> bundle, but it loses the clarity your rewritten message provides
> when the problematic remote indeed is a bundle.
>
> Do we have the remote information at this point in the code (i.e., the
> URL of the remote that was given to the remote-object-info command)
> that the user would immediately recognize as the remote they asked
> the command to access?  If so, "object-info is not supported by %s"
> filled with the remote information would probably work better.

We have what the user wrote for the remote in argv[0], so we could have
"object-info is not supported by <remote>", and it would be the same
for anything that sets smart_options to NULL.

But with that in mind, what if in the future there's a transport without
smart_options that does support object-info? The smart_options check is
there because of "this transport can't do object-info", and nothing
on the object-info path reads smart_options from what I checked.

So I'm going to drop that die(). Later in the code there's already a 
check on transport_fetch_object_info(), which looks for 
fetch_object_info in the transport's vtable. It's a bit later, but it 
checks the real capability and it will give less trouble in the future. 

Currently, it has a generic error but it can print a more detailed 
version such as: "object-info is not supported by <remote>"

I'll send v2 later this week as I have to study for some of my midterms.

Regards,
Pablo

>
> Thanks.