bugmake - Bugs: bug #61226, Missing included files that have...

 
 

bug #61226: Missing included files that have rules don't show as an error

Submitted by:  Dmitry Goncharov <dgoncharov>
Submitted on:  Sun 26 Sep 2021 02:34:37 PM UTC
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Component Version:  SCM Operating System:  None
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

( Jump to the original submission )

Tue 18 Jan 2022 07:01:38 PM UTC, comment #11: 

In the abstract, it seems to me that if a user asks to include a file and that file doesn't exist, that the include should generate some kind of error.

However, maybe you're right that if a rule exists that tells make how to build that included file we should not generate an error as long as the recipe for that rule doesn't fail, even if the included file still doesn't exist after the recipe is run.

I'm still not excited about the idea of distinguishing recipes that are empty, versus recipes that are not empty but don't update the target file.

Paul D. Smith <psmith>
Project Administrator
Tue 18 Jan 2022 12:43:49 AM UTC, comment #10: 

comment #9:

> I will leave this issue open to think about how to best introduce a backward-incompatible change that might help in this situation.


Do i understand it correctly, that as long as compatibility allows, you'd like make to fail whenever make cannot include a makefile, as long as the included makefile has a rule with a recipe, even empty?

i'd like to mention that en empty recipe in a force rule has the benefit of avoiding a redundant implicit search. This helps performance and makes a meaningful debug output, because it removes repetitive "Looking for an implicit rule..." and "No implicit rules found..." for each .d file.
So, i prefer rules like

%.d:;
%.h:;

Dmitry Goncharov <dgoncharov>
Mon 17 Jan 2022 11:59:34 PM UTC, comment #9: 

For now I decided to put back the original behavior (revert the main.c changes).  I will leave this issue open to think about how to best introduce a backward-incompatible change that might help in this situation.

I really don't like the idea of creating a distinction between an "empty" recipe and a "non-empty" recipe.  Just to point out the description in "Rules without Recipes or Prerequisites" is talking about NO recipe vs. some recipe, not empty recipe vs non-empty recipe.  That is, the difference between FORCE: and FORCE:;.  However as I mentioned before it's not actually required that you don't have a recipe here: any recipe (or no recipe) that doesn't update the target will serve just as well.  This text is not wrong, although it could be argued that it's incomplete by not describing all the other ways a target might be considered up to date in addition to a missing recipe.

The text in "Including Other Makefiles" was confusing so I modified it to say this:

> Only after it has failed to find a rule to remake the makefile, or it found a rule but the recipe failed, will @code{make} diagnose the missing makefile as a fatal error.

Paul D. Smith <psmith>
Project Administrator
Sun 17 Oct 2021 04:33:04 PM UTC, comment #8: 

> > Switching to -include robs the user of a useful message, should there be a real issue.


> I'm not sure what this means: in what situation do we lose a useful message?


-include robs the user of a not readable or corrupted .d file, even though user's intervention is required.

i think, hand written included makefiles require different error handling than generated .d files.

A missing hand written makefile is an error. The user has the choice of 'include' vs '-include' as you described above. I agree with that logic.

On the other hand, a missing .d file (or any other included makefile generated by a rule) is not an error. It is simply a clean build.

i think, make should not print a warning when a .d file is missing. make should proceed and create the missing file. However, when .d is present, but make cannot include it, then make should print an error and stop.

The traditional make behavior, which is still in 4.3 is exactly this.
i think, we should retain this behavior.

in regards to the change in main.c see the following possibilities

1. Revert the change in main.c.
2. Modify the change in main.c to allow for missing .d files, but print an error and stop in the other cases.
3. Modify the change in main.c to print a warning and continue.

i like option 2, which is what the first patch does. You correctly observed that this will break any rule that has prerequisites and a recipe and fails to generate the file and returns 0.
This boils down to that difference between the implementation and the manual. Do you want to update the manual to match the implementation or vice versa?

Dmitry Goncharov <dgoncharov>
Sun 17 Oct 2021 02:54:21 PM UTC, comment #7: 

Well, we could print one warning per include line rather than one warning per file included.  Obviously, that warning would only talk about the first file on the include line but since the suggested solution is to use "-include" maybe that's sufficient.

> Switching to -include robs the user of a useful message, should there be a real issue.


I'm not sure what this means: in what situation do we lose a useful message?  Either the user wants the include to fail if the file doesn't exist in which case they use "include" and get an error, or the user wants the include to not fail if the file doesn't exist, in which case they use "-include" and don't get an error which is presumably what they wanted.

Paul D. Smith <psmith>
Project Administrator
Mon 04 Oct 2021 03:08:52 AM UTC, comment #6: 

It is disturbing that the warning is printed each time a .d file is missing. In other words, make prints a warning in an everything goes as planned situation, like a clean build. Switching to -include robs the user of a useful message, should there be a real issue.

Dmitry Goncharov <dgoncharov>
Sun 03 Oct 2021 10:23:04 PM UTC, comment #5: 

> When you say the special behavior do you mean the section "Rules without Recipes or Prerequisites"?


exactly

> I think that text is a little misleading.  There's actually nothing magical about having no recipe or an empty recipe here, as best as I can remember.  Make doesn't check for this.  Even if you provide a recipe for a force target it will still behave the same as it would without a recipe, as long as the recipe doesn't actually create the force target.


There is a difference between the implementation and the text in the manual. Given that the implementation is bound by backward compatibility, i think the text needs to be updated. Even though, this may not be what the original authors intended.

There is also this statement in "3.3 Including Other Makefiles"
"...after it has tried to find a way to remake a makefile and failed, will make diagnose the missing makefile as a fatal error. "

This statement says a missing makefile is a fatal error, regardless of whether the rule returned success. Looks like this part of the manual needs to be updated too.

> My current thinking on this is we should revert this part of the fix, but leave the original part.


Do you mean keep the change in remake.c?

> Maybe the thing to do is, if we detect this situation we can emit a warning message that users should change their makefiles to use "-include" instead of "include" which would avoid that warning.  Then in a few releases we could put back this change where make would fail if included files weren't built.


i am attaching another patch with a warning. If this patch is good, then we'll need to update the tests.
This warning in the attached patch is quite verbose, because it is printed for every missing .d file.

Dmitry Goncharov <dgoncharov>
Sat 02 Oct 2021 07:11:20 PM UTC, comment #4: 

When you say the special behavior do you mean the section "Rules without Recipes or Prerequisites"?

I think that text is a little misleading.  There's actually nothing magical about having no recipe or an empty recipe here, as best as I can remember.  Make doesn't check for this.  Even if you provide a recipe for a force target it will still behave the same as it would without a recipe, as long as the recipe doesn't actually create the force target.

So, I think the change to fail includes if the target doesn't actually build them, only if it has a recipe, is too large of a backward-compatibility break to consider for now.

My current thinking on this is we should revert this part of the fix, but leave the original part.  Maybe the thing to do is, if we detect this situation we can emit a warning message that users should change their makefiles to use "-include" instead of "include" which would avoid that warning.  Then in a few releases we could put back this change where make would fail if included files weren't built.

Paul D. Smith <psmith>
Project Administrator
Sun 26 Sep 2021 05:35:16 PM UTC, comment #3: 

i agree, this is a special case.
imo, empty vs not empty is okay for make to consider, given that make manual documents this special behavior when the recipe or prerequisites are empty and the file is missing.

This is in sharp contract with the other case, when there is a recipe and the recipe fails to create the file. imo, it is correct for make to fail that case.

Would it be acceptable if we could retain the previous fix and allow for this special case?

Dmitry Goncharov <dgoncharov>
Sun 26 Sep 2021 04:49:48 PM UTC, comment #2: 

Hm.  I'm not a fan of this solution.  I really don't like having make do different things depending on the content of recipes, even empty vs. not empty.

Additionally, this is just a special case; it could be the case that we need to actually have some recipe invoked, but then the recipe decides that there is nothing to do for some reason and doesn't create the file.  One simple example is that some makefiles use the shell no-op target ":" rather than an empty recipe, just to be more clear about what's happening; like this:

hello.d: ; @:

If we don't consider, for the moment, backward compatibility, would it be more correct and work properly to suggest that people should use -include in situations where they don't want an error if the included file is not actually rebuilt?

hello.o: hello.d; touch $@ $<
hello.d:;
-include hello.d

Of course even if this is the best answer it's not acceptable from a backward-compatibility standpoint to break this common usage, at least not immediately.  I think perhaps at least some of the change in the previous fix might need to be reverted, although I'm not sure the entire thing needs to be.

Paul D. Smith <psmith>
Project Administrator
Sun 26 Sep 2021 03:01:56 PM UTC, comment #1: 

This is an example of the failure

$ ls
makefile
$ cat makefile
hello.o: hello.d; touch $@ $<
hello.d:;
include hello.d
$ make
makefile:3: hello.d: No such file or directory
$ echo $?
2

Dmitry Goncharov <dgoncharov>
Sun 26 Sep 2021 02:34:37 PM UTC, original submission:  

7c4e6b0299774280e3a93060c70c661f459aaf40 introduced a change in main.c, among others.
This change causes make to exit with an error message when a rule that matches an included file does not create the file.
This change is good when the included file is indeed intended to be created.
However, this change is a regression when the included file is not intended to be created, as in the case of generated dependency files.
E.g.

hello.o: hello.d; touch $@ $<
hello.d:;
include hello.d

In this makefile the rule 'hello.d:;' is not intended to create 'hello.d'.
Instead, when 'hello.d' is missing make runs 'hello.d:;' rule and imagines 'hello.d' was updated and then proceeds to create 'hello.o' and thus generates 'hello.d' as a byproduct of 'hello.o' generation.

Dmitry Goncharov <dgoncharov>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dgoncharov (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-18 psmith Item GroupBug => Enhancement
        SummaryA regression prevents generation of missing included dependency files. => Missing included files that have rules don't show as an error
    2021-10-03 dgoncharov Attached File- => Added sv_61226_no_build_on_missing_includes.diff, #52015
    2021-09-26 dgoncharov Attached File- => Added sv_61226_no_build_on_missing_includes.diff, #51967
        Attached File- => Added sv_61226_no_build_on_missing_includes_test.diff, #51968

    Back to the top


    Powered by Savane 3.9