Re: [PATCH v2 5/6] connect: advertise OS version

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

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> On Sat, Jan 18, 2025 at 3:52 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
>>
>> > As some issues that can happen with a Git client can be operating system
>> > specific, it can be useful for a server to know which OS a client is
>> > using. In the same way it can be useful for a client to know which OS
>> > a server is using.
>>
>> Hmph.  The other end may be running different version of Git, and
>> the version difference of _our_ software is probably more relevant.
>> For that matter, they may even be running something entirely
>> different from our software, like Gerrit.  So I am not sure I am
>> convinced that os-version thing is a good thing to have with that
>> paragraph.
> Hi Junio,
>
> What could be a better way of describing this ? Also, user-agent capability is
> already sharing the information about the version of Git.

I dunno.  I only said that what you said does not convince me that
os-version is a good thing.  Try harder, perhaps, to be more
convincing?  After all it is your itch.

An alternative that may be conceptually cleaner is to encourage
people to include not just Git version but OS variant information in
the existing "agent" capability, making it easier to do (which
probably means an addition to configuration knob), and encourage
implementors of other Git-compatible software to also let their
systems identify themselves via the "agent" capability.

As we have documented that "agent" strings are purely informative,
there shouldn't be any problem if we started identifying the version
of Git running on one end as "git/2.47.0 Linux" (instead of
"git/2.47.0").

>> And among these four combinations, only one of them results in "send
>> to each other", but the description above is fuzzy.
> Yeah, describing the four combinations would better right ?

I do not think readers necessarily would want to hear about the four
combinations; a paragraph that makes it clear that the configuration
is independently set on either end of the connection will make it
obvious to them without being told.

>> So the question is again, do we ever need to use os_version() that
>> is a raw string that may require sanitizing?  I do not think of any
>> offhand.
> In this case, I guess there has to be a conclusion on what to do.

As I didn't hear any concrete use cases of the version string before
sanitizing, I would suggest to make os_version() to

 - ask for a string from the underlying system layer (like uname(2)
   or its emulation), 

 - immediately sanitize it,

 - cache the result from the above (just like the
   os_version_sanitized() in the posted patch does with a variable
   of type "static char *" in the function scope), and

 - keep returning that sanitized version to the callers.

to simplify the API surface.

Thanks.