Subject: Re: [PATCH v2] t/Makefile: remove 'test-results' on 'make clean'

From: Junio C Hamano <[email-masked]>
Date: Wed, 21 Sep 2022 10:49:48 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Tue, Sep 20 2022, SZEDER Gábor wrote:
>
>> The 't/test-results' directory and its contents are by-products of the
>> test process, so 'make clean' should remove them, but, alas, this has
>> been broken since fee65b194d (t/Makefile: don't remove test-results in
>> "clean-except-prove-cache", 2022-07-28).
>>
>> The 'clean' target in 't/Makefile' was not directly responsible for
>> removing the 'test-results' directory, but relied on its dependency
>> 'clean-except-prove-cache' to do that [1].  ee65b194d broke this,
>> because it only removed the 'rm -r test-results' command from the
>> 'clean-except-prove-cache' target instead of moving it to the 'clean'
>> target, resulting in stray 't/test-results' directories.
>>
>> Add that missing cleanup command to 't/Makefile', and to all
>> sub-Makefiles touched by that commit as well.
>>
>> [1] 60f26f6348 (t/Makefile: retain cache t/.prove across prove runs,
>>                 2012-05-02)
>>
>> Signed-off-by: SZEDER Gábor <[email-masked]>
>> ---
>
> Thanks, and sorry about the breakage. I've looked this over carefully &
> it fixes the edge-case you noted without making anything else that I
> could spot worse.
>
> In case it helps:
>
> 	Reviewed-by: Ævar Arnfjörð Bjarmason <[email-masked]>
>
> It's still a bit odd to have a "clean" that cleans up a thing it *might
> have* generated, i.e. sometimes we create & use these via a Makefile
> target, and sometimes by manually invoking test scripts.

I think this is perfectly fine, but we could also use "distclean"
for cleaning up something that might have been created.

Thanks, both.

Will queue.