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

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

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> +
> +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`?

> +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.

> +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.

> +# 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.