Re: different behavior on 3.82, 3.81, 3.80-and-older

[Brandon Casey]

On 09/06/2011 05:15 PM, Paul Smith wrote:
> On Tue, 2011-09-06 at 16:31 -0500, Brandon Casey wrote:
> 
>> all::
>>
>> SHELL_PATH = /bin/sh
>>
>> SHELL = $(SHELL_PATH)
>>
>> some_file: shell_compatibility_test FORCE
>>      @echo making some_file
>>      @$(SHELL) true
> 
> Are you sure you don't mean "$(SHELL) -c true" here?  The above will
> never succeed unless "true" is a shell script.

Yes, that's what I have

        @$(SHELL) -c true

sorry for that copy/paste typo.

>> -include some_file
>>
>> all:: shell_compatibility_test
>>      @echo building all
>>
>> please_set_SHELL_PATH_to_a_more_modern_shell:
>>      @echo testing shell $(SHELL)
>>      @$$(:)
>>
>> shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>>
>> .PHONY: all
>> .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
>> .PHONY: FORCE
> 
> That is an... interesting... approach.
> 
>> With make 3.81, make prints:
>>
>>    make: Nothing to be done for `all'.
>>
>> Since "all" is a PHONY target, its commands should always be executed
>> right?  Perhaps make is remembering the previous failure of
>> shell_compatibility_test and ignoring the second all:: target?
> 
> It's because you're using "-include some_file", and some_file depends on
> shell_compatibility_test, and shell_compatibility_test depends on
> please_set_SHELL_PATH_to_a_more_modern_shell.
> 
> Since the "some_file" target does not exist, make attempts to rebuild it
> so that it can be included; once the target is rebuilt then make will
> re-exec itself and read that file in, then continue on.
> 
> But, your commands fail and so the build of those target fails.
> 
> But, since you're running under "-include", where the "-" is telling
> make that when it tries to build the included makefile, it should ignore
> failures, no error is reported.
> 
> So make does ignore the failure and doesn't print anything.  Now make
> sees that none of the included makefiles were modified, so it does NOT
> re-exec itself and just continues on.
> 
> However make already knows it can't build those targets and so the next
> time you try to use that target make won't try to build it.
> 
> There's a good argument to be made that when make comes across a target
> which it tried and failed to build during the makefile rebuild phase, it
> should realize that the target failed the first time and treat it as a
> failure this time as well.
> 
> It looks like your makefile was depending on the behavior of some
> cobwebby corners of the GNU make feature set, and those corners have
> since been dusted.

But the "all" target is not optional and it has a failed dependency.
Shouldn't an error message be produced informing the user about the
failed dependency?  This is kind of moot since it is about 3.81.  It
appears that this has been improved in 3.82, since 3.82 at least
produces an error message, though it is a somewhat misleading one.

Here's another 3.81 twist that I just discovered, add another rule
for "all" after the others:

all::
        @echo empty all

Now, 3.81 goes into an infinite loop when called like this:

   make SHELL_PATH=/bin/false

The others (3.80 and 3.82) produce the same results as before.

>> On make 3.82, it prints:
>>
>>    make: *** No rule to make target 
>> `please_set_SHELL_PATH_to_a_more_modern_shell', needed by 
>> `shell_compatibility_test'.  Stop.
>>
>> ?? The rule exists, but somehow marked as missing because of a prevous
>> failure via the some_file target?
> 
> I don't get this message so I don't know why you're seeing that.

Are you calling make like this (sorry, I probably should have set
SHELL_PATH like this in my example makefile):

   make SHELL_PATH=/bin/false

if so, what result are you getting?

I downloaded the 3.82 tar ball and compiled on CentOS 5.6.  Possibly,
are you using a checkout from version control?

>> If I remove the dependency on shell_compatibility_test from the
>> some_file target
> 
> Now you've broken the link, so that make doesn't try to build
> shell_compatibility_test during the first phase (when it's trying to
> rebuild the makefiles).  So then when it tries to run that target during
> the normal builds, it will not have been run yet, and will fail.

Right.  I removed shell_compatibility_test as a dependency of some_file
to show that it affected the result.

My expectation is that the same error message will be produced
in either configuration since the build should halt at the same place
and for the same reason in both cases.

In either configuration, the build should halt because the
please_set_SHELL_PATH_to_a_more_modern_shell target failed to be built.

Currently, 3.82 produces the following message when shell_compatibility_test
is a dependency of some_file:

   make: *** No rule to make target 
`please_set_SHELL_PATH_to_a_more_modern_shell', needed by 
`shell_compatibility_test'.  Stop.

and it produces this next message when shell_compatibility_test is not
a dependency of some_file:

   make: *** [please_set_SHELL_PATH_to_a_more_modern_shell] Error 1

At least 3.82 produces some error message, 3.81, which still seems to be
commonly distributed (RHEL 6, ubuntu 11.04), doesn't give any visual
indication of failure.

> However, I suspect that this change will defeat the original intent of
> this bit of code.  Without any docs as to why things were done this way
> I can't be sure but my suspicion is that it's trying to force the build
> to fail immediately in the case of a "bad shell", regardless of which
> target was invoked.  But I could be wrong.

Yes, you read it correctly.  The intent is that the build should fail
when a "bad shell" is configured.  The make file itself has occurrences
of modern shell syntax like $() and others.  The name of the target is
meant as a hint to the user that a different shell should be configured.

-Brandon