From 5f64589b84a7349316a04cdbe607a5824459b731 Mon Sep 17 00:00:00 2001 From: Ron Yorston Date: Thu, 3 Nov 2022 15:49:11 +0000 Subject: make: fixes to inference rules Austin Group defect report 875 clarifies some aspects of inference rules. The crux of the issue is related to chained inference rules so it doesn't affect POSIX mode. The test makefile looks like this: .SUFFIXES: .a .b .c .a.c: @echo .a.c .b.c: @echo .b.c test.c: test.b test.a: test.b: The correct output is deemed to be '.a.c'. Additional complications are: - whether or not the prerequisite files are present; - the use of the suffixes '.a' and '.c' may result in the builtin inference rule '.c.a' being considered. In favourable circumstances pdpmake managed to give the correct result, in unfavourable it reported circular dependencies or segfaulted. Changes to fix these issues are: - When prerequisites are being recursively built the standard says: 'Upon recursion, each prerequisite shall become a target itself.' Follow this requirement. - At the end of make() the target being built should have its time (as represented by n_tim in struct name) updated when any action has been taken. - When dyndep() is looking for prerequisites it should: * skip candidates that are in the process of being built; * consider whether an explicit candidate is a target, not whether it has any commands associated with it. pdpmake now behaves similarly to GNU make when presented with makefiles like the above. bmake gives the incorrect output '.b.c'. --- miscutils/make.c | 39 +++++++++++++++++++++------------------ testsuite/make.tests | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/miscutils/make.c b/miscutils/make.c index daad75f5c..86d06e970 100644 --- a/miscutils/make.c +++ b/miscutils/make.c @@ -717,21 +717,23 @@ dyndep(struct name *np, struct rule *imprule) sp = findname(auto_concat(newsuff, suff)); if (sp && sp->n_rule) { // Generate a name for an implicit prerequisite - pp = newname(auto_concat(base, newsuff)); - if (!pp->n_tim.tv_sec) - modtime(pp); - if ((!chain && (pp->n_tim.tv_sec || getcmd(pp))) || - (chain && dyndep(pp, NULL))) { + struct name *ip = newname(auto_concat(base, newsuff)); + if ((ip->n_flag & N_DOING)) + continue; + if (!ip->n_tim.tv_sec) + modtime(ip); + if (chain ? ip->n_tim.tv_sec || (ip->n_flag & N_TARGET) : + dyndep(ip, NULL) != NULL) { // Prerequisite exists or we know how to make it if (imprule) { dp = NULL; - newdep(&dp, pp); + newdep(&dp, ip); imprule->r_dep = dp; imprule->r_cmd = sp->n_rule->r_cmd; } + pp = ip; goto finish; } - pp = NULL; } } } @@ -2121,6 +2123,7 @@ make(struct name *np, int level) } for (dp = rp->r_dep; dp; dp = dp->d_next) { // Make prerequisite + dp->d_name->n_flag |= N_TARGET; estat |= make(dp->d_name, level + 1); // Make strings of out-of-date prerequisites (for $?), @@ -2166,28 +2169,28 @@ make(struct name *np, int level) if (quest) { if (timespec_le(&np->n_tim, &dtim)) { - clock_gettime(CLOCK_REALTIME, &np->n_tim); - return 1; // 1 means rebuild is needed + didsomething = 1; + estat = 1; // 1 means rebuild is needed } } else if (!(np->n_flag & N_DOUBLE) && ((np->n_flag & N_PHONY) || (timespec_le(&np->n_tim, &dtim)))) { if (estat == 0) { - if (!sc_cmd) { - if (!doinclude) - bb_error_msg("nothing to be done for %s", np->n_name); - } else { + if (sc_cmd) estat = make1(np, sc_cmd, oodate, allsrc, dedup, impdep); - clock_gettime(CLOCK_REALTIME, &np->n_tim); - } + else if (!doinclude) + bb_error_msg("nothing to be done for %s", np->n_name); + didsomething = 1; } else if (!doinclude) { bb_error_msg("'%s' not built due to errors", np->n_name); } free(oodate); - } else if (didsomething) { + } + + if (didsomething) clock_gettime(CLOCK_REALTIME, &np->n_tim); - } else if (level == 0) { + else if (!quest && level == 0) printf("%s: '%s' is up to date\n", applet_name, np->n_name); - } + free(allsrc); free(dedup); return estat; diff --git a/testsuite/make.tests b/testsuite/make.tests index f52770438..7bb736ade 100755 --- a/testsuite/make.tests +++ b/testsuite/make.tests @@ -397,6 +397,45 @@ target: : hash # hash ' +# Austin Group defect report 875 clarifies certain aspects of the +# behaviour of inference rules. Study of this resulted in a number +# of changes to pdpmake. Since the issue at hand involves the use +# of chained inference rules it doesn't affect POSIX mode. +mkdir make.tempdir && cd make.tempdir || exit 1 +touch test.j test.k +testing "make proper handling of inference rules 1" \ + "make -f -" \ + ".j.l\n" "" ' +.SUFFIXES: .j .k .l +.j.l: + @echo .j.l +.k.l: + @echo .k.l +test.l: test.k +test.j: +test.k: +' +cd .. || exit 1; rm -rf make.tempdir 2>/dev/null + +# The actual test in the above-mentioned defect report was actually +# more demanding. It used the suffixes '.a .b .c'. This results in +# the built-in '.c.a' inference rule being introduced into the mix. +mkdir make.tempdir && cd make.tempdir || exit 1 +touch test.a test.b +testing "make proper handling of inference rules 2" \ + "make -f -" \ + ".a.c\n" "" ' +.SUFFIXES: .a .b .c +.a.c: + @echo .a.c +.b.c: + @echo .b.c +test.c: test.b +test.a: +test.b: +' +cd .. || exit 1; rm -rf make.tempdir 2>/dev/null + # Skip duplicate entries in $? and $^ mkdir make.tempdir && cd make.tempdir || exit 1 touch -t 202206171200 file1 file3 -- cgit v1.2.3-55-g6feb