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

 
 

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

Submitted by:  None
Submitted on:  Sun 24 Oct 2010 03:53:40 AM UTC  
Votes:  1  
 
Severity: 3 - NormalItem Group: Enhancement
Status: FixedPrivacy: Public
Assigned to: Paul D. Smith <psmith>Open/Closed: Closed
Component Version: 4.0Operating System: Any
Fixed Release: 4.0Triage Status: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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:

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

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

Paul D. Smith <psmith>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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 File(s):
   
   
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 (14kB - application/octet-stream)
file #21781:  shell-assignment.patch added by None (9kB - 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
  • -unavailable- added by blilly (Updated the item)
  • -unavailable- added by blilly
  • -unavailable- added by dwheeler (Posted a comment)
  • -unavailable- added by psmith (Posted a comment)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 1 encouragement so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 22 Sep 2013 09:15:58 PM UTCpsmithStatusNone=>Fixed
      Assigned toNone=>psmith
      Open/ClosedOpen=>Closed
      Fixed ReleaseNone=>4.0
    Thu 31 Mar 2011 03:20:39 PM UTCblillyAttached File-=>Added conf.compat, #23065
      Carbon-Copy-=>Added blilly
    Tue 26 Oct 2010 03:27:15 AM UTCdwheelerAttached File-=>Added shell-assignment.v2.patch, #21822
    Sun 24 Oct 2010 03:53:40 AM UTCNoneAttached File-=>Added shell-assignment.patch, #21781

    Back to the top


    Powered by Savane 3.1-cleanup