git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 3/5] tests: explicitly skip `chmod` calls on Windows
Date: Tue, 23 Aug 2022 09:34:42 +0200 (CEST)	[thread overview]
Message-ID: <2553p8p9-q04r-75r6-5spn-8n9npno6q8p3@tzk.qr> (raw)
In-Reply-To: <p79ppo86-022r-7sss-41q1-39n499n3266s@tzk.qr>

[-- Attachment #1: Type: text/plain, Size: 3286 bytes --]

Hi Ævar,

On Mon, 22 Aug 2022, Johannes Schindelin wrote:

> Instead of wasting the run time (even on non-Windows platforms!) to
> determine whether the `MINGW` prereq is set in order to skip the `chmod`
> call or not, we can make `chmod` a no-op explicitly in that `case
> $uname_s` block.
>
> I will make it so.

tl;dr this patch needs to be dropped, without the suggested replacement.

Gaaah!

After struggling with this for much longer than I care to admit, and even
debugging inside the MSYS2 runtime (which is a level boss if there ever
was one), I found out that all about this patch was wrong.

The suggested patch (making `chmod` a no-op) was wrong: the test suite
started failing left and right. Why? Because `chmod` is not a _complete_
no-op. The test suite does not use the `-o` flag you suggested (which
would not make sense in Git's test suite, whether or not you remove file
permissions for "others" than the current user or group, Git behaves
identically), but it _does_ use the `-w`/`+w` flags, and those _are_
respected, even on Windows.

The way the write permission bits are translated to ACLs is admittedly
somewhat magical or all kinds of wrong, too, depending on your point of
view.

But my original patch was also wrong. Why? Because it claimed that `chmod
+x` does not work outside of MSYS2's pseudo Unix root directory tree. And
that's simply not true.

Whether you call `chmod +x C:/Users/avar/my-file.txt` or `chmod +x
/tmp/avar.sh`, it "succeeds" (by silently ignoring the flag that is
inapplicable on Windows, whether or not a file is executable is determined
by its file extension, and yes, that means that shell scripts are never
executable and we have to live with a nasty hack in Git to pretend that
they are, based on their contents starting with a hash-bang line).

So why did I claim that the `chmod +x` invocation does not work outside of
that pseudo Unix root directory tree? Because it actually did not work, at
least for me! But it worked on the build agents. Why? Because I have a
command in my `~/.profile` that mounts several of my Git for Windows SDKs
as "short-cuts" like `/sdk64`, `/sdk32` and the likes (think of these as
bind-mounts). And those `mount` commands did not specify the `noacl` flag.

So what's that `noacl` flag? It is magic, I can tell you. It basically
makes all this pretense work where MSYS2 pretends that we are in a Unix
environment with Unix permissions when we're not actually. The details are
too gnarly and involved to explain, I won't write them up here in order to
avoid putting even more readers to sleep than I must have done already.

So my analysis was based on observation instead of inspection, cutting
corners, and I dearly regret that now. By default, MSYS2 "mounts" the
system drives as `/c`, `/d`, etc with that `noacl` flag, the same as it
mounts the pseudo Unix root as `/`. And that is why the Git test suite
does not throw up every time `chmod +x` is called. And it was a simple
pilot error on my part that caused it to fail on my system, and I did not
realize that the problem was confined to my system, and the bug was in my
`~/.profile` and not in Git's code base.

Working on Git for Windows is never boring.

Ciao,
Johannes

  reply	other threads:[~2022-08-23  7:34 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10 15:02 [PATCH 0/5] Some fixes and an improvement for using CTest on Windows Johannes Schindelin via GitGitGadget
2022-08-10 15:02 ` [PATCH 1/5] cmake: align CTest definition with Git's CI runs Johannes Schindelin via GitGitGadget
2022-08-10 17:48   ` Junio C Hamano
2022-08-16 10:11     ` Johannes Schindelin
2022-08-16 15:15       ` Junio C Hamano
2022-08-19 13:57         ` Johannes Schindelin
2022-08-11 11:18   ` Ævar Arnfjörð Bjarmason
2022-08-10 15:02 ` [PATCH 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-08-10 15:02 ` [PATCH 3/5] tests: explicitly skip `chmod` calls on Windows Johannes Schindelin via GitGitGadget
2022-08-11 11:22   ` Ævar Arnfjörð Bjarmason
2022-08-22 10:19     ` Johannes Schindelin
2022-08-23  7:34       ` Johannes Schindelin [this message]
2022-08-10 15:02 ` [PATCH 4/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-08-10 17:54   ` Junio C Hamano
2022-08-16  9:56     ` Johannes Schindelin
2022-08-16 15:10       ` Junio C Hamano
2022-08-19 14:52         ` Johannes Schindelin
2022-08-11 12:49   ` Phillip Wood
2022-08-16 10:00     ` Johannes Schindelin
2022-08-16 14:23       ` Phillip Wood
2022-08-19 14:07         ` Johannes Schindelin
2022-08-10 15:02 ` [PATCH 5/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-08-11 11:35   ` Ævar Arnfjörð Bjarmason
2022-10-18 14:02     ` Johannes Schindelin
2022-08-11 12:58   ` Phillip Wood
2022-08-16 10:09     ` Johannes Schindelin
2022-08-16 14:27       ` Phillip Wood
2022-08-23  8:30 ` [PATCH v2 0/5] Some fixes and an improvement for using CTest on Windows Johannes Schindelin via GitGitGadget
2022-08-23  8:30   ` [PATCH v2 1/5] cmake: make it easier to diagnose regressions in CTest runs Johannes Schindelin via GitGitGadget
2022-09-07 22:10     ` Victoria Dye
2022-10-18 14:02       ` Johannes Schindelin
2022-09-08  7:22     ` Ævar Arnfjörð Bjarmason
2022-09-28  6:55       ` Eric Sunshine
2022-08-23  8:31   ` [PATCH v2 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-08-23  8:31   ` [PATCH v2 3/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-08-23  8:31   ` [PATCH v2 4/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-09-08  7:39     ` Ævar Arnfjörð Bjarmason
2022-10-18 14:03       ` Johannes Schindelin
2022-10-18 15:09         ` Ævar Arnfjörð Bjarmason
2022-09-08 23:37     ` Victoria Dye
2022-09-08 23:42       ` Victoria Dye
2022-09-08 23:58       ` Junio C Hamano
2022-10-18 14:03       ` Johannes Schindelin
2022-08-23  8:31   ` [PATCH v2 5/5] cmake: increase time-out for a long-running test Johannes Schindelin via GitGitGadget
2022-09-08  7:34     ` Ævar Arnfjörð Bjarmason
2022-09-08 17:29       ` Victoria Dye
2022-09-08  3:51   ` [PATCH v2 0/5] Some fixes and an improvement for using CTest on Windows Victoria Dye
2022-10-18 10:59   ` [PATCH v3 " Johannes Schindelin via GitGitGadget
2022-10-18 10:59     ` [PATCH v3 1/5] cmake: make it easier to diagnose regressions in CTest runs Johannes Schindelin via GitGitGadget
2022-10-18 13:41       ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59     ` [PATCH v3 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-10-18 13:49       ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59     ` [PATCH v3 3/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-10-18 13:53       ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59     ` [PATCH v3 4/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-10-18 13:54       ` Ævar Arnfjörð Bjarmason
2022-10-18 14:21         ` Johannes Schindelin
2022-10-18 10:59     ` [PATCH v3 5/5] cmake: increase time-out for a long-running test Johannes Schindelin via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=2553p8p9-q04r-75r6-5spn-8n9npno6q8p3@tzk.qr \
    --to=johannes.schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).