tech-toolchain archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

make: avoid breaking backwards compatability



The recent changes to suffix white-space handling are cool,
but they break a number of existing makefiles.

I've been doing some testing building Junos - which is a decent torture
test.  It has been built with bmake for 15 years and there isn't a
single feature that isn't leveraged.

While each branch knows which bmake version it should use, to the extent
possible we want all dev builds to use the latest deployed version.
With 100's of active branches, fixing makefiles to accommodate changes in
bmake is generally not a viable option.

While working on converting FreeBSD to use bmake, backwards
compatability was a major issue too.
While fixing FreeBSD itself to build with bmake was quite simple, there are
many downstream vendors who use it and have their own code base derrived
from it or imply using the BSD build infra.
I'm sure this is true of NetBSD as well.

Changing makefiles we have access to is one thing, but there are far more
makefiles out there we have no access to.

A target that has a \ at the end of the last line of its script now breaks.
Such targets were valid until recently and the practice is not uncommonn
since it reduces noise in diff's when additional commands added
(eg. great log sed commands)

--------------------8<--------------------
all: sedit

_this:= ${MAKEFILE}
.ifdef WITH_FINAL_SLASH
bs= \ 
.endif

.OBJDIR: /tmp

sedit:
        sed < ${_this} \
        -e 's,sed,SED,g' ${bs}
--------------------8<--------------------

The other case I hit was where SRCS contains sub/file.c 
sometimes adding the dir where sub/file.c lives to .PATH isn't an option
because of other files present that are not the desired ones.

Until recently

foo.o:  subdir/foo.c

worked, but now even after subdir/foo.c has been correctly found
a search for foo.c is made and overrides the previous result - which can
cause the wrong foo.c to be compiled.
Sometimes this can all be sorted out by tweaking .PATH, but again only
if you have access to the makefile that needs fixing.

The patch below adds a knob "POSIX_STRICT" (better names welcome),
that enables the new problematic functionality, and as a concequence
retains backwards compability for the two cases above.

I'd prefer to put something like this into NetBSD make rather than bmake
directly. 

--sjg

diff -r 60c0e077d44f bmake/compat.c
--- a/bmake/compat.c    Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/compat.c    Mon Sep 01 11:26:12 2014 -0700
@@ -191,6 +191,33 @@
 
 /*-
  *-----------------------------------------------------------------------
+ * strip_cmd --
+ *     strip leading white-space
+ *     and trailing white-space unless posix_strict is set
+ *
+ * Side Effects:
+ *     The input may be truncated.
+ *
+ *-----------------------------------------------------------------------
+ */
+char *
+strip_cmd(char *cmd)
+{
+    while (isspace((unsigned char)*cmd))
+       cmd++;
+    if (!posix_strict) {
+       char *cp;
+
+       cp = cmd + strlen(cmd) - 1;
+       while (cp > cmd && (*cp == '\\' || isspace((unsigned char)*cp))) {
+           *cp-- = '\0';
+       }
+    }
+    return cmd;
+}
+    
+/*-
+ *-----------------------------------------------------------------------
  * CompatRunCommand --
  *     Execute the next command for a target. If the command returns an
  *     error, the node's made field is set to ERROR and creation stops.
@@ -278,8 +305,7 @@
        cmd++;
     }
 
-    while (isspace((unsigned char)*cmd))
-       cmd++;
+    cmd = strip_cmd(cmd);
 
     /*
      * If we did not end up with a command, just skip it.
diff -r 60c0e077d44f bmake/job.c
--- a/bmake/job.c       Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/job.c       Mon Sep 01 11:26:12 2014 -0700
@@ -874,8 +874,7 @@
        cmd++;
     }
 
-    while (isspace((unsigned char) *cmd))
-       cmd++;
+    cmd = strip_cmd(cmd);
 
     /*
      * If the shell doesn't have error control the alternate echo'ing will
diff -r 60c0e077d44f bmake/main.c
--- a/bmake/main.c      Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/main.c      Mon Sep 01 11:26:12 2014 -0700
@@ -195,6 +195,7 @@
 char *makeDependfile;
 pid_t myPid;
 int makelevel;
+int posix_strict;
 
 Boolean forceJobs = FALSE;
 
@@ -806,6 +807,8 @@
     if (!mode)
        mode = mp = Var_Subst(NULL, "${" MAKE_MODE ":tl}", VAR_GLOBAL, 0);
 
+    posix_strict = getBoolean("POSIX_STRICT", 0);
+
     if (mode && *mode) {
        if (strstr(mode, "compat")) {
            compatMake = TRUE;
diff -r 60c0e077d44f bmake/make.h
--- a/bmake/make.h      Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/make.h      Mon Sep 01 11:26:12 2014 -0700
@@ -469,6 +469,7 @@
 extern char    **savedEnv;      /* if we replaced environ this will be 
non-NULL */
 
 extern int     makelevel;
+extern int     posix_strict;
 
 /*
  * We cannot vfork() in a child of vfork().
diff -r 60c0e077d44f bmake/nonints.h
--- a/bmake/nonints.h   Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/nonints.h   Mon Sep 01 11:26:12 2014 -0700
@@ -85,6 +85,7 @@
 int Arch_IsLib(GNode *);
 
 /* compat.c */
+char *strip_cmd(char *);
 int CompatRunCommand(void *, void *);
 void Compat_Run(Lst);
 int Compat_Make(void *, void *);
diff -r 60c0e077d44f bmake/suff.c
--- a/bmake/suff.c      Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/suff.c      Mon Sep 01 11:26:12 2014 -0700
@@ -1680,6 +1680,29 @@
     Lst_ForEach(target->suff->children, SuffAddSrc, &ls);
 }
 
+struct suff_hit_s {
+    char *want;
+    int len;
+    GNode *node;
+};
+
+/* does GNode name end with the wanted string ? */
+static int
+suffTailMatch(void *np, void *wp)
+{
+    GNode *gn = np;
+    struct suff_hit_s *ws = wp;
+    char *cp;
+    
+    if ((cp = strstr(gn->name, ws->want)) &&
+       cp[ws->len] == '\0') {
+       ws->node = gn;
+       return 1;                       /* stop searching */
+    }
+    return 0;
+}
+
+
 /*-
  *-----------------------------------------------------------------------
  * SuffFindThem -- find a transformation chain from a list of possibilities.
@@ -1738,6 +1761,27 @@
        if (parent != i->parent) {
            SuffDebugChain(i->parent);
            parent = i->parent;
+           if (!posix_strict && parent->node) {
+               struct suff_hit_s sh;
+
+               sh.want = i->file;
+               sh.len = strlen(i->file);
+               sh.node = NULL;
+
+               Lst_ForEach(parent->node->children, suffTailMatch, &sh);
+               if ((n = sh.node)) {
+                   SuffDebug("%s: has child: %s -> %s\n",
+                             parent->node->name, i->file, n->name);
+                   result = SuffNewSrc(
+                                       bmake_strdup(n->path ? n->path : 
n->name),      
+                                       bmake_strdup(i->pref),
+                                       i->suff,
+                                       sh.node,
+                                       i->parent);
+                   Lst_AtEnd(cleanup, result);
+                   break;
+               }
+           }
        }
        SuffDebug ("\t%*.s%s: ", i->depth, " ", i->file);
 
@@ -2528,6 +2572,8 @@
            free(target->path);
            target->path = Dir_FindFile(target->name,
                (suffix == NULL ? dirSearchPath : suffix->searchPath));
+           if (target->path)
+               SuffDebug("found: %s\n", target->path);
        }
     }
 
diff -r 60c0e077d44f bmake/unit-tests/escape.mk
--- a/bmake/unit-tests/escape.mk        Sat Aug 30 16:06:21 2014 -0700
+++ b/bmake/unit-tests/escape.mk        Mon Sep 01 11:26:12 2014 -0700
@@ -2,6 +2,8 @@
 #
 # Test backslash escaping.
 
+POSIX_STRICT=1
+
 # Extracts from the POSIX 2008 specification
 # <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/make.html>:
 #


Home | Main Index | Thread Index | Old Index