Re: [PATCH 3/4] connect: advertise OS version

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2025, 16:22 UTC
Message-ID: <xmqqfrlvzzcd.fsf@gitster.g>
In-Reply-To: <20250106103713.1452035-4-usmanakinyemi202@gmail.com>

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

Show 7 quoted lines

> +
> +transfer.advertiseOSVersion::
> +	When `true`, the `os-version` capability is advertised by clients and
> +	servers. It makes clients and servers send to each other a string
> +	representing the operating system name, like "Linux" or "Windows".
> +	This string is retrieved from the 'sysname' field of the struct returned
> +	by the uname(2) system call. Defaults to true.

Shouldn't `sysname` be typeset as a literal, just like `true` and `os-version`?

Show 7 quoted lines

> +os-version
> +~~~~~~~~~~
> +
> +In the same way as the `agent` capability above, the server can
> +advertise the `os-version` capability with a value `X` (in the form
> +`os-version=X`) to notify the client that the server is running an
> +operating system that can be identified by `X`. The client may

Hmph.  I am not sure what's the value of mentioning 'X' here.  To me

    ... can advertise the `os-version` capability to notify the kind
    of operating system it is running on.

conveys the same thing with much fewer bytes.

Show 5 quoted lines

> +optionally send its own `os-version` string by including the
> +`os-version` capability with a value `Y` (in the form `os-version=Y`)
> +in its request to the server (but it MUST NOT do so if the server did
> +not advertise the os-version capability). The `X` and `Y` strings may
> +contain any printable ASCII characters except space (i.e., the byte

This is misleading. ASCII printable characters range from 33 to 126 (inclusive), but by saying "except space", the readers are led to believe that the author of this documentation thinks ASCII 32 is printable, too.

About 'X' and 'Y', we can just say "the value of this capability may consist of ASCII printable characters (from 33 to 126 inclusive)" or something.

Is there a need for a registry of canonical os-version strings? One reason why you would want this user-settable (as opposed to being derived from "uname -s") is that a system that is presumably the same in end-user perception can call itself in different names (your Windows/MINGW64 example) and having the users set it to a string chosen from a small repertoire, the other end would be able to identify them more easily. I do not think it is a necessarily a good idea to limit what value the users can set to this configuration variable, but at least with a published guideline on calling various types of systems (and an explanation on the reason why we publish such a guideline), users would make an informed decision when picking what string to send.

Show 7 quoted lines

> +# Trim and replace each character with ascii code below 32 or above
> +# 127 (included) using a dot '.' character.
> +# Octal intervals \001-\040 and \177-\377
> +# corresponds to decimal intervals 1-32 and 127-255
> +test_redact_non_printables () {
> +    tr -d "\n" | tr "[\001-\040][\177-\377]" "."
> +}

Just being curious. Do we need to worry about carriage-returns not just line-feeds, and if not why?

Thanks.