bugmake - Bugs: bug #31430, Add support for the BSD...

 
 

bug #31430: Add support for the BSD "shell assignment" operator (!=)

Submitter:  None
Submitted:  Sun 24 Oct 2010 03:53:40 AM UTC
Votes: 1
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.0 Operating System:  Any
Fixed Release:  4.0 Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 22 Sep 2013 10:39:29 PM UTC, comment #8: 

Thanks so much for adding the BSD shell assignment operator to GNU make!!  I look forward to this and other improved-portability goodies (such as the new POSIX ::= assignment operator) in the next GNU make release.

David A. Wheeler <dwheeler>
Sun 22 Sep 2013 09:15:58 PM UTC, comment #7: 

The BSD != feature will be available in the next release of GNU make.  Thanks for the report.

Paul D. Smith <psmith>
Group administrator
Fri 01 Apr 2011 09:43:28 PM UTC, comment #6: 

I can confirm that removing whitespace works w/ gmake 3.81, 3.82 and BSD make. Thanks.

Ideally, the proposed patch will be incorporated and BSD != lines will be parsed in a compatible manner, eliminating the need for multiple lines for BSD make/gmake compatible makefiles.

Bruce Lilly <blilly>
Thu 31 Mar 2011 07:55:36 PM UTC, comment #5: 

Those lines were never ignored (GNU make never ignores malformed lines).  Instead, because the prefix "!" was not recognized, GNU make assumed that it was part of the variable name.  If you look at the output of GNU make -p with your sample makefile and search for "OS" you'll see a variable defined named "OS      !".  That's why it appears that the line has no effect because you never try to access that variable.

The change in GNU make 3.82 was that variable names containing whitespace were disallowed; this IS documented in the NEWS file as a backward-incompatibility:


* WARNING: Backward-incompatibility!
  As a result of parser enhancements, three backward-compatibility issues
  exist: first, a prerequisite containing an "=" cannot be escaped with a
  backslash any longer.  You must create a variable containing an "=" and
  use that variable in the prerequisite.  Second, variable names can no
  longer contain whitespace, unless you put the whitespace in a variable and
  use the variable.  Third, in previous versions of make it was sometimes
  not flagged as an error for explicit and pattern targets to appear in the
  same rule.  Now this is always reported as an error.


You can work around this by removing the whitespace in your makefile; changing it to use this:


OS:=       $(shell uname -s)
OS!=       uname -s


Then it will work again.  Not that I'm necessarily recommending this as a best practice or anything...

Paul D. Smith <psmith>
Group administrator
Thu 31 Mar 2011 03:20:39 PM UTC, comment #4: 

Note that gmake 3.81 apparently silently ignored such lines, making cross-platform makefiles reasonably easy to write.
For example, consider the attached simple makefile:

on openSuSE 11.3 (gmake 3.81):
# make -f conf.compat
OS Linux

on FreeBSD 8.0-RELEASE (BSD make):
# make -f conf.compat
OS FreeBSD

on openSuSE 11.4 (gmake 3.82):
# make -f conf.compat
conf.compat:2: * missing separator.  Stop.

This will indeed "Stop" the rollout of openSuSE 11.4 here.

Note also that this change in behavior from 3.81 to 3.82 is an undocumented backwards incompatibility.

I suggest changing the Severity and Item Group  accordingly.

(file #23065)

Bruce Lilly <blilly>
Tue 26 Oct 2010 03:27:15 AM UTC, comment #3: 

I've updated the patch, so here is a new version of the patch:
 http://www.dwheeler.com/misc/shell-assignment.v2.patch

This new "version 2" uses the func_shell routine instead of popen(), so it should work anywhere that $(shell ...) works.  I also added documentation and even more tests.  All the tests, including the new ones, pass.  It should be "ready to go".



(file #21822)

David A. Wheeler <dwheeler>
Mon 25 Oct 2010 12:59:44 AM UTC, comment #2: 

Yes, please send me the paperwork, to dwheeler at dwheeler dot com.


David A. Wheeler <dwheeler>
Sun 24 Oct 2010 04:40:13 AM UTC, comment #1: 

I've nothing against adding != for portability.  However this level of work will require copyright assignment to the FSF.  Please let me know if you need paperwork/assistance.

Cheers!

Paul D. Smith <psmith>
Group administrator
Sun 24 Oct 2010 03:53:40 AM UTC, original submission:  

GNU make shell assignment patch by David A. Wheeler, 2010-10-23

I propose adding support for the BSD "shell assignment" operator, and I've attached a patch to add this support.

Many "make" implementations support the shell assignment operator "!=".  This includes the "make" of FreeBSD, OpenBSD, and NetBSD:
http://www.freebsd.org/cgi/man.cgi?query=make&sektion=1
http://www.openbsd.org/cgi-bin/man.cgi?query=make&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html
http://netbsd.gw.com/cgi-bin/man-cgi?make+1+NetBSD-current
It is also supported by the "pmake" program of Adam de Boor:
http://www.freebsd.org/doc/en/books/pmake/variables.html

The semantics are not complicated.  Given:
  string1 != string2
The macro named string1 is a recursively-defined variable, with a value determined by the following process: First, string2 is immediately evaluated. Second, that resulting evaluation is passed to the shell for execution. Third, that result is modified removing a single trailing newline (if there is one), and then replacing all remaining newlines with spaces.

GNU make can't currently handle BSD makefiles with "!=", since it doesn't have the operator.  It's not even easy to simulate with the current GNU make constructs.  In many cases, this GNU make construct will have the same final result:
  VAR = ${system echo "$(STUFF)" | sed -e 's/://'}
but notice that $(VAR) is re-evaluated EVERY TIME, creating efficiency problems, and the change in result may be surprising.  GNU make's ":=" doesn't quite do it either:
  VAR := ${system echo "$(STUFF)" | sed -e 's/://'}
In this case, in GNU make it's executed only once, while with BSD make the result is recursively evaluated.  Thus, in GNU make using := you cannot insert $(VAR2) as the output of the shell script and have it be re-evaluated.  Nothing in GNU make does this exactly... so those wanting this functionality will want it added.

The following patch adds this capability to GNU make, and includes test cases.  This patch works against GNU make per its CVS repository of 2010-10-23.

This patch is released under either GNU GPL version 2 or later, or
the BSD 3-clause license.  Thus, it's compatible with lots of licenses
including the current and older licenses used by GNU make.

This is also available from:
 http://www.dwheeler.com/misc/shell-assignment.patch


==================================================

--- ./read.c.orig 2010-08-13 22:50:14.000000000 -0400
+++ ./read.c 2010-10-23 18:52:42.285751829 -0400
@@ -2470,7 +2470,7 @@
      w_colon        A colon
      w_dcolon       A double-colon
      w_semicolon    A semicolon
-     w_varassign    A variable assignment operator (=, :=, +=, or ?=)
+     w_varassign    A variable assignment operator (=, :=, +=, ?=, or !=)
 
    Note that this function is only used when reading certain parts of the
    makefile.  Don't use it where special rules hold sway (RHS of a variable,
@@ -2521,6 +2521,7 @@
 
     case '+':
     case '?':
+    case '!':
       if (*p == '=')
         {
           ++p;
@@ -2540,7 +2541,7 @@
 
   /* This is some non-operator word.  A word consists of the longest
      string of characters that doesn't contain whitespace, one of [:=#],
-     or [?+]=, or one of the chars in the DELIM string.  */
+     or [?+!]=, or one of the chars in the DELIM string.  */
 
   /* We start out assuming a static word; if we see a variable we'll
      adjust our assumptions then.  */
--- ./tests/scripts/features/shell_assignment.orig 2010-10-23 23:14:22.797322744 -0400
+++ ./tests/scripts/features/shell_assignment 2010-10-23 23:11:33.714947475 -0400
@@ -0,0 +1,48 @@
+#                                                                    -*-perl-*-
+
+$description = "Test BSD-style shell assignments (VAR != VAL) for variables.";
+
+$details = "";
+
+# TEST 0: Basic shell assignment (!=).
+
+run_make_test('
+.POSIX:
+
+demo1!=printf \'  1   2 3\n4\n\n5 \n \n 6\n\n\n\n\'
+demo2 != printf \'7 8\n \'
+demo3 != printf \'$$(demo2)\'
+all: ; @echo "<$(demo1)> <$(demo2)> <$(demo3)>"
+',
+              '', "<  1   2 3 4  5     6   > <7 8  > <7 8  >\n");
+
+# TEST 1: Handle '#' the same way as BSD make
+
+run_make_test('
+foo1!=echo bar#baz
+hash != printf \'\043\'
+foo2!= echo "bar$(hash)baz"
+
+all: ; @echo "<$(foo1)> <$(hash)> <$(foo2)>"
+',
+              '', "<bar> <#> <bar#baz>\n");
+
+# TEST 2: shell assignment variables (from !=) should be recursive.
+# Note that variables are re-evaluated later, so the shell can output
+# a value like $(XYZZY) as part of !=.  The $(XYZZY) will be EVALUATED
+# when the value containing it is evaluated.  On the negative side, this
+# means if you don't want this, you need to escape dollar signs as $$.
+# On the positive side, it means that shell programs can output macros
+# that are then evaluated as they are traditionally evaluated.. and that
+# you can use traditional macro evaluation semantics to implement !=.
+
+run_make_test('
+XYZZY = fiddle-dee-dee
+dollar = $$
+VAR3 != printf \'%s\' \'$(dollar)(XYZZY)\'
+
+all: ; @echo "<$(VAR3)>"
+',
+              '', "<fiddle-dee-dee>\n");
+
+1;
--- ./variable.c.orig 2010-08-27 11:01:42.000000000 -0400
+++ ./variable.c 2010-10-23 22:45:33.899545348 -0400
@@ -1111,6 +1111,52 @@
   return var;
 }
 
+/* Given a string, shell-execute it and return a malloc'ed string of the
+ result. If it fails, returns NULL. /
+
+char *
+shell_result (const char *p)
+{
+  size_t length = 0;     /* Length of actual result */
+  size_t alloc_size = 0; /* size of buffer we've allocated */
+  FILE *result_file;
+  char buffer = NULL;   / We read our results into this */
+  size_t length_read;
+  unsigned i;
+
+  result_file = popen(p, "r");
+  if (!result_file)
+    return NULL;
+
+  while (!feof(result_file) && !ferror(result_file)) {
+    if (length >= alloc_size) { /* Need to (re)allocate a result buffer */
+      alloc_size += 8192; /* This size amount is arbitrary */
+      buffer = xrealloc(buffer, alloc_size); /* No return on fail */
+    }
+
+    length_read = fread(buffer + length, (size_t) 1, (size_t)
+                     (size_t) (alloc_size - length), result_file);
+    /* Defend against fread implementations that incorrectly return -1: */
+    if (length_read != (size_t) -1)
+      length += length_read;
+  }
+  pclose(result_file);
+
+  /* Per http://austingroupbugs.net/view.php?id=337 the semantics are:
+     result is modified removing a single trailing newline (if there is one),
+     and then replacing all remaining newlines with spaces." */
+  if (length > 0 && buffer[length-1]=='\n') {
+    buffer[length-1] = '\0';
+    length--;
+  }
+  for (i = 0; i < length; i++)
+    if (buffer[i] == '\n')
+      buffer[i] = ' ';
+
+  return buffer;
+}
+
+
 /* Given a variable, a value, and a flavor, define the variable.
    See the try_variable_definition() function for details on the parameters. */
 
@@ -1120,7 +1166,9 @@
                         enum variable_flavor flavor, int target_var)
 {
   const char *p;
+  char *q;
   char *alloc_value = NULL;
+  char *alloc_value2 = NULL;
   struct variable *v;
   int append = 0;
   int conditional = 0;
@@ -1140,6 +1188,11 @@
          target-specific variable.  */
       p = alloc_value = allocated_variable_expand (value);
       break;
+    case f_shell:
+      q = alloc_value = allocated_variable_expand (value);
+      p = alloc_value2 = shell_result (q);
+      flavor = f_recursive;
+      break;
     case f_conditional:
       /* A conditional variable definition "var ?= value".
          The value is set IFF the variable is not defined yet. */
@@ -1357,6 +1410,8 @@
 
   if (alloc_value)
     free (alloc_value);
+  if (alloc_value2)
+    free (alloc_value2);
 
   return v->special ? set_special_var (v) : v;
 }
@@ -1432,7 +1487,7 @@
    return (char *)p;
  }
 
-      /* Match assignment variants (:=, +=, ?=)  */
+      /* Match assignment variants (:=, +=, ?=, !=)  */
       if (*p == '=')
         {
           switch (c)
@@ -1446,6 +1501,9 @@
               case '?':
                 *flavor = f_conditional;
                 break;
+              case '!':
+                *flavor = f_shell;
+                break;
               default:
                 /* If we skipped whitespace, non-assignments means no var.  */
                 if (wspace)
--- ./variable.h.orig 2010-07-12 21:20:43.000000000 -0400
+++ ./variable.h 2010-10-23 11:08:36.036623200 -0400
@@ -38,7 +38,8 @@
     f_simple,           /* Simple definition (:=) */
     f_recursive,        /* Recursive definition (=) */
     f_append,           /* Appending definition (+=) */
-    f_conditional       /* Conditional definition (?=) */
+    f_conditional,      /* Conditional definition (?=) */
+    f_shell             /* Shell definition (!=) */
   };
 
 /* Structure that represents one variable definition.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #23065:  conf.compat added by blilly (86B - application/octet-stream - sample cross-platform compatibility test makefile)
file #21822:  shell-assignment.v2.patch added by dwheeler (14KiB - application/octet-stream)
file #21781:  shell-assignment.patch added by None (9KiB - application/octet-stream - I entered this earlier under "support", but it probably belongs here instead)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by blilly (Updated the item)
  • -email is unavailable- added by blilly
  • -email is unavailable- added by dwheeler (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  •  

    There is 1 vote 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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-09-22 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.0
    2011-03-31 blilly Attached File- Added conf.compat, #23065
        Carbon-Copy- Added blilly
    2010-10-26 dwheeler Attached File- Added shell-assignment.v2.patch, #21822
    2010-10-24 None Attached File- Added shell-assignment.patch, #21781

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code