Re: [PATCH 04/17] Makefile: a dry-run can error out if no perl. Document the issue
- From
Philip Oakley <philipoakley@iee.org>
- Date
- Jun 25, 2015, 19:33 UTC
- Message-ID
- <DBFC4D88D6F34023BA43F5F10134331E@PhilipOakley>
- In-Reply-To
- <xmqq8ub724ls.fsf@gitster.dls.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
Show 12 quoted lines
> Philip Oakley <philipoakley@iee.org> writes: > >> A 'make -n' dry-run is used as part of the /compat/vcbuild and >> /contrib/buildsystems code. The commit ee9be06 (perl: detect new >> files >> in MakeMaker builds, 2012-07-27) was not aware of that dry-run usage >> and thus would not execute the target. >> >> Add a comment to the make file stating the issue and the available >> solutions of either NO_PERL or a '+recipe'. > > I am not sure what this patch is trying to achieve.
It was probably a bit of 'don't mess with working code', given that I'd used the NO_PERL solution, rather 'document the issue'. The issue had me stuck for a while. The possible use of '+' came later when driving down through the Makefile manual.
Show 13 quoted lines
> > If you are not touching what this Makefile actually does, then I > would imagine that you are running the "buildsystems code" that > actually drives 'make -n' with some "available solution" (perhaps > you are running 'make -n NO_PERL=NoThanks' or something like that)? > Then instead of a command like this that nobody would read in this > file, the same command can instead go there to explain what the > "workaround" (e.g. unusual-looking 'make -n NO_PERL=NoThanks') is > doing? > > I suspect you mean by +recipe that you modify this makefile to make > such a workaround unnecessary? If that is the case, why isn't such > a change actually be done with this commit, instead of a comment?
Yes. See above about the lack of 'fearless coding'; been working in real world engineering too long ;-)
Show 37 quoted lines
>
> I am not sure what this patch is trying to achieve.
>
> Puzzled...
>
>>
>> Signed-off-by: Philip Oakley <philipoakley@iee.org>
>> ---
>> Makefile | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index 149f1c7..22108bb 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1746,6 +1746,9 @@ $(SCRIPT_PERL_GEN): perl/perl.mak
>>
>> perl/perl.mak: perl/PM.stamp
>>
>> +# 'make -n' (dry-run) will not execute this target which
>> creates/updates the PM.stamp file.
>> +# To avoid the error of failing to find the target PM.stamp, either
>> use NO_PERL=1 (YesPlease),
>> +# or add a leading '+' to the recipe '+$(QUIET_GEN)$(FIND) perl ...'
>> so that it is executed.
>> perl/PM.stamp: FORCE
>> @$(FIND) perl -type f -name '*.pm' | sort >$@+ && \
>> { cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \
>> --
>> 2.3.1
>>
>> --
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>