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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

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

> Samuel Ferencik <sferencik@xxxxxxxxx> 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux