Re: [PATCH v4 6/6] agent: advertise OS name via agent capability

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

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.
>
> Our current agent capability is in the form of "package/version" (e.g.,
> "git/1.8.3.1"). Let's extend it to include the operating system name (os)
> i.e in the form "package/version os" (e.g., "git/1.8.3.1 Linux").
>
> Including OS details in the agent capability simplifies implementation,
> maintains backward compatibility, avoids introducing a new capability,
> encourages adoption across Git-compatible software, and enhances
> debugging by providing complete environment information without affecting
> functionality.

I obviously agree with the benefits enumerated in the above
paragraph.  The simpler, the better.

I however wonder ...

> Add the `transfer.advertiseOSInfo` config option to address privacy
> concerns. It defaults to `true` and can be changed to `false`.

... if this configuration knob is at the right granularity.

For privacy concious folks, I would imagine that the distinction
between "git/1.8.3.1" vs "git/2.48.1" would be something they do not
want to reveal equally as, if not more than, which Operating System
they are on.  Such a privacy concious user may already be using
GIT_USER_AGENT environment variable to squelch it already, anyway.

If we were to give them an improvement in the area for privacy
features, I would think it would be to add a configuration variable
to turn the agent off, instead of having to leave GIT_USER_AGENT
environment variable set in the environment of their processes.

On the other hand, for the rest of us who think "git/1.8.3.1 Linux"
is not too much of a secret, we do not need a knob to configure it
between "git/1.8.3.1" and "git/1.8.3.1 Linux".

So, while I view some parts of the series would have been a good
exercise to use various features (like config subsystem) from our
API, I prefer if we kept the end-user interface not overly
customizable (iow, without a config-knob, we do not need to add a
code to inspect the new configuration variable).

After all, GIT_USER_AGENT let's you hide not just the OS part but
any other things from the user-agent string already.

I notice that unlike user_agent() vs user_agent_sanitized(), you
only have a single function for os_info(), which I think is a good
design.  But if we were to go that route, shouldn't we call the
function os_info(), not os_info_sanitized()?  The idea behind a
single function is that you cannot obtain unsanitized version of
os_info() out of the system at all, so what _sanitized() returns
would be what os_info() without _sanitized suffix would return to
the caller anyway.

Thanks.