All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Samuel Ferencik" <sferencik@gmail.com>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>
Subject: Re: [PATCH v2] config: introduce an Operating System-specific `includeIf` condition
Date: Wed, 19 Apr 2023 14:22:50 +0200 (CEST)	[thread overview]
Message-ID: <67a2d12c-6250-c4ee-dd26-fd8ecc71b8bc@gmx.de> (raw)
In-Reply-To: <xmqq7cuamjmu.fsf@gitster.g>

Hi Junio,

On Mon, 17 Apr 2023, Junio C Hamano wrote:

> Samuel Ferencik <sferencik@gmail.com> writes:
>
> >>>> 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`)
>
> I do not think it is a good idea to squat on too generic a name like
> 'os', especially when there are multiple layers people will care
> about.  But I think the original thread discussed this to death, and
> I do not see a point bringing it up again as the first bullet point.

Given what you said about "Operating System", i.e. that both "Ubuntu" and
"Linux" should be able to match at the same time, I kind of concur, but in
the other direction: rather than forcing the current patch series to use a
less intuitive (i.e. user-unfriendlier) name than `os`, I would want to
modify the patch series so that it _can_ match "Ubuntu" and "Linux".

> > 2. casing (use of `/i`)
>
> My preference is to do this case sensitively (in other words, start
> stupid) and if somebody wants to use "/i", add it later after the
> dust settles.

I strongly disagree with this. This feature is meant for Git users, who I
must assume based on my experience would expect the value to be
case-insensitive. I.e. they would expect both `linux` and `Linux` to
match. Let's not make this feature harder to use than necessary!

> > 3. handling Windows (MinGW, WSL)
>
> This comes back to the reason why "os" is a horrible choice.  Is WSL
> a Windows?  Is WSL a Linux?  The same question can be asked for Cygwin.

These questions actually have pretty obvious answers, with the exception
of WSL1 (which nobody should use anymore): WSL is a Linux, Cygwin is not
an Operating System by itself but runs on Windows. But of course that's
not helpful to help configure Git specifically in a Cygwin setup.

> The answer depends on which layer you care about.

Yes, this is the crucial bit.

> The underlying kernel and system may be Windows, and some
> characteristics of the underlying system may seep through the
> abstraction, but these systems aim to give user experience of something
> like GNU/Linux.
>
> And this is not limited to Windows.  There may be similar issue for
> systems like PacBSD.  Is it a Linux?  Is it a BSD?
>
> > 6. what's the use-case?
>
> I think that this is the most important question to ask, and from
> here, we'd see how #3 above should be resolved (I suspect that you
> may want to have at least two layers to allow WSL to be grouped
> together with MinGW and Cygwin at one level, and at the same time
> allow it to be grouped together with Ubuntu at a different level).
> And after we figure that out, we'll have a clear and intuitive
> answer to #1.

This is probably the most valuable feedback in this entire thread: What is
the problem we're trying to solve here?

The original report (which this patch tries to address) asks for a way to
have a user-wide ("global") Git configuration that can be shared across
machines and that allows for adapting to the various environments. The
rationale is motivated well e.g. in
https://medium.com/doing-things-right/platform-specific-gitconfigs-and-the-wonderful-includeif-7376cd44994d
where platform-specific credential managers, editors, diff highlighters
that work only in certain setups, and work vs personal environments are
mentioned.

So as long as Git offers ways to discern between the mentioned
environments by including environment-specific configuration files, we
solve the problem.

Bonus points if we can do that without getting ever deeper into a pretty
contentious discussion about naming.

The strategy chosen in above-mentioned article uses the presence of
certain directories as tell-tales for the Operating System in which
Git is called: Linux, macOS or Windows. Concretely, it suggests this:

	[includeIf "gitdir:/Users"]
		path = ~/.gitconfig-macos
	[includeIf "gitdir:C:"]
		path = ~/.gitconfig-windows
	[includeIf "gitdir:/home"]
		path = ~/.gitconfig-linux

Now, the presence of directories like `/home/` might work well to discern
Linux from macOS, but this is not the correct way to identify Linux in
general (a `/home/` directory exists in many Unices, too). And it only
works when the _gitdir_ is in those directories, too. That's why I thought
that Git could do better.

In many cases, though, the presence of directories is probably "good
enough" to address the need described in above-mentioned article.

What triggered me to write this here patch was the report that those
`/home/` and `/Users` paths in the Git config ran into the warning that
Git for Windows no longer treats paths starting with a slash as relative
to the runtime prefix. This warning was introduced when the `%(prefix)/`
feature was upstreamed, superseding Git for Windows' original handling of
paths starting with a slash. The warning was introduced in November '21
(virtually together with Git for Windows v2.34.0):
https://github.com/git-for-windows/git/commit/28fdfd8a41836f49666c65e82d92de9befea2e69

So while I still think that something like `includeIf.os:<name>` would
make for a better way to address the concern in question, I realize that
the path of least resistance is simply to drop the now-deprecated feature
from Git for Windows (including the warning that was the reason for the
original report): https://github.com/git-for-windows/git/pull/4389

This still leaves Git in the somewhat unsatisfying state where there is no
better way to discern between Operating Systems than to work around by
detecting the presence of certain directories. But I do not see any viable
way to reach consensus about the `includeIf.os:<name>` patch, so I'll
leave it at that.

Ciao,
Johannes

  parent reply	other threads:[~2023-04-19 12:23 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-21 13:39 [PATCH] config: introduce an Operating System-specific `includeIf` condition Johannes Schindelin via GitGitGadget
2022-11-21 13:51 ` Ævar Arnfjörð Bjarmason
2022-11-21 15:51   ` Phillip Wood
2022-11-21 19:18     ` Johannes Schindelin
2022-11-21 19:19 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2022-11-21 23:32   ` Jeff King
2022-11-23 11:54     ` Đoàn Trần Công Danh
2022-11-24  0:56       ` Jeff King
2022-11-22 14:01   ` Ævar Arnfjörð Bjarmason
2022-11-22 14:31     ` Phillip Wood
2022-11-23  0:16       ` Junio C Hamano
2022-11-23 15:07         ` Phillip Wood
2022-11-23 23:51           ` Junio C Hamano
2022-11-22 18:40   ` Philippe Blain
2022-11-23 10:40   ` Philip Oakley
2022-11-25  7:31     ` Junio C Hamano
2023-04-17  7:04       ` Samuel Ferencik
2023-04-17 18:46         ` Junio C Hamano
2023-04-18  2:04           ` Felipe Contreras
2023-04-19 12:22           ` Johannes Schindelin [this message]
2023-04-19 14:26             ` Chris Torek
2023-04-19 14:32               ` Samuel Ferencik
2023-04-19 15:21             ` rsbecker
2023-04-19 16:07             ` Junio C Hamano
2023-04-19 16:14             ` Junio C Hamano
2022-11-22  0:03 ` [PATCH] " 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=67a2d12c-6250-c4ee-dd26-fd8ecc71b8bc@gmx.de \
    --to=johannes.schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.email \
    --cc=phillip.wood123@gmail.com \
    --cc=sferencik@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.