From: Eric Sunshine <sunshine@sunshineco.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Git List <git@vger.kernel.org>, Derrick Stolee <stolee@gmail.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2 0/2] make macOS `git maintenance` test work on Windows
Date: Mon, 30 Nov 2020 22:58:36 -0500 [thread overview]
Message-ID: <CAPig+cT-axeG3UrxaQR3c+r7dMZkjmS19C3cHRVzu5KSc72BOQ@mail.gmail.com> (raw)
In-Reply-To: <87blffjkne.fsf@evledraar.gmail.com>
On Mon, Nov 30, 2020 at 4:20 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> LGTM. FWIW I think your v1 is fine too, just meant to comment on the
> basis of "you could also do it like that". Having a C program call
> getuid() is fine, so is faking it. If you prefer the latter, cool.
I do like the simplicity of the latter, and I wasn't super happy about
introducing a new test-tool subcommand just to make this one test
pass, especially since `test-tool getuid` is so different from most
other subcommands which are typically added to give us access to some
internal element of Git otherwise not accessible to the test scripts.
> I did wonder "why not just call perl -e 'print $<' ?" first. But then
> found (by reading the Perl source[1], didn't actually test this) that it
> fakes up UID=0 for everything on Windows.
I totally forgot about Perl's `$<`.
Under the Git for Windows SDK, Perl's `$<` returns a large positive
number. I suspect this differs from what you saw in the Perl source
code because the Windows-specific code you looked at does not come
into play in this case. For Git for Windows SDK, Perl is almost
certainly instead built in Unix-like mode, linking against the MSYS2
library for its POSIX-like emulation, thus the Windows-specific Perl
goop is not needed.
> I couldn't find any "is root?" in our tree that relies on Perl's $< in a
> bad way (the couuple of occurances seem innocuous), we have some "id -u"
> checks, but those also seem OK if it returned 0 on Windows (what does it
> return?). Seems the worst we'd do there is unintentionally skip some
> "skip this as root" tests.
Under Git for Windows SDK, `id -u` returns the same large positive
number as returned by Perl's `$<`, which makes sense since `id` is
also likely linked against the MSYS2 library.
As for getuid() in Git itself, that always returns 1. I see now that's
because 1 is hard-coded in Git's compat/mingw.h override of getuid().
So, an alternative would have been for the test to use `uid=1` on
MINGW, but I like the current approach better of having the test be
UID-agnostic.
> It's also my impression that just using $("$PERL_PATH" -e ...) is fine,
> and at least to my reading the Perl RX is more readable with look-behind
> assertions, but I'm biased by familiarity with it.
The `sed` version seems simpler and more straightforward to me,
whereas look-behind feels harder to reason about, but of course it's
all subjective and not terribly important. Either would be fine.
> Our PERL prereq & NO_PERL=YesPlease is just for "this may require a
> non-ancient Perl" & "don't install Perl for runtime stuff"
> respectively. Is that not the case and we'd like to avoid new perl
> invocations where possible?
>
> I don't really care either way (or, if your switch in this case was just
> a personal preference, also fine), but if we are trying to somewhat
> discourage perl use (and maybe eventually get rid of it entirely) that
> would be a useful t/README doc update.
Using `sed` was just a personal preference, partly because the `sed`
expression seems simpler to me, but mostly because Perl still feels
heavyweight to me compared to `sed`. It also may be that I'm just
old-school, preferring the small, sharp utilities (`sed`, `grep`,
`sort`, etc.), and only turning to Perl (or one of its ilk) when the
task demands a more general-purpose tool.
I haven't measured, but it's possible that Perl may indeed be
heavyweight on Windows in terms of startup time.
> I know Johannes (CC'd) has (this is from wetware memory) wanted to
> (understandably) not need to bother with Perl as part of GFW, but I
> can't remember if that's for a reason covered by NO_PERL=YesPlease,
> i.e. packaging it up, or whether it's also to not wanting to provide a
> perl for the test suite.
I _think_ that was for the NO_PERL=YesPlease case, but I expect Dscho
can answer more concretely.
next prev parent reply other threads:[~2020-12-01 4:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 4:42 [PATCH v2 0/2] make macOS `git maintenance` test work on Windows Eric Sunshine
2020-11-30 4:42 ` [PATCH v2 1/2] t7900: fix test failures when invoked individually via --run Eric Sunshine
2020-11-30 4:42 ` [PATCH v2 2/2] t7900: make macOS-specific test work on Windows Eric Sunshine
2020-11-30 13:12 ` Derrick Stolee
2020-12-01 3:03 ` Eric Sunshine
2020-11-30 9:20 ` [PATCH v2 0/2] make macOS `git maintenance` " Ævar Arnfjörð Bjarmason
2020-11-30 23:32 ` Junio C Hamano
2020-12-01 3:58 ` Eric Sunshine [this message]
2020-12-01 12:31 ` Johannes Schindelin
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=CAPig+cT-axeG3UrxaQR3c+r7dMZkjmS19C3cHRVzu5KSc72BOQ@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=stolee@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).