Re: [PATCH] t7900: use pwd -P in macOS maintenance test

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 23, 2025, 21:36 UTC
Message-ID: <CAPig+cQXdhT2R2_D7pYBRgCUPLtbmEK+ta+dd9EByk2i3yBkaw@mail.gmail.com>
In-Reply-To: <88e8231f-7c08-652d-f734-22c4632aa728@chromium.org>

On Fri, May 23, 2025 at 4:43 PM Mark Mentovai <mark@chromium.org> wrote:

Show 13 quoted lines

> Eric Sunshine wrote:
> >    # t/test-lib.sh
> >    ...
> >    # git sees Windows-style pwd
> >    pwd () {
> >        builtin pwd -W
> >    }
>
> That MinGW fallback pwd ignores arguments, so any pwd in a test regardless
> of whether it's specified as pwd or pwd -P will result in an underlying
> pwd -W. The t7900 test's behavior should not change as a result of this
> patch. If it's succeeding in some MinGW environment before this patch,
> it'll continue to succeed after.

You're right. I (stupidly) overlooked that the `pwd` function ignores its arguments.

Show 13 quoted lines

> > [*]: In the long run, a better fix would probably be for the tests to
> > sanitize the output of the Git command, replacing (via `sed`) the
> > actual emitted path with some placeholder, such as "%HOME%" or
> > something, and then have the tests look for (`grep` or whatnot)
> > needles using that literal placeholder rather than trying to perfectly
> > match the path emitted by Git. This approach makes sense since these
> > tests are about overall functionality of git-maintenance, not about
> > the specific path in which the person happens to be running the tests.
>
> The specific front of the path is not important, but the tail should be as
> expected, and I suspect that it remains much less fragile and complex to
> perform this equality comparison than it would be to try to reason about
> the path's inner components.

`sed` in my suggestion was an afterthought. What I really had in mind was perhaps augmenting the value of the GIT_TEST_MAINT_SCHEDULER environment variable, which git-maintenance already specially recognizes to tweak its behavior when being run by a test, such that it instead emits the placeholder (literal "%HOME%" or whatever) as prefix of the path rather than the actual prefix. Tests could then just employ the same placeholder prefix rather than trying to exactly match the real path prefix.

Such a change is, of course, well outside the scope of the patch under discussion (hence my phrasing "in the long run"). The minimal fix you presented should be perfectly satisfactory.