[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #37, bug #48643 (project make):

> The first one is that the output appears even if make ultimately decides
that there is no way to build the target.

i think, this is good. Hopefully, this will prevent users from writing
makefiles that depend on compat search.


> When I put my extra warning in I get this output:
> 
> 
> make: Detected a compatibility rule to build 'hello.z'.
> make: Using compatibility rule '%.z:: %.x' due to unrelated 'hello.x'.
> make: *** No rule to make target 'hello.x', needed by 'hello.z'.  Stop.
> 

i think, "Detected a compatibility rule..." message is problematic.

> Although I didn't find too many instances of this in my tests I know they
are actually quite common and this makes me concerned that adding this warning
will cause lots of warnings in real makefiles.

i think, make should issue this warning only when two conditions are
satisfied
1. No implicit rule is found.
and
2. There is an unrelated rule which mentiones the implicit prerequisite
explicitly.

Even though the 2nd condition is common, the 1st is not that common. Most of
makefiles (as far as i can tell) have proper rules. For those proper makefiles
make will fail to find the rule only if the source file is missing, vpath is
messed up or a bug was introduced to the makefile.
All those who reported after the 2009 fix, presented some legal, but
convoluted makefile, such as examples 3 and 5 or this test from
features/double_colon. These makefiles have the prerequisite mentioned
explicitly on an unrelated rule to cause make to cut implicit search short and
then rely on either default rule or a match anything rule to build the target.
make will issue the warning for such makefiles only. Makefiles which have
proper rules should not cause make to issue the warning. Your testing
described in update 30 found no such convoluted examples.

The "Detected a compatibility rule..." can be quite common, that's the
condition 2. That's why, i suggest, make does not issue "Detected a
compatibility rule..." message.

i think, make should tell the user that no implicit rule is found. Otherwise,
"Using compatibility rule..." looks like an neutral message and does not look
like make is warning about something bad.


> It also may well mean that we won't ever be able to remove the compatibility
search at all, because without this we don't get the above error at all:
instead we just get a successful "Nothing to be done for 'all'."

Only if the target exists and there is no proper (the one that does not depend
on its implicit prerequisite mentioned explicitly somewhere else) rule.


___

Reply to this item at:

  

___
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #36, bug #48643 (project make):

I played for a while this weekend with adding a warning for compatibility
mode, but I'm not sure we can really do it.  There are two problems which may
be related.

The first one is that the output appears even if make ultimately decides that
there is no way to build the target.  For example, the last test in
features/double_colon fails: it uses this makefile:


all: hello.z
%.z:: %.x; touch $@
%.x: ;
unrelated: hello.x


The expected output is:


make: *** No rule to make target 'hello.x', needed by 'hello.z'.  Stop.


When I put my extra warning in I get this output:


make: Detected a compatibility rule to build 'hello.z'.
make: Using compatibility rule '%.z:: %.x' due to unrelated 'hello.x'.
make: *** No rule to make target 'hello.x', needed by 'hello.z'.  Stop.


It seems odd to warn about finding compatibility rules if we then end up not
being able to build anyway, but there's no way to know, when we detect the
compatibility rule, whether we will succeed or not.  But, maybe this isn't a
problem since it may be that the warnings help us understand why make decided
to follow that path and get that error.

However, the larger problem can be seen with this exact same example: as you
mentioned in comment #33 the above makefile form is actually a very common way
to prevent make from treating a prerequisite as an intermediate file
(mentioning it in a different rule).  Although I didn't find too many
instances of this in my tests I know they are actually quite common and this
makes me concerned that adding this warning will cause lots of warnings in
real makefiles.  It also may well mean that we won't ever be able to remove
the compatibility search at all, because without this we don't get the above
error at all: instead we just get a successful "Nothing to be done for
'all'."

Of course, the user could add an explicit *hello.z: hello.x* to fix this
problem.  But, I'm not sure we can add this requirement.

Anyway that is something to consider for the future.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #35, bug #48643 (project make):

> Do you mean that if I apply the test patch mentioned there, some of those
tests would fail?

All tests should pass.
in the case of example 6 make fails to set pat->is_explicit for 'hello.x'.
This used to fail example 6 until commit 6682fb. My concern is that
pat->is_explicit still has incorrect value and that will manifest itself
someday.



> Regarding your question in comment #33, the current behavior is correct.

glad to hear that.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #34, bug #48643 (project make):

I didn't fully grasp the impact of the issue you raised in comment #31.  Do
you mean that if I apply the test patch mentioned there, some of those tests
would fail?  I will try to look at this more this evening.

Regarding your question in comment #33, the current behavior is correct.  It
has always been the case that any mention of a target in the makefile is
sufficient to remove the "intermediate-ness" of a file.  In fact, this is a
very common, and very often recommended, way to remove intermediate status of
a file; we suggest adding a separate target like:


build-intermediates: 


and even if that target is never invoked, it's sufficient to mark those
targets as "mentioned in" the makefile and thus remove their intermediate
status.

Changing this would break a lot of things.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #33, bug #48643 (project make):

This fix turned out to be difficult. i'll be surprised if we don't miss
anything.

i wanted to bring to your attention a couple of leftovers.
One is that pat->is_explicit that is described in update 31.

For the other let's consider example 8.


all: hello.tsk
%.tsk: %.o; touch $@
%.o: %.c; $(info $@)
#unrelated: hello.o


Here, 'hello.o' is intermediate and 'hello.o' and 'hello.tsk' are not rebuild
when 'hello.o' is missing.
If we uncomment rule 'unrelated: hello.o', then 'hello.o' is explicit and
'hello.o' and 'hello.tsk' are rebuild when 'hello.o' is missing.

So, even though rule 'unrelated: hello.o' is unrelated and no longer
participates in overload resolution, presence of this rule affects make's
behavior.

The tests in sv48643_exp_preqreq_is_not_interm_tests.diff all test that such
prerequisite explicitly mentioned on an unrelated rule is not intermediate.
i'd like to solicit your opinion on this.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #32, bug #48643 (project make):

There are too many patches attached to this bug: I lost track of which had
been applied and which hadn't and I guess I missed that one; sorry about
that!

The change in 6682fb was discussed in this email to bug-make:
https://lists.gnu.org/archive/html/bug-make/2021-12/msg00077.html

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #31, bug #48643 (project make):

Paul, in update 27 i added 2 patches. sv48643_exp_preqreq_is_not_interm.diff
has a fix for example 6. sv48643_exp_preqreq_is_not_interm_tests.diff has
related tests.

Not sure, if you decided not to apply these 2 patches or they fell through the
cracks.

You introduced another change in commit 6682fb. This change in commit 6682fb
causes example 6 to succeed even when sv48643_exp_preqreq_is_not_interm.diff
is not applied.

However, without sv48643_exp_preqreq_is_not_interm.diff
pat->is_explicit has incorrect value of 0 in cases like example 6. This
incorrect value of pat->is_explicit now goes unnoticed with this new
assigment
f->is_explicit |= imf->is_explicit || pat->is_explicit;
, because f->is_explicit is set in enter_prereqs.

My concern is that this incorrect value of pat->is_explicit can manifest
itself in some other scenario or after a change is introduced. For example,
this incorrect value is assigned to
dep->is_explicit = pat->is_explicit;
a few lines below.



___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #30, bug #48643 (project make):

> Let us list the options
> 1. keep compat search enabled by default and silent.
> 2. introduce a warning message when make begins compat search.
> 3. introduce a flag to disable compat search.
> 4. have compat disabled by default and introduce a flag to enable it.

I added #2 to a local build and ran through a suite of different kinds of open
source software that I build locally, including packages that are built with
home-grown makefiles, automake-generated makefiles, and cmake-generated
makefiles.  I didn't test a Linux kernel build however.  I should probably add
that to my test process.

I didn't see any instances of this warning appear.  So hopefully that suggests
that adding this would not result in a tsunami of warnings.

I'm tempted to add this warning for the release candidates process, and see
whether we get complaints.  Of course we always run into issues after the
release that were not caught during the RC process but...

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #29, bug #48643 (project make):

Thanks Dmitry.  I applied these patches.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Additional Item Attachment, bug #48643 (project make):

File name: sv48643_preserve_target_specific_vars_of_interm_test.diff Size:2 KB
   


File name: sv48643_preserve_target_specific_vars_of_interm.diff Size:1 KB
   




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #28, bug #48643 (project make):

Let us consider Example 7.


all: hello.tsk
%.tsk: hello.x; $(info $@)
%.x:; $(flags)
hello.x: flags:=true


Here 'hello.x' has a target specific variable 'flags' with value 'true'.
Setting a target specific variable causes make to enter 'hello.x' to the
database as a prerequisite. Implicit search then finds 'hello.x' in the
database and knows that 'hello.x' is mentioned explicitly.

make-4.3 would at that point consider 'hello.'x as ought-to-exist.
master figures out that 'hello.x' is not a target and is not a prerequisite of
the current target 'hello.tsk'. master therefore decides that 'hello.x' does
not qualify as ought-to-exit. master proceeds and finds that 'hello.x' can be
built as an intermediate by rule '%.x:;'.

At that point make needs to init the file in the database from the
intermediate (cmds, deps, etc) and it incorrectly wipes out the target
specific variables of the file already stored in the database.

Previously this didn't not matter, because 4.3 treats 'hello.x' as
ought-to-exist and does not even attempt to built it as an intermediate.
Now this matters.
Please find a fix in the attached patch
sv48643_preserve_target_specific_vars_of_interm.diff
and related tests in
sv48643_preserve_target_specific_vars_of_interm_test.diff.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Additional Item Attachment, bug #48643 (project make):

File name: sv48643_exp_preqreq_is_not_interm_tests.diff Size:4 KB
   


File name: sv48643_exp_preqreq_is_not_interm.diff Size:1 KB
   




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #27, bug #48643 (project make):

i figured this change is still incomplete. There is a case when make marks
this almost-ought-to-exist prerequisite as intermediate incorrectly.

Let us say hello.tsk exists and hello.x is missing and consider

Example 6.


all: hello.tsk
%.tsk: %.x; touch hello.tsk
%.x: ;
unrelated: hello.x


make-4.3 does the following
1. Tries rule '%.tsk: %.x' for 'hello.tsk'.
2. Tries prerequisite 'hello.x'.
3. Notices that 'hello.x' is mentioned explicitly on an unrelated rule.
4. Considers 'hello.x' ought-to-exist.
5. Chooses rule '%.tsk: %.x' for 'hello.tsk'. Notice, make-4.3 has not tried
'hello.x' as an intermediate and thus 'hello.x' does not have intermediate
flag set.
6. Remakes 'hello.tsk', because 'hello.x' is not intermediate.

master does the following
1. Tries rule '%.tsk: %.x' for 'hello.tsk'.
2. Tries prerequisite 'hello.x'.
3. Notices that 'hello.x' is mentioned explicitly on an unrelated rule.
4. Considers that 'hello.x' does not qualify as ought-to-exist.
5. Tries 'hello.x' as an intermediate.
6. Chooses rule '%.x' for 'hello.x'.
7. Sets intermediate flag for 'hello.x'.
8. Chooses rule '%.tsk: %.x' for 'hello.tsk'.
9. Fails to remake 'hello.tsk', because 'hello.x' is intermediate.

Please find a fix in sv48643_exp_preqreq_is_not_interm.diff and tests in
sv48643_exp_preqreq_is_not_interm_tests.diff.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #26, bug #48643 (project make):

The reason to include a flag for option #4 is that users may have newer
versions of GNU make (installed on their distros) but not be able to modify
all the makefiles they are using to address their problems (older software
downloaded from somewhere... or they just can't fix it for Reasons(tm)).

However I don't want to do 3 or 4 immediately: I'd prefer to give some time
for people to address issues.

I'll do a build of a collection of F/OSS I have here with a version of GNU
make with #2 installed and see what I get.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #25, bug #48643 (project make):

Let us list the options
1. keep compat search enabled by default and silent.
2. introduce a warning message when make begins compat search.
3. introduce a flag to disable compat search.
4. have compat disabled by default and introduce a flag to enable it.

1 is the current state of the code in master. i think, this is releasable.

2 is the same as 1 plus a warning message. i think, this message a good hint
and hopefully will drive users away from compat search.

3 requires that users specify the flag. If the flag is specified in MAKEFLAGS,
then it will be inherited by recursive makes, which may or may not be
desired.

4 is backward incompatible. This choice forces the users to modify the
makefiles that depend on the old definition of ought-to-exist. Given that the
users have to modify the makefile, they can as well fix the rules to avoid
this dependency on the old def of ought-to-exist. This ability to fix the
rules makes a flag to enable compat-search redundant.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #24, bug #48643 (project make):

If we decide to do this I'm not sure it should be a command line flag: command
line flags should be limited to modifying how a given user wants their build
to run.  It shouldn't modify fundamental ways that a makefile is parsed.

The author of the makefile should set some value in the makefile itself, if
they want to modify the way in which rules are searched or applied, because
that's a requirement of the makefile.

Of course that's the problem with backward-incompat: it needs to be an option
so users with newer versions of GNU make can use older makefiles that haven't
been updated.  I guess we could allow MAKEFLAGS += --backward-compat to set
it.  For sure we'd need to add a new .FEATURE value for this.

What I'd really like to see is a cross-section of builds for popular free
software packages that have this warning enabled to see how many times it's
actually hit.  Obviously that's not definitive but it would be useful to know.
 I have a setup that builds a bunch of things, maybe I'll try it.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #23, bug #48643 (project make):

Yet another option is to have compatibility search disabled by default and
introduce flag '--enable-compatibility-search'.
This is a good choice going forward, because we really don't want new
makefiles to rely on compatibility search.
This choice will require those users who rely on compatibility search to
either modify the makefile to not rely on compatibility search or to add
--enable-compatibility-search to makeflags.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #22, bug #48643 (project make):

> If I add a note when we start the compatibility check, it shows up here
(obviously we'd pick a more informative note if we added this):
...
> I guess it's appropriate here but it seemed odd to me.

i also think, it is appropriate here.
Does the message seem better if it says e.g.


"No implicit rule found to build 'hello.o'.
Trying compatibility search due to explicit prerequisite 'hello.tsk'."

?

> I wonder how many existing build systems would trigger this note?

When this behavior was modified in 2009 there were 5 bug reports listed in
https://savannah.gnu.org/bugs/?17752.

i am less concerned that existing makefiles, which rely on compatibility
search, trigger this new message.

My main concern is the makefiles which never relied on the old definition of
ought-to-exist. Those makefile (or brand new ones) can accidentally fall
victims of compatibility search, because compatibility search is enabled by
default and it is silent.

When i maintain a makefile, i definitely do not want to depend on
compatibility search and would appreciate if make gave me a hint whenever such
dependency is introduced.

Such a message could be enabled by default.

Alternatively, we can introduce a flag e.g. '--warn-compatibility-search' or
'--disable-compatibility-search'.
i like 'disable' better than 'warn', because 'disable' would cause make to
exit with an error on the 1st failed prerequisite. 'Warn', on the other hand,
would cause make to print the same message multiple times (once for every
prerequisite).
i also suspect, that most users won't enable such flag, should we add one.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #21, bug #48643 (project make):

I thought about this but adding such a note caused it to turn up in somewhat
unusual places.  For example in this test:

all: hello.tsk
%.tsk: %.o; $(info hello.tsk)
%.o: %.c; $(info hello.c)
%.o: %.f %.tsk; $(info hello.f)

we expect to get this output:

  make: Circular hello.o <- hello.tsk dependency dropped.
  hello.f
  hello.tsk


If I add a note when we start the compatibility check, it shows up here
(obviously we'd pick a more informative note if we added this):

  Starting compatibility search
  make: Circular hello.o <- hello.tsk dependency dropped.
  hello.f
  hello.tsk


I guess it's appropriate here but it seemed odd to me.

We should consider it.  I wonder how many existing build systems would trigger
this note?  That would be an interesting statistic.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #20, bug #48643 (project make):

Thank you, Paul.

The user may not realize that their makefile relies on compatibility search
and the related performance penalty.
Do you think it is appropriate to print a message when make begins
compatibility search to let the user know?

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Update of bug #48643 (project make):

  Status:None => Fixed  
 Open/Closed:Open => Closed 

___

Follow-up Comment #19:

I pushed Dmitry's fixes and new tests, plus added changes to the GNU make
manual to describe the new algorithm.

Thanks!

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #18, bug #48643 (project make):

Attached sv48643_fix2.diff and sv48643_test2.diff.

sv48643_test2.diff contains two new tests compared to sv48643_test.diff..
These new tests require that compatibility search builds intermediates.

sv48643_fix2.diff has two changes compared to sv48643_fix.diff.
Change 1: allow compatibility search to build intermediates. This is needed
for the new tests to pass.
Change 2: optimize compatibility search by keeping prerequisites explicitly
mentioned on unrelated rules as "possible".

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Additional Item Attachment, bug #48643 (project make):

File name: sv48643_fix2.diff  Size:8 KB


File name: sv48643_test2.diff Size:12 KB




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #17, bug #48643 (project make):

Thank you, Steven.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #16, bug #48643 (project make):

[comment #15 comment #15:]
> If patch-v1 is applied and released, then users will start writing
> makefiles which depend on this feature.  This will slow down make,
> including for those users who do not even use this feature.
Okay.

> Once this feature is released, it will be very difficult to undo.
Agreed.

> Let us say hello.c and hello.tsk are missing and consider the
> following example.
> 
> Example 5.
> 
> all: hello.tsk
> %.tsk: %.c; gcc -o $@ $<
> .DEFAULT:; echo 'int main() {}' > $@
> unrelated: hello.c
> 
> Here, make chooses rule '%.tsk: %.c' to build hello.tsk, because
> hello.c is mentioned on an unrelated rule.  Make then finds the
> default rule to build hello.c. Both make-4.3 and dgfix support example
> 5. Patch-v1 fails example 5.
This seems more compelling.

> I attempted to modify the search algorithm to support all the possible
> use cases and concluded that the most reliable and the simplest is the
> algorithm that performs compatibility search, presented above.
> We can reason that the compatibility search supports all uses cases
> that make-4.3 supports, because compatibility search is the same as
> the current search of make-4.3.

> i hope, this demonstrates that there are other differences.
It does.  You've convinced me that accepting 'unrelated' rules needs to
continue to be supported, and your patch does that.  Thanks!

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #15, bug #48643 (project make):

> Are you saying that this also should be the correct behaviour? 

I now realize, that inadequate description of example 3 was provided and
caused misunderstanding.
Let me provide a more verbose description.

Make has to do both skip unrelated rules and stay compatible with the
makefiles that depend on unrelated rules.


Example 3 is depends on unrelated rules. Make-4.3 successfully builds example
3 and the fix has to take care to keep building it.

Let us repeat example 3 here.

Example 3.

all: hello.tsk
%.tsk: %; $(info $@ from $<)
unrelated: hello


In example 3 rule '%.tsk: %' is chosen to build 'hello.tsk', because 'hello'
is mentioned explicitly on an unrelated rule.  Builtin rule '%: %.f' is chosen
to build 'hello' from 'hello.f', because 'hello.f' exists.  The chain is
hello.tsk <- hello <- hello.f.

Patch-v2 fails example 3.
Patch-v1 has code to handle this use case and is capable of building example
3.

However, i believe, there are other issues with patch-v1, described below.

Let us say hello.f is present in the filesystem and consider the following
example.

Example 4.

all: hello.tsk
%.tsk: %; $(info hello.tsk)
%: %.f; $(info hello.f)


Make looks for a rule to build 'hello.tsk' and considers '%.tsk: %'.  '%'
expands to 'hello'. There is no file 'hello' and 'hello' does not qualify as %
ought-to-exist.  There is rule '%: %.f' which can be used to build 'hello'
from 'hello.f', because 'hello.f' exists.  However, make rejects rule '%:
%.f', because 'hello' is an intermediate.
The manual does not allow match-anything rules to build intermediates for
performance reasons.
The original make authors made this (good imo) design choice and put this
restriction in the manual. See "10.5.5 Match-Anything Pattern Rules.".
Both make-4.3 and dgfix fail example 4.

Patch-v1, however, allows match-anything rules to build intermediates.
Patch-v1 causes makes to build 'hello' from '%: %.f' and then 'hello.tsk' from
'hello.'
This is the major issue with patch-v1.
This is not a bug fix any longer. This is a new and an expensive undesired
feature, accidentally sneaked into make.

If patch-v1 is applied and released, then users will start writing makefiles
which depend on this feature. This will slow down make, including for those
users who do not even use this feature. Once this feature is released, it will
be very difficult to undo.

Another issue with patch-v1 is that we don't know which uses cases it fails to
be compatible with.
Let us say hello.c and hello.tsk are missing and consider the following
example.

Example 5.

all: hello.tsk
%.tsk: %.c; gcc -o $@ $<
.DEFAULT:; echo 'int main() {}' > $@
unrelated: hello.c


Here, make chooses rule '%.tsk: %.c' to build hello.tsk, because hello.c is
mentioned on an unrelated rule.  Make then finds the default rule to build
hello.c. Both make-4.3 and dgfix support example 5. Patch-v1 fails example 5.

I attempted to modify the search algorithm to support all the possible use
cases and concluded that the most reliable and the simplest is the algorithm
that performs compatibility search, presented above.
We can reason that the compatibility search supports all uses cases that
make-4.3 supports, because compatibility search is the same as the current
search of make-4.3.


> here's only a difference between the implementations when the Makefile isn't
going to work anyway

i hope, this demonstrates that there are other differences.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #14, bug #48643 (project make):

I've got three implementations of Make lined up.  One is master, one has my
recent changes applied to master from last month (ssfix; patch included), and
one is master with your sv48643_fix.diff applied (dgfix).  I've attached a
script to compare these on the examples you've provided (and a couple more),
and the output with these three implementations.  The comments below refer to
this output.

[comment #12 comment #12:]
> Let us say hello.c is missing, hello.f is present in the filesystem and
> consider the following example
> 
> Example 1.
> 
> all: hello.tsk
> %.tsk: %.c; $(info $@ from $<)
> %.tsk: %.f; $(info $@ from $<)
> unrelated: hello.c
> 
> The current behavior is to choose rule '%.tsk: %.c' because 'hello.c' is
> mentioned explicitly on an unrelated rule.
> The desired behavior is to skip '%.tsk: %.c' rule and choose '%.tsk: %.f'
rule.
> 
> This desired behavior alone could be achieved by eliminating
ought-to-exist.
For this example, all three Makes fail when no source is provided, but
make-dgfix and master report no rule for 'hello.c', while make-ssfix reports
no rule for 'hello.tsk'.  When 'hello.f' is present but 'hello.c' is not,
master finds no rule for 'hello.c', while make-dgfix and make-ssfix correctly
build from 'hello.f'.  In the other two circumstances, they are identical.

For further comparison, I also dropped the 'unrelated' line, and all three
implementations produce the same results in all circumstances.  Note that
make-ssfix generates the same error in the no-source case, regardless of the
presence or absence of the 'unrelated' line, so it is treating it as
unrelated.

> However, let us consider the following example which demonstrates that
> ought-to-exist concept has to stay.
To be clear, I agree that ought-to-exist has to stay, but the documented and
implemented definitions are too broad, as you say.

> i think, a good change is to restrict ought-to-exist as this
> 
> "If a file name is mentioned in the makefile as a target or as an explicit
> prerequisite of the current target, then we say it ought to exist."
> 
Sounds good.

> Example 2.
> 
> all: hello.tsk
> hello.tsk: hello.c
> %.tsk: %.c; $(info $@ from $<)
> %.tsk: %.f; $(info $@ from $<)
> 
For this one, all three Makes are identical in all circumstances, so
make-ssfix _has not got rid of_ this part of the ought-to-exist definition.

> Example 3.
> 
> all: hello.tsk
> %.tsk: %; $(info $@ from $<)
> unrelated: hello
> 
(Aside: Is the lack of a suffix significant?)

> In this example rule '%.tsk: %' is chosen to build hello.tsk, because
'hello'
> is mentioned explicitly on an unrelated rule.
Are you saying that this also should be the correct behaviour?

With no 'hello' file, master and make-dgfix complain of no rule for 'hello',
while make-ssfix complains for 'hello.tsk'.  All three Makes behave
identically when 'hello' exists.

With the 'unrelated' line removed, all three Makes are identical in both
cases.

As with Example 1, make-ssfix's behaviour is consistent whether the
'unrelated' line is present or not, especially in the case where source is
absent, i.e., the error message is the same.

There's only a difference between the implementations when the Makefile isn't
going to work anyway, i.e., when 'hello' doesn't exist.  If there's a way to
build 'hello' too, it must appear as a target, so I don't think Example 3
demonstrates a case where the broader behaviour is required.


(file #52238, file #52239, file #52240)
___

Additional Item Attachment:

File name: fix-48643-simpsons-v2.diff Size:1 KB
   


File name: ss-trials.sh   Size:2 KB


File name: ss-trials-out.txt  Size:6 KB




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #13, bug #48643 (project make):

Added 4 patches. fix, test, doc and news.
The same changeset can be found in repo g...@github.com:dgoncharov/make.git on
branch sv48643_retry_compat_rules.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Additional Item Attachment, bug #48643 (project make):

File name: sv48643_test.diff  Size:10 KB


File name: sv48643_doc.diff   Size:1 KB


File name: sv48643_fix.diff   Size:8 KB


File name: sv48643_news.diff  Size:0 KB




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Dmitry Goncharov]

Follow-up Comment #12, bug #48643 (project make):

Let us say hello.c is missing, hello.f is present in the filesystem and
consider the following example

Example 1.

all: hello.tsk
%.tsk: %.c; $(info $@ from $<)
%.tsk: %.f; $(info $@ from $<)
unrelated: hello.c


The current behavior is to choose rule '%.tsk: %.c' because 'hello.c' is
mentioned explicitly on an unrelated rule.
The desired behavior is to skip '%.tsk: %.c' rule and choose '%.tsk: %.f'
rule.

This desired behavior alone could be achieved by eliminating ought-to-exist.

However, let us consider the following example which demonstrates that
ought-to-exist concept has to stay.

Example 2.

all: hello.tsk
hello.tsk: hello.c
%.tsk: %.c; $(info $@ from $<)
%.tsk: %.f; $(info $@ from $<)


When make considers rule '%.tsk: %.c' make discovers that hello.c is an
explicit prerequisite of the current target hello.tsk.  This qualifies hello.c
as ought to exist and tells make to choose this rule.  Regardless of whether
hello.c is present in the filesystem.  Even if hello.c is missing and hello.f
is present make still has to choose '%.tsk: %.c' rule, rather then '%.tsk:
%.f' rule, because the user specified hello.c as an explicit prerequisite of
hello.tsk.


On the other hand, when hello.c is an explicit prerequisite of an unrelated
rule (as in example 1), then the current definition of ought-to-exist is too
broad and causes an unrelated rule to be chosen.

i think, a good change is to restrict ought-to-exist as this

"If a file name is mentioned in the makefile as a target or as an explicit
prerequisite of the current target, then we say it ought to exist."


This will exclude unrelated rules in example 1, but still allow the intended
use case of ought-to-exist presented above in example 2.

However, this change of the definition of ought-to-exist and the related
change in the implementation will break the makefiles that depend on the old
behavior.

Example 3.

all: hello.tsk
%.tsk: %; $(info $@ from $<)
unrelated: hello


In this example rule '%.tsk: %' is chosen to build hello.tsk, because 'hello'
is mentioned explicitly on an unrelated rule.

We need to solve both
1. Skip unrelated rules.
and
2. Be compatible with makefiles that depend on unrelated rules.

The simplest and most reliable solution that meets both of these goals, i have
found is the following

1. Change the definition of ought-to-exist as described above.
2. Search according to the new definition of ought-to-exist. This will skip
all unrelated rules and achieve the desired effect.
3. If no rule is found, then search according to the old definition of
ought-to-exist. This allows makefiles which depend on the old definition of
ought-to-exist to keep working.

Notes about this proposed solution
1. This solution does not attempt to build the prerequisite in order to decide
if it can be used. I explored possible solutions which build the prerequisite
and came to the conclusion, the right behavior is to not attempt to build
one.

2. Compatibility search (step 3 in the algorithm above) should only be
performed, if the initial search finds such a prerequisite that is explicitly
mentioned on an unrelated rule. This ensures that if the makefile does not
have such a prerequisite, then no compatibility search is performed.

3. We could introduce a warning when compatibility search is performed and
advice the user to fix the makefile.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #11, bug #48643 (project make):

I saw a couple of tests in targets/NOTINTERMEDIATE failing, 2 and 10. 
Preserving an already set notintermediate flag (in the same way that
intermediate is preserved) allows the tests to pass.  This
(src/implicit.c#919):

  f->notintermediate = imf->notintermediate;
  f->intermediate = !(pat->is_explicit || f->notintermediate);

...becomes:

  f->notintermediate = f->notintermediate || imf->notintermediate;
  f->intermediate = f->intermediate ||
!(pat->is_explicit || f->notintermediate);

It's almost the same issue as before.  Without the f->is_target test, the
deplist entry is set at around line 777, the branch at 892 fails, so the lines
above are avoided.  With it, the entry is set at 821, and the branch at 892
passes.


[comment #6 comment #6:]
> I'm seeing some failures after applying this patch.
Now [make check] reports no failures, although features/guile and vms/library
are N/A, and options/dash-I warns of non-numeric arguments.  Were you seeing
any others?
> That is, pattern rule trees are searched "width first", not "depth first".
(By "width first" do you mean "breadth-first"?  I think the point might be
moot, though, since depth-vs-breadth affects the order of solutions, but we're
talking about whether foo:bar is a solution at all when looking for bar.)
> In the example we have:
> 
> default: a.foo
> %.foo: %.correct ; @echo $@ from $<
> %.foo: %.mislead ; @echo $@ from $<
> 
[snip]
> 
> misleading_target: a.mislead
> 
> It's true that it's a prerequisite of some completely different target, and
we're not even trying to build that target, but that doesn't matter.  The rule
5.3 in the above page says that it matches if all the prerequisites "exists or
*ought to exist*".  As can be seen in the rule statement, the definition of
"ought to exist" is simply, "is mentioned in the makefile as a target or as an
explicit prerquisite".
You're right - it's there in black and white.  But it seems to me an odd
definition of "ought to exist" for the purpose.  (So, yes, I seem to have a
similar position to Boris's.)  If you've tentatively identified that a.foo
could be built from a.mislead, you want to be looking next for a rule that
*targets* a.mislead, not merely one that mentions it.  Otherwise, you're not
forming a chain A:B, B:C, C:D, etc.  Furthermore, having failed the (f =
lookup_file) test, execution shortly goes on to look for another pattern rule
(by the recursive call to pattern_search), and it will be matching against the
target side of the rule, not the prerequisite side (right?).

I'm not certain of this, so I'd be interested in seeing other cases where the
broader definition is required.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #10, bug #48643 (project make):

[comment #9 comment #9:]
> Unfortunately it's exactly the is_target test that causes the problems.  The
rest of the changes, without that, don't seem to have any issues.
Okay, thanks.

I hope to get round to the documentation issue.  For now, I've had a look at
targets/INTERMEDIATE test 8, which is using this:


all: hello.z
%.z: test.x; touch $@
%.x: ;
.INTERMEDIATE: test.x


It then pre-creates hello.z, and asserts that nothing will be rebuilt, because
test.x is intermediate.

With the is_target test in place, execution reaches src/implicit.c line ~920:

  f->intermediate = !(pat->is_explicit || f->notintermediate);

f is pointing at the entry for "test.x", which already has its intermediate
flag set, but this line cancels it.  Later, the file struct is looked up
again:

  dep->file = lookup_file (s);

And so the file is seen as non-intermediate in src/remake.c:

  if (d_mtime == NONEXISTENT_MTIME && !d->file->intermediate)
/* We must remake if this dep does not
   exist and is not intermediate.  */
must_make = 1;


Without is_target, pat->file is null, so the f->intermediate assignment is
never carried out.  When the file is looked up later, f->intermediate hasn't
been reset, so test.x's intermediacy is preserved.

f->intermediate could be preserved like this:

  f->intermediate = f->intermediate || !(pat->is_explicit ||
f->notintermediate);


Then, without is_target, we get:

$ ~/Works/make/make -rRd
GNU Make 4.3.90
[snip]
Considering target file 'all'.
 File 'all' does not exist.
 Looking for an implicit rule for 'all'.
 No implicit rule found for 'all'.
  Considering target file 'hello.z'.
   Looking for an implicit rule for 'hello.z'.
   Trying pattern rule with stem 'hello'.
   Trying rule prerequisite 'test.x'.
   Found an implicit rule for 'hello.z'.
Looking for an implicit rule for 'test.x'.
Trying pattern rule with stem 'test'.
Found an implicit rule for 'test.x'.
   Finished prerequisites of target file 'hello.z'.
   Prerequisite 'test.x' of target 'hello.z' does not exist.
  No need to remake target 'hello.z'.
 Finished prerequisites of target file 'all'.
Must remake target 'all'.
Successfully remade target file 'all'.
make: Nothing to be done for 'all'.


With is_target:

$ ~/Works/make/make -rRd
GNU Make 4.3.90
[snip]
Considering target file 'all'.
 File 'all' does not exist.
 Looking for an implicit rule for 'all'.
 No implicit rule found for 'all'.
  Considering target file 'hello.z'.
   Looking for an implicit rule for 'hello.z'.
   Trying pattern rule with stem 'hello'.
   Trying rule prerequisite 'test.x'.
   Trying pattern rule with stem 'hello'.
   Trying rule prerequisite 'test.x'.
   Looking for a rule with explicit file 'test.x'.
Avoiding implicit rule recursion.
Trying pattern rule with stem 'test'.
   Found an implicit rule for 'hello.z'.
   Finished prerequisites of target file 'hello.z'.
   Prerequisite 'test.x' of target 'hello.z' does not exist.
  No need to remake target 'hello.z'.
 Finished prerequisites of target file 'all'.
Must remake target 'all'.
Successfully remade target file 'all'.
make: Nothing to be done for 'all'.


So either way, test 8 passes.

In summary, the lack of is_target causes the .INTERMEDIATE rule to be mistaken
for a 'real' rule about test.x, and avoids the execution path that would reset
the intermediate flag.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #9, bug #48643 (project make):

Unfortunately it's exactly the is_target test that causes the problems.  The
rest of the changes, without that, don't seem to have any issues.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #8, bug #48643 (project make):

[comment #6 comment #6:]
> I'm seeing some failures after applying this patch.
Just to narrow down the investigation, it might help to split this patch into
two.  The is_target test and the supporting declaration of f would be the
primary patch.  The rest is really just about allowing features/patternrules
test 6 to continue to pass with the primary in place, so perhaps that test
should be temporarily disabled while seeing whether the primary alone is still
the cause of these failures.

(Looking at your other points, meanwhile...)

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Follow-up Comment #7, bug #48643 (project make):

Perhaps your argument is (similar to Boris's argument in bug #17752 ) that the
algorithm in the manual is bad and should be changed; in particular the rule
5.3 that says "ought to exist" means "mentioned as a target or a prerequisite"
anywhere in the makefile is not right.

That may be so, but I don't think the change you've proposed here is
sufficient, similar to that conversation we had back in 2009.

The change you suggest, so far as I understand, changes the 5.3 rule so that
"ought to exist" means that the file must actually be mentioned as a target,
and being mentioned as a prerequisite is not sufficient.

At the very least, if we decide to move forward with this, we have to install
a documentation change and mention it in the NEWS file as a
backward-compatibility break.

The problem with this change is that it does have other effects.  For one
thing, it causes some regression tests related to intermediate files to fail. 
That's because after this change, targets that were previously considered as
"ought to exist" are no longer considered so, and thus where previously we
would stop at step #5 now we fall through to step #6 which changes the
behavior.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Update of bug #48643 (project make):

  Status:   Fixed => None   
 Open/Closed:  Closed => Open   

___

Follow-up Comment #6:

I'm seeing some failures after applying this patch.

I think that the behavior of the makefile provided is actually correct in 4.x
(that is, the error is correct and this attempt to change it is not right).

I know it seems counter-intuitive.  However, the manual says that first, make
will try to build all targets using a simple implicit rule (that is, it tries
all pattern rules that match the target) and only if that doesn't work, will
it "try harder".  That is, pattern rule trees are searched "width first", not
"depth first".

Refer to
https://www.gnu.org/software/make/manual/html_node/Implicit-Rule-Search.html

Note that in step #2 we construct a list of pattern rules that match the
target, then in step #5 we walk all those patterns.  Then only if no rule
matches in step #5 do we proceed to step #6, and try to build non-existent
prerequisites of pattern rules found in step #2.

In the example we have:


default: a.foo
%.foo: %.correct ; @echo $@ from $<
%.foo: %.mislead ; @echo $@ from $<


so we have two patterns that match "a.foo".  We will try BOTH of these, using
step #5 in the above page, before we try to recurse and look for ways to
create prerequisites of these patterns (step #6).

We first look at '%.foo : %.correct' but there is no file 'foo.correct'
existing, and there's no mention of it in the makefile anywhere, so this rule
doesn't match and we keep going.

We then look at '%.foo : %.mislead'.  There is no file 'foo.mislead',
*however* the file 'foo.mislead' is mentioned in the makefile, here:


misleading_target: a.mislead


It's true that it's a prerequisite of some completely different target, and
we're not even trying to build that target, but that doesn't matter.  The rule
5.3 in the above page says that it matches if all the prerequisites "exists or
*ought to exist*".  As can be seen in the rule statement, the definition of
"ought to exist" is simply, "is mentioned in the makefile as a target or as an
explicit prerquisite".  The file "a.mislead" definitely meets that criteria,
thus this rule matches... and leads to an error.

After the extra debugging output added as part of bug #61042 the process
becomes easier to understand:

  Considering target file 'a.foo'.
   File 'a.foo' does not exist.
   Looking for an implicit rule for 'a.foo'.
   Trying pattern rule '%.foo: %.correct' with stem 'a'.
   Trying implicit prerequisite 'a.correct'.
   Not found 'a.correct'.
   Trying pattern rule '%.foo: %.mislead' with stem 'a'.
   Trying implicit prerequisite 'a.mislead'.
   'a.mislead' ought to exist.
   Found implicit rule '%.foo: %.mislead' for 'a.foo'.


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Paul D. Smith]

Update of bug #48643 (project make):

  Status:None => Fixed  
 Assigned to:None => psmith 
 Open/Closed:Open => Closed 
   Fixed Release:None => SCM
   Triage Status:None => Medium Effort  

___

Follow-up Comment #5:

I applied this fix and also the new regression test for it.

Thanks!

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Additional Item Attachment, bug #48643 (project make):

File name: fix-17752-48643.patch  Size:4 KB




___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #4, bug #48643 (project make):

Here's a patch that restores the is_target test, and counts the %: rules on
the stack, permitting at most one.  This should fix this bug, without bug
#17752 returning, as features/patternrules test 6 still passes, though perhaps
its explanation should be changed:


# TEST 6: Make sure that non-target files are still eligible to be created
# as part of implicit rule chaining.  Savannah bug #17752.


The problem was not that the intermediate file was not a target, but that it
matched a pattern with no suffix.


diff --git a/src/implicit.c b/src/implicit.c
index b281a17..4223d0f 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -24,7 +24,8 @@ this program.  If not, see . 
*/
 #include "commands.h" /* set_file_variables */
 
 static int pattern_search (struct file *file, int archive,
-   unsigned int depth, unsigned int recursions);
+   unsigned int depth, unsigned int recursions,
+   unsigned int anydepth);
 
 /* For a FILE which has no commands specified, try to figure out some
from the implicit pattern rules.
@@ -42,7 +43,7 @@ try_implicit_rule (struct file *file, unsigned int depth)
  (the archive search omits the archive name), it is more specific and
  should come first.  */
 
-  if (pattern_search (file, 0, depth, 0))
+  if (pattern_search (file, 0, depth, 0, 0))
 return 1;
 
 #ifndef NO_ARCHIVES
@@ -52,7 +53,7 @@ try_implicit_rule (struct file *file, unsigned int depth)
 {
   DBF (DB_IMPLICIT,
_("Looking for archive-member implicit rule for '%s'.\n"));
-  if (pattern_search (file, 1, depth, 0))
+  if (pattern_search (file, 1, depth, 0, 0))
 return 1;
 }
 #endif
@@ -173,6 +174,9 @@ struct tryrule
 
 /* Nonzero if the LASTSLASH logic was used in matching this rule. */
 char checked_lastslash;
+
+/* True if the rule would match anything. */
+char anyrule;
   };
 
 int
@@ -200,7 +204,8 @@ stemlen_compare (const void *v1, const void *v2)
 
 static int
 pattern_search (struct file *file, int archive,
-unsigned int depth, unsigned int recursions)
+unsigned int depth, unsigned int recursions,
+unsigned int anydepth)
 {
   /* Filename we are searching for a rule for.  */
   const char *filename = archive ? strchr (file->name, '(') : file->name;
@@ -317,12 +322,20 @@ pattern_search (struct file *file, int archive,
   const char *target = rule->targets[ti];
   const char *suffix = rule->suffixes[ti];
   char check_lastslash;
+  char anyrule = 0;
 
   /* Rules that can match any filename and are not terminal
- are ignored if we're recursing, so that they cannot be
- intermediate files.  */
-  if (recursions > 0 && target[1] == '\0' && !rule->terminal)
-continue;
+ must not be allowed to nest very deeply.  Record whether
+ we have such a rule, so that if a further level of
+ recursion is required as a result of matching this rule,
+ we can increment the number of such rules we're nested
+ in.  */
+  if (target[1] == '\0' && !rule->terminal)
+{
+  if (anydepth > 0)
+continue;
+  anyrule = 1;
+}
 
   if (rule->lens[ti] > namelen)
 /* It can't possibly match.  */
@@ -395,6 +408,7 @@ pattern_search (struct file *file, int archive,
  target in MATCHES.  If several targets of the same rule match,
  that rule will be in TRYRULES more than once.  */
   tryrules[nrules].rule = rule;
+  tryrules[nrules].anyrule = anyrule;
   tryrules[nrules].matches = ti;
   tryrules[nrules].stemlen = stemlen + (check_lastslash ? pathlen :
0);
   tryrules[nrules].order = nrules;
@@ -704,6 +718,7 @@ pattern_search (struct file *file, int archive,
   /* Go through the nameseq and handle each as a prereq name. 
*/
   for (d = dl; d != 0; d = d->next)
 {
+  struct file *f;
   struct dep *expl_d;
   int is_rule = d->name == dep_name (dep);
 
@@ -753,7 +768,7 @@ pattern_search (struct file *file, int archive,
  FILENAME's directory), so it might actually exist.  */
 
   /* @@ dep->changed check is disabled. */
-  if (lookup_file (d->name) != 0
+  if (((f = lookup_file (d->name)) != 0 && f->is_target)
   /*|| ((!dep->changed || check_lastslash) && */
   || file_exists_p (d->name))
 {
@@ -794,7 +809,8 @@ pattern_search (struct file *file, int archive,
   if (pattern_search (int_file,
   0,
 

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #3, bug #48643 (project make):

With the is_target test restored in implicit.c, "make check" fails:


features/patternrules ... Error running
/home/simpsons/Works/make/tests/../make (expected 0; got 512):
'/home/simpsons/Works/make/tests/../make' '-f'
'work/features/patternrules.mk.6'
FAILED (11/12 passed)


Seems to be in conflict with bug #17752 - the is_target test was removed to
resolve it, I gather.

I adapted a makefile from features/patternrules so that the intermediate can
have a specific suffix:


STEM = xyz
BIN = $(STEM)$(SFX)
COPY = $(STEM).cp
SRC = $(STEM).c
allbroken: $(COPY) $(BIN) ; @echo ok
$(SRC): ; @echo 'main(){}'
%.cp: %$(SFX) ; @echo $@ from $<
%$(SFX) : %.c ; @echo $@ from $<


Used with no builtin rules, if $(SFX) is non-empty, this works in 3.80, 3.81
and 4.3.  If $(SFX) is empty, only 3.81 fails.

So a %.sfx:%.c rule works, but %:%.c doesn't, because the target lacks a
suffix.  It seems this test eliminates the rule from the candidates:


  /* Rules that can match any filename and are not terminal
 are ignored if we're recursing, so that they cannot be
 intermediate files.  */
  if (recursions > 0 && target[1] == '\0' && !rule->terminal)
continue;


This suggests that the test case in bug #17752 is a 'feature', as it tries to
use a rule "that can match any filename" to build an intermediate,
specifically, a builtin rule %:%.c.  The in_use flag prevents these repeating,
as in foo.c.c.c, but with umpteen builtin rules of this sort, every
arrangement of every subset of suffixes in these rules is tried, and that
takes a long time, which can result in some checks (variables/EXTRA_PREREQS)
being aborted.

I managed to introduce a new parameter to pattern_search, unsigned anydepth,
which is incremented on the recursive call only if such a rule is used.  The
test above then becomes:


  char anyrule = 0;
  if (target[1] == '\0' && !rule->terminal)
{
  if (anydepth > 3)
continue;
  anyrule = 1;
}


anyrule is then stored in the tryrules entry, so it can be tested on
recursion:


  if (pattern_search (int_file,
  0,
  depth + 1,
  recursions + 1,
  anydepth + !!tryrules[ri].anyrule))


This allows %:%.X rules to be applied at any depth, but limits the total
number on the stack.

For me, "make check" passes with the anydepth limit set to 0, 1, 2 or 3, but 4
took too long on the variables/EXTRA_PREREQS tests.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #2, bug #48643 (project make):

I think the problem might be in src/implicit.c.

>From 3.81 code:


  /* The DEP->changed flag says that this dependency resides in a
 nonexistent directory.  So we normally can skip looking for
 the file.  However, if CHECK_LASTSLASH is set, then the
 dependency file we are actually looking for is in a
different
 directory (the one gotten by prepending FILENAME's
directory),
 so it might actually exist.  */

  /* @@ dep->changed check is disabled. */
  if (((f = lookup_file (name)) != 0 && f->is_target)
  /*|| ((!dep->changed || check_lastslash) && */
  || file_exists_p (name))
continue;


The corresponding check in 4.3:


  if (lookup_file (d->name) != 0
  /*|| ((!dep->changed || check_lastslash) && */
  || file_exists_p (d->name))


There's no check for f->is_target on the result of lookup_file, so I presume
it is picking up the name as a dependency.  Perhaps it should read:


  if (((f = lookup_file (d->name)) != 0 && f->is_target)
  /*|| ((!dep->changed || check_lastslash) && */
  || file_exists_p (d->name))


f is a [struct file *], not declared in 4.3.

___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[Steven Simpson]

Follow-up Comment #1, bug #48643 (project make):

According to what I wrote in
, this
behaviour is present in 3.82, 4.1 and 4.2.1, but not 3.81.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

[bug #48643] Irrelevant targets can confuse make on which pattern rule to select.

[anonymous]

URL:
  

 Summary: Irrelevant targets can confuse make on which pattern
rule to select.
 Project: make
Submitted by: None
Submitted on: Wed 27 Jul 2016 05:02:12 AM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: 4.2.1
Operating System: POSIX-Based
   Fixed Release: None
   Triage Status: None

___

Details:

This is a based on
https://lists.gnu.org/archive/html/help-make/2016-01/msg00011.html which I
couldn't find an actual bug report for.

I'm not 100% certain that this is a bug; it's possible that we're missing
something subtle in the manual, but I really do think it's a bug.

I've attached a test case.  The bad behavior is triggered by the last line in
the Makefile; commenting it out causes things to work correctly.

The correct graph for make to choose is


default -> a.foo (%.foo, line 3) -> a.correct -> a.correct_src


But, if we append this line, then make makes the wrong decision:


misleading_target: a.mislead


The line should be irrelevant, because misleading_target isn't something we're
trying to build.  But, it causes make to instead choose this graph:


default -> a.foo (%.foo, line 6) -> a.mislead -> a.mislead_src


That is, it chose the wrong pattern rule for %.foo .



___

File Attachments:


---
Date: Wed 27 Jul 2016 05:02:12 AM UTC  Name: Makefile  Size: 543B   By: None



___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make