git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Usman Akinyemi <usmanakinyemi202@gmail.com>
Cc: git@vger.kernel.org,  christian.couder@gmail.com,  ps@pks.im,
	johncai86@gmail.com,  Johannes.Schindelin@gmx.de,
	 me@ttaylorr.com, phillip.wood@dunelm.org.uk,
	 sunshine@sunshineco.com, rsbecker@nexbridge.com,
	 Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v2 6/6] version: introduce osversion.command config for os-version output
Date: Fri, 17 Jan 2025 14:33:49 -0800	[thread overview]
Message-ID: <xmqqwmethxyq.fsf@gitster.g> (raw)
In-Reply-To: <20250117104639.65608-7-usmanakinyemi202@gmail.com> (Usman Akinyemi's message of "Fri, 17 Jan 2025 16:16:18 +0530")

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> Let's introduce a new configuration option, `osversion.command`, to handle
> the string exchange between servers and clients. This option allows
> customization of the exchanged string by leveraging the output of the
> specified command. This customization might be especially useful on some
> quite uncommon platforms like NonStop where interesting OS information is
> available from other means than uname(2).

After reading the above rationale, I doubt the usefulness of this
feature even more.

Shouldn't that kind of anomalies be handled by compat/ layer to make
their uname(2) emulated, or allow get_uname_info() to be customized
at compile time by platform implementations, to yield more useful
pieces of information instead?

That way, we do not need to add another mechanism that lets people
spawn an arbitrary command while Git is running, we do not need to
worry about security implications, and we do not need to worry about
people abusing the facility to throw totally random and useless
garbage information at the other end to make their stats useless.

I'll skip the overly wide documentation changes.

> diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt
> ...
> diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt
> ...

> +test_expect_success 'test capability advertisement with osVersion.command config set' '
> +	test_config osVersion.command "uname -srvm" &&

If osversion.command configuration variable turns out to be
acceptable addition, I do not think we want to use "uname -srvm" as
its value for its test.  Do you know for sure how portable srvm is?

If you use something like "printf ' \001a\011b\015\012c '", you do
not even have to worry about how portable srvm is and on top, you
can test your unprintable-redacting logic in the code.

But all of that may be moot, if we take the "fewer customization at
runtime" approach.

Thanks.

  parent reply	other threads:[~2025-01-17 22:33 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-06 10:30 [PATCH 0/4][Outreachy] Introduce os-version Capability with Configurable Options Usman Akinyemi
2025-01-06 10:30 ` [PATCH 1/4] version: refactor redact_non_printables() Usman Akinyemi
2025-01-06 22:35   ` Eric Sunshine
2025-01-08 12:58     ` Usman Akinyemi
2025-01-06 10:30 ` [PATCH 2/4] version: refactor get_uname_info() Usman Akinyemi
2025-01-06 16:04   ` Junio C Hamano
2025-01-08 13:06     ` Usman Akinyemi
2025-01-06 10:30 ` [PATCH 3/4] connect: advertise OS version Usman Akinyemi
2025-01-06 16:22   ` Junio C Hamano
2025-01-08 13:06     ` Usman Akinyemi
2025-01-08 16:15       ` Junio C Hamano
2025-01-09 14:25         ` Usman Akinyemi
2025-01-09 15:46           ` Junio C Hamano
2025-01-10 17:56             ` Usman Akinyemi
2025-01-10 19:24               ` Junio C Hamano
2025-01-11 11:07                 ` Usman Akinyemi
2025-01-13 15:46                   ` Junio C Hamano
2025-01-13 18:26                     ` Usman Akinyemi
2025-01-13 19:47                       ` Junio C Hamano
2025-01-13 20:07                         ` rsbecker
2025-01-06 23:17   ` Eric Sunshine
2025-01-08 13:14     ` Usman Akinyemi
2025-01-06 10:30 ` [PATCH 4/4] version: introduce osversion.command config for os-version output Usman Akinyemi
2025-01-17 10:46 ` [PATCH v2 0/6][Outreachy] Introduce os-version Capability with Configurable Options Usman Akinyemi
2025-01-17 10:46   ` [PATCH v2 1/6] version: refactor redact_non_printables() Usman Akinyemi
2025-01-17 18:26     ` Junio C Hamano
2025-01-17 19:48       ` Junio C Hamano
2025-01-20 17:10       ` Usman Akinyemi
2025-01-21  8:12         ` Christian Couder
2025-01-21 18:01           ` Junio C Hamano
2025-01-17 10:46   ` [PATCH v2 2/6] version: refactor get_uname_info() Usman Akinyemi
2025-01-17 10:46   ` [PATCH v2 3/6] version: extend get_uname_info() to hide system details Usman Akinyemi
2025-01-17 18:27     ` Junio C Hamano
2025-01-17 10:46   ` [PATCH v2 4/6] t5701: add setup test to remove side-effect dependency Usman Akinyemi
2025-01-17 19:31     ` Junio C Hamano
2025-01-20 17:32       ` Usman Akinyemi
2025-01-20 19:52         ` Junio C Hamano
2025-01-21 13:43           ` Usman Akinyemi
2025-01-17 10:46   ` [PATCH v2 5/6] connect: advertise OS version Usman Akinyemi
2025-01-17 19:35     ` Junio C Hamano
2025-01-17 22:22     ` Junio C Hamano
2025-01-17 22:47       ` rsbecker
2025-01-17 23:04         ` Junio C Hamano
2025-01-20 18:15       ` Usman Akinyemi
2025-01-21 19:06         ` Junio C Hamano
2025-01-17 10:46   ` [PATCH v2 6/6] version: introduce osversion.command config for os-version output Usman Akinyemi
2025-01-17 21:44     ` Eric Sunshine
2025-01-20 18:17       ` Usman Akinyemi
2025-01-20 18:41         ` Eric Sunshine
2025-01-20 19:08           ` Usman Akinyemi
2025-01-17 22:33     ` Junio C Hamano [this message]
2025-01-17 22:49       ` rsbecker
2025-01-17 23:06         ` Junio C Hamano
2025-01-17 23:18           ` rsbecker
2025-01-20 18:58       ` Usman Akinyemi
2025-01-21 19:14         ` Junio C Hamano
2025-01-21 19:51           ` rsbecker
2025-01-24 12:21   ` [PATCH v3 0/6][Outreachy] Introduce os-version Capability with Configurable Options Usman Akinyemi
2025-01-24 12:21     ` [PATCH v3 1/6] version: replace manual ASCII checks with isprint() for clarity Usman Akinyemi
2025-01-24 18:13       ` Junio C Hamano
2025-01-24 12:21     ` [PATCH v3 2/6] version: refactor redact_non_printables() Usman Akinyemi
2025-01-24 12:21     ` [PATCH v3 3/6] version: refactor get_uname_info() Usman Akinyemi
2025-01-24 12:21     ` [PATCH v3 4/6] version: extend get_uname_info() to hide system details Usman Akinyemi
2025-01-24 12:21     ` [PATCH v3 5/6] t5701: add setup test to remove side-effect dependency Usman Akinyemi
2025-01-24 18:12       ` Junio C Hamano
2025-01-24 12:21     ` [PATCH v3 6/6] connect: advertise OS version Usman Akinyemi
2025-02-05 18:52       ` [PATCH v4 0/6][Outreachy] extend agent capability to include OS name Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 1/6] version: replace manual ASCII checks with isprint() for clarity Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 2/6] version: refactor redact_non_printables() Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 3/6] version: refactor get_uname_info() Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 4/6] version: extend get_uname_info() to hide system details Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 5/6] t5701: add setup test to remove side-effect dependency Usman Akinyemi
2025-02-05 18:52         ` [PATCH v4 6/6] agent: advertise OS name via agent capability Usman Akinyemi
2025-02-05 21:48           ` Junio C Hamano
2025-02-06  6:37             ` Usman Akinyemi
2025-02-06 15:13               ` Junio C Hamano
2025-02-07 17:27                 ` Usman Akinyemi
2025-02-07 17:57                   ` Junio C Hamano
2025-02-07 19:25             ` Usman Akinyemi
2025-02-14 12:36         ` [PATCH v5 0/6][Outreachy] extend agent capability to include OS name Usman Akinyemi
2025-02-14 12:36           ` [PATCH v5 1/6] version: replace manual ASCII checks with isprint() for clarity Usman Akinyemi
2025-02-14 12:36           ` [PATCH v5 2/6] version: refactor redact_non_printables() Usman Akinyemi
2025-02-14 12:36           ` [PATCH v5 3/6] version: refactor get_uname_info() Usman Akinyemi
2025-02-14 12:36           ` [PATCH v5 4/6] version: extend get_uname_info() to hide system details Usman Akinyemi
2025-02-14 12:36           ` [PATCH v5 5/6] t5701: add setup test to remove side-effect dependency Usman Akinyemi
2025-02-14 21:49             ` Junio C Hamano
2025-02-14 12:36           ` [PATCH v5 6/6] agent: advertise OS name via agent capability Usman Akinyemi
2025-02-14 22:07             ` Junio C Hamano
2025-02-15 15:29               ` Usman Akinyemi
2025-02-15 15:50           ` [PATCH v6 0/6][Outreachy] extend agent capability to include OS name Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 1/6] version: replace manual ASCII checks with isprint() for clarity Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 2/6] version: refactor redact_non_printables() Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 3/6] version: refactor get_uname_info() Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 4/6] version: extend get_uname_info() to hide system details Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 5/6] t5701: add setup test to remove side-effect dependency Usman Akinyemi
2025-02-15 15:50             ` [PATCH v6 6/6] agent: advertise OS name via agent capability Usman Akinyemi
2025-02-18 17:14               ` Junio C Hamano
2025-02-18 17:09             ` [PATCH v6 0/6][Outreachy] extend agent capability to include OS name Junio C Hamano
2025-01-24 18:39     ` [PATCH v3 0/6][Outreachy] Introduce os-version Capability with Configurable Options Junio C Hamano
2025-01-27 13:38       ` Christian Couder
2025-01-27 15:26         ` Junio C Hamano
2025-01-31 14:30           ` Christian Couder
2025-01-31 16:37             ` Junio C Hamano
2025-01-31 19:42               ` Usman Akinyemi
2025-01-31 20:15                 ` Junio C Hamano
2025-01-31 19:46               ` Usman Akinyemi
2025-01-31 20:17                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwmethxyq.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=rsbecker@nexbridge.com \
    --cc=sunshine@sunshineco.com \
    --cc=usmanakinyemi202@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).