Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 28, 2018, 20:21 UTC
Message-ID: <CAPig+cQ_j4OyBjsZHE8ZPBojqD7HhSEb14-CFY9qYfXX+dafpQ@mail.gmail.com>
In-Reply-To: <CAGZ79kYD+RC_Z7AH_uH9tZSDHhkikML4HkxUNGxkW8voHJ1i3A@mail.gmail.com>

On Wed, Mar 28, 2018 at 4:08 PM, Stefan Beller <sbeller@google.com> wrote:

Show 17 quoted lines

> On Wed, Mar 28, 2018 at 11:57 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> +test_expect_success 'moving the submodule does not break the superproject' '
>>> +       (
>>> +               cd addtest2 &&
>>> +
>>> +               mv repo repo.bak &&
>>> +               git submodule status >actual &&
>>> +               grep -e "^-" -e repo actual &&
>>> +
>>> +               mv repo.bak repo
>>
>> Should this "move back" be encapsulated in a test_when_finished?
>
> I thought about that, but decided against it for some reason as I was debating
> where to put the test_when_finished. I mostly saw those at the very beginning
> of a test and wondered if it can be called from within a subshell.
> (I'd not want to put it at the beginning but rather adjacent to the move.)

It looks like test_when_finished() shouldn't be used in a subshell. However, wouldn't the following be reasonable?

    mv addtest2/repo addtest2/repo.bak &&
    test_when_finished "mv addtest2/repo.bak addtest2/repo" &&
    (
        cd addtest2 &&
        git submodule status >actual &&
        grep -e "^-" -e repo actual
    )

Or, even simpler:

    mv addtest2/repo addtest2/repo.bak &&
    test_when_finished "mv addtest2/repo.bak addtest2/repo" &&
    git -C addtest2 submodule status >actual &&
    grep -e "^-" -e repo actual