From: Junio C Hamano Date: Wed, 21 Sep 2022 17:49:48 GMT Subject: Re: [PATCH v2] t/Makefile: remove 'test-results' on 'make clean' Message-ID: In-Reply-To: <220921.865yhh5hx0.gmgdl@evledraar.gmail.com> Ævar Arnfjörð Bjarmason 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 >> --- > > 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 > > 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.