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

From: Junio C Hamano <gitster@pobox.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>
Cc: Philippe Blain via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] t/README: mention test files are make targets
Date: Sun, 24 Mar 2024 18:24:26 -0700	[thread overview]
Message-ID: <xmqqfrwfp0hh.fsf@gitster.g> (raw)
In-Reply-To: <97212b72-70d6-e55f-8b2a-04ede4321288@gmail.com> (Philippe Blain's message of "Sun, 24 Mar 2024 13:04:34 -0400")

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.