Re: [PATCH] t/README: mention test files are make targets

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2024, 01:24 UTC
Message-ID: <xmqqfrwfp0hh.fsf@gitster.g>
In-Reply-To: <97212b72-70d6-e55f-8b2a-04ede4321288@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> Since '$(T)' is listed at the bottom of the Makefile as .PHONY,
> I think it is OK and not that ugly since this uses a documented feature
> of make.

You're prehaps right.

I've always felt that the documented .PHONY feature was to mark targets that do not correspond to any filename on the filesystem, e.g., "all", "clean", "install". Of course these can exist as filenames as well, and .PHONY works as an instruction that says "the existence or freshness of these targets do not matter at all". For our use, it is OK.

I however wonder if marking $(T) as .PHONY is the right thing to begin with. Declaring that the existence or freshness of t0000-basic.sh does not matter means we will not be able to later write rules other than "just run it!" that does depend on the freshness of t0000-basic.sh file, no? IOW, if we wanted to add a target like this at the end of the t/Makefile:

        t-combined.sh: t0000-basic.sh t0001-init.sh
                cat t0000-basic.sh t0001-init.sh >"$@"

then "make -C t t-combined.sh" would end up running these two test scripts (because they are .PHONY) and then leave the concatenation in t-combined.sh file. Without changing anything, doing the same "make -C t t-combined.sh" again will again run these two tests and recreate the same t-combined.sh file, even though there is no need to.

So I think that is what I felt ugly.

As long as we do not use these $(T) files as an input to some other thing and list them as the dependencies, we are OK, though.