bugmake - Bugs: bug #61042, Enhance implicit rule search...

 
 

bug #61042: Enhance implicit rule search logging

Submitter:  Dmitry Goncharov <dgoncharov>
Submitted:  Sun 15 Aug 2021 04:11:25 AM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.4 Operating System:  Any
Fixed Release:  4.4 Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 28 Nov 2021 10:18:31 PM UTC, comment #12: 


> I used it because I wanted to mark that data member as being "private" in the sense that it shouldn't be used directly by other parts of the code; instead the get_rule_defn() function should always be called to retrieve that value.


Good to know. i'll follow this convention.

Dmitry Goncharov <dgoncharov>
Sun 28 Nov 2021 09:11:59 PM UTC, comment #11: 

I used it because I wanted to mark that data member as being "private" in the sense that it shouldn't be used directly by other parts of the code; instead the get_rule_defn() function should always be called to retrieve that value.

If we were using C++ many things would be a lot cleaner :).

Paul D. Smith <psmith>
Group administrator
Sun 28 Nov 2021 08:49:52 PM UTC, comment #10: 

Thank you, Paul.

Does the leading underscore in rule->_defn has a special meaning, follow gnu make convention or similar?

Dmitry Goncharov <dgoncharov>
Tue 19 Oct 2021 03:06:13 AM UTC, comment #9: 

Applied, with minor fixups.  Thanks Dmitry!

Paul D. Smith <psmith>
Group administrator
Sun 17 Oct 2021 06:58:49 PM UTC, comment #8: 

Merged master to my branch and resolved the conflict in patch
 sv_61042_implicit_rule_search_logging2.diff.

The same change is also available here
git@github.com:dgoncharov/make.git
on branch
sv61042_enhance_implicit_search_logging.

(file #52115)

Dmitry Goncharov <dgoncharov>
Sun 17 Oct 2021 06:31:05 PM UTC, comment #7: 

There are 2 situations when implicit rule lookup takes this short cut.

1.

hello.tsk: hello.o
%.tsk: %.o; touch $@


and

2.

hello.tsk:
%.tsk: %.o; touch $@
hello.o:



There are 2 if branches in implicit.c

This branch handles the case 1 described above.


for (expl_d = file->deps; expl_d != 0; expl_d = expl_d->next)
if (streq (dep_name (expl_d), d->name))
  break;
if (expl_d != 0)
{
  (pat++)->name = d->name;
  continue;
}


and

This branch handles the case 2 described above.



f = lookup_file (d->name);
if (f /* || ((!dep->changed || check_lastslash) */
  || file_exists_p (d->name))
{
  (pat++)->name = d->name;
  continue;
}


This patch has both if branches combined to handle both cases, because the handling is the same.
The conflict was introduced by this fix to set f->is_explicit.
Let me merge master onto my branch and resubmit.

Dmitry Goncharov <dgoncharov>
Sun 17 Oct 2021 05:47:10 PM UTC, comment #6: 

I see that I missed a part of the original patch.  Hm, there are a number of recent changes to this section of code which are in conflict and the way I originally resolved this was incorrect.  I'll need to figure out what's going on.

Paul D. Smith <psmith>
Group administrator
Sun 17 Oct 2021 03:44:11 PM UTC, comment #5: 

Actually that change causes a number of regression tests to fail with incorrect output so I removed it.

Paul D. Smith <psmith>
Group administrator
Sun 17 Oct 2021 03:18:28 PM UTC, comment #4: 

Dmitry I'm not sure about this change:


@@ -744,15 +752,9 @@ pattern_search (struct file *file, int archive,
                   /* If this prereq is also explicitly mentioned for FILE,
                      skip all tests below since it must be built no matter
                      which implicit rule we choose. */
-
                   for (expl_d = file->deps; expl_d != 0; expl_d = expl_d->next)
                     if (streq (dep_name (expl_d), d->name))
                       break;
-                  if (expl_d != 0)
-                    {
-                      (pat++)->name = d->name;
-                      continue;
-                    }

                   /* The DEP->changed flag says that this dependency resides
                      in a nonexistent directory.  So we normally can skip


Why are we removing this shortcut here?  It seems like we're paying a cost everywhere to get better output when debugging is enabled...?  Or is there some correctness reason to remove this?

Paul D. Smith <psmith>
Group administrator
Sat 16 Oct 2021 02:06:38 PM UTC, comment #3: 

Another scenario that i noticed, when this enhanced logging helps, is when the source file is present, but vpath is incorrect.
E.g. vpath %.cpp %.c $(srcdir).
Again make ignores the rule, because it cannot find the source file, but gives no indication of why it ignores the rule. The new logging prints "Not found '<file>'.".

Dmitry Goncharov <dgoncharov>
Sun 15 Aug 2021 04:16:52 AM UTC, comment #2: 

sv_61042_reuse_rule_name_for_dash_p.diff reuses rule->name for printing the db under -p.

Dmitry Goncharov <dgoncharov>
Sun 15 Aug 2021 04:14:28 AM UTC, comment #1: 

sv_61042_implicit_rule_search_logging.diff has the following

Changes in the implicit rule search logging:
1. The messages which refer to a rule print the first line
    (the targets and prerequisites) of the rule.
2. A new message tells when a rule is rejected, along with the reason.
3. The 'Looking for an implicit rule...' message is printed for every
    prerequisite, rather than the top level target only.
4. "Trying harder" message is printed, when intermediate prerequisites are
    allowed.
5. The 'No rule found...' and 'Found implicit rule...' messages are printed for
    every prerequisite, rather than the top level target only.
6. "Ought to exist...", "Found..." or "Not found..." message is printed for each prerequisite.

This is the diagnostics of the same example above with enhanced logging.

$ ~/src/gmake/make/l64/make -R --debug=implicit hello.o |tail +9
Updating goal targets....
 File 'hello.o' does not exist.
 Looking for an implicit rule for 'hello.o'.
 Trying pattern rule '%.o: %.c' with stem 'hello'.
 Trying implicit prerequisite 'hello.c'.
 Not found 'hello.c'.
 Trying harder.
 Trying pattern rule '%.o: %.c' with stem 'hello'.
 Trying implicit prerequisite 'hello.c'.
 Looking for a rule with intermediate file 'hello.c'.
  Avoiding implicit rule recursion for rule '%.o: %.c'.
  No implicit rule found for 'hello.c'.
 Rejecting rule '%.o: %.c' due to impossible prerequisite 'hello.c'.
 No implicit rule found for 'hello.o'.
Must remake target 'hello.o'.
make: * No rule to make target 'hello.o'.  Stop.

Dmitry Goncharov <dgoncharov>
Sun 15 Aug 2021 04:11:25 AM UTC, original submission:  

Enhance implicit search logging.

Consider this rule
%.o: %.c; $(CC) -o $@ -c $<

When the user wants to make hello.o and hello.c is missing, make gives the
following diagnostics.

$ ls
makefile
$ cat makefile
%.o: %.c; $(CC) -o $@ -c $<
$ ~/src/gmake/make/m64/make -R --debug=implicit hello.o |tail +9
Updating goal targets....
 File 'hello.o' does not exist.
 Looking for an implicit rule for 'hello.o'.
 Trying pattern rule with stem 'hello'.
 Trying implicit prerequisite 'hello.c'.
 Trying pattern rule with stem 'hello'.
 Trying implicit prerequisite 'hello.c'.
 Looking for a rule with intermediate file 'hello.c'.
  Avoiding implicit rule recursion.
 No implicit rule found for 'hello.o'.
Must remake target 'hello.o'.
make: * No rule to make target 'hello.o'.  Stop.
$

The bottom line in this diagnostics is the "No rule found" message.
While the issue is that hello.c is missing.
And the user stares at the makefile and says
"How cannot make find the rule? Here it is."

Preferably make should tell which rule it is trying and why it is rejecting it
and which prerequisite make is trying and what the outcome is.

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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-10-19 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4
    2021-10-17 dgoncharov Attached File- Added sv_61042_implicit_rule_search_logging2.diff, #52115
    2021-08-15 dgoncharov Attached File- Added sv_61042_reuse_rule_name_for_dash_p.diff, #51781
    2021-08-15 dgoncharov Attached File- Added sv_61042_implicit_rule_search_logging.diff, #51780

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code