Re: [PATCH 03/27] t1092: compare sparse-checkout to sparse-index

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc" <pclouds@gmail.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH 03/27] t1092: compare sparse-checkout to sparse-index
Date: Wed, 27 Jan 2021 08:54:31 -0800	[thread overview]
Message-ID: <CABPp-BEjwaXXJggNDGT4fN2s+A4S-3xkNDRqc4RqM5K5bmZpaw@mail.gmail.com> (raw)
In-Reply-To: <d652eb83-808a-946e-37ef-9112a1790199@gmail.com>

On Wed, Jan 27, 2021 at 5:30 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 1/26/2021 10:08 PM, Elijah Newren wrote:
> > On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >>
> >> From: Derrick Stolee <dstolee@microsoft.com>
> >>
> >> Add a new 'sparse-index' repo alongside the 'full-checkout' and
> >> 'sparse-checkout' repos in t1092-sparse-checkout-compatibility.sh. Also
> >> add run_on_sparse and test_sparse_match helpers. These helpers will be
> >> used when the sparse index is implemented.
> >>
> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> >> ---
> >>  t/t1092-sparse-checkout-compatibility.sh | 29 ++++++++++++++++++++----
> >>  1 file changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> >> index 8cd3e5a8d22..8876eae0fe3 100755
> >> --- a/t/t1092-sparse-checkout-compatibility.sh
> >> +++ b/t/t1092-sparse-checkout-compatibility.sh
> >> @@ -7,6 +7,7 @@ test_description='compare full workdir to sparse workdir'
> >>  test_expect_success 'setup' '
> >>         git init initial-repo &&
> >>         (
> >> +               (GIT_TEST_SPARSE_INDEX=0 && export GIT_TEST_SPARSE_INDEX) &&
> >
> > I thought parentheses started a subshell; once the subshell ends,
> > wouldn't the setting of GIT_TEST_SPARSE_INDEX be thrown away?
>
> I think the "export" specifically pushes the setting out of the
> first level of subshell. This is the recommendation that comes up

You're having a child process change the environment variables of a
parent process? ...without some kind of gdb or other debugger
wizardry?

> if one runs
>
>         export GIT_TEST_SPARSE_INDEX=1 &&
>
> inside a test on macOS, since this isn't completely portable.

Um, I think you meant to run
      GIT_TEST_SPARSE_INDEX=0 &&
      export GIT_TEST_SPARSE_INDEX &&
in order to avoid the unportable
      export GIT_TEST_SPARSE_INDEX=0 &&
because
      (GIT_TEST_SPARSE_INDEX=0 &&
       export GIT_TEST_SPARSE_INDEX) &&
looks like a useless no-op.  At least it would be in normal bash; is
the test harness doing some special magic with it?  In normal bash,
the value definitely does NOT survive the subshell; (export just means
that subprocesses of the subshell where the environment variable is
set will see the value):

$ echo Before: $GIT_TEST_SPARSE_INDEX && (GIT_TEST_SPARSE_INDEX=0 &&
export GIT_TEST_SPARSE_INDEX) && echo After: $GIT_TEST_SPARSE_INDEX
Before:
After:

But in contrast, without the parentheses:
$ echo Before: $GIT_TEST_SPARSE_INDEX && GIT_TEST_SPARSE_INDEX=0 &&
export GIT_TEST_SPARSE_INDEX && echo After: $GIT_TEST_SPARSE_INDEX
Before:
After: 0