Re: [PATCH v2] config: introduce an Operating System-specific `includeIf` condition

From: Samuel Ferencik <sferencik@gmail.com>
Date: Apr 17, 2023, 07:04 UTC
Message-ID: <CABwTEiRz+-+Zdx3Ed7O09Ch8GoXH-SnmJyc-vFOdF-hk_uO-yA@mail.gmail.com>
In-Reply-To: <xmqqk03jcwxz.fsf@gitster.g>

>>> Let's introduce a new condition: `os:<uname-s>` where `<uname-s>` is the
>>> system name, i.e. the output of `uname -s`.

The discussion about https://github.com/gitgitgadget/git/pull/1429 seems to have stalled on several points. I'll try to summarise; let's see if we can move forward.

(I am the reporter of https://github.com/git-for-windows/git/issues/4125, which led to this PR. I am vested in making progress here.)

1. name of the setting (`os` vs `uname-s` vs `sysname`)
   * dscho@ suggested `os`; Phillip and Philip suggested `uname-s` and
     `sysname`, respectively
   * I vote for `os`; I'm afraid perfect is the enemy of good here as
     * `man uname` says `-s` gives "the name of the operating system
       implementation"; no other `uname` switch comes closer to whatever
       concept "OS" represents
     * this is also correct on Windows (the "Windows" string) - see below
     * I find it extremely unlikely a future unforeseen git feature would have
       a better use for `includeIf os` (in parallel with `includeIf sysname`),
       i.e. I don't worry that we're squatting on a good name for a poor
       use-case
2. casing (use of `/i`)
   * dscho@ implemented case-insensitive comparison but without test coverage,
     documentation, and it's inconsistent with the other `includeIf` options
     that support the `/i` switch.
   * I propose that we compare case-sensitively because
     * no user can reasonably complain about this if the documentation is
       clear; the OS names are definitive and stable and it's not a big deal
       getting the case right for "Linux"
     * without the case insensitivity being documented, the users who [discover
       the insensitivity and] rely on it are risking breakage; plus the git
       maintainers are exposing themselves to the effects of Hyrum's law
       (https://xkcd.com/1172) - it's a disservice for both sides
     * this still allows us to add support for `/i` later (should a use-case
       emerge)
     * it is consistent with the other settings
     * it requires less code (incl. tests) and shorter documentation
3. handling Windows (MinGW, WSL)
   * As implemented currently, `includeIf "os:Windows"` would work in
     git-for-Windows. I think that's desirable, and no-one suggested otherwise.
   * In contrast, Philip points out `includeIf "os:Linux"` would be the way to
     match on WSL. Is that an issue? Do we want WSL to match "os:Windows" or
     "os:WSL"? As a Windows user, when I switch to WSL I do expect a "proper"
     Linux experience (unlike when I run "Git bash" on Windows, which is more
     like a port of utilities, but still Windows). I think this treating WLS as
     Linux is OK-ish, and we may get away with not discerning WSL. Thoughts?
4. documentation (w.r.t. the details in 1. - 3.)
   * We should document all of 1. - 3. I'm happy to give it a go if we can
     reach consensus.
   * Specifically, the documentation should mention that the OS string equals
     "Windows" in Git-for-Windows, and `$(uname -s)` otherwise; it should list
     examples, incl. "Linux" and "Darwin"; it should mention the case
     sensitivity.
5. tests (potential segfaults)
   * Johannes points out the tests hide segfaults. I haven't looked at this
     closely but hopefully Johannes's suggestion ("use test_cmp or
     test_cmp_config") is a clear enough pointer. I can try to fix this.
6. what's the use-case?
   * As the reporter of https://github.com/git-for-windows/git/issues/4125,
     here are my use-cases, i.e. settings that I currently set conditionally
     per OS (using `includeIf gitdir`):
     * different `difftool.path`, `mergetool.path` per OS (e.g. paths
       containing `C:\Program Files\...\...exe` vs Unix file paths)
     * different `merge.tool` per OS (I have a BeyondCompare license for Linux
       only)
     * different `core.autocrlf`: `true` on Windows, `input` otherwise
     * `core.whitespace` set to `cr-at-eol` on Windows
     * `core.editor` set to `gvim` on Windows
   * Note all of the use-cases above would cope with WSL reporting "Linux",
     except of `merge.tool`.

I hope this revives the discussion; I know it's been a while but I would appreciate your input. If possible, please refer to the numbered points (1 - 6) for clarity.

I'm happy to iterate on dscho@'s PR if we can reach consensus.

On Fri, 14 Apr 2023 at 01:44, Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

>
> Philip Oakley <philipoakley@iee.email> writes:
>
> >> Let's introduce a new condition: `os:<uname-s>` where `<uname-s>` is the
> >> system name, i.e. the output of `uname -s`.
> >
> > This `uname -s` doesn't work well on Git-for-Windows (GfW) / Windows
> > because GfW has its own internal compatibility code to spoof the result.
> > ...
> > Or just drop the mentions of "<uname-s>" in this commit message and
> > rename it 'sysname' to match the field of the struct utsname?
>
> FWIW I do not mind "sysname".  It is much better to say
>
>         [includeIf "sysname:Linux"] path = ...
>
> than "os:Linux", as "sysname" informs us the granularity used to
> identify the system better than "os".
>
>
>