bug-make
[Top][All Lists]
Advanced

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

Enhancement request regarding make variable names


From: Dale R. Worley
Subject: Enhancement request regarding make variable names
Date: Fri, 9 Jan 2004 13:41:45 -0500

[This enhancement request is for Gnu make 3.79.1.]

It would be a useful thing if Make variable names were forbidden to
contain slash (`/'), as well as `:', `#', and `='.  The reason I make
this request is that I was rebuilding a Linux kernel in a directory
named (more or less) `/auto/LABEL=Backup/linux-2.4.20-24.8.test4'.
The Linux kernel makefile expands out the full path of the files it is
working with, rather than using relative names, so at one point, it
generated a Makefile rule looking something like this:

cciss.h: \
   /auto/LABEL=Backup/linux-2.4.20-24.8.test4/include/linux/genhd.h \
   /auto/LABEL=Backup/linux-2.4.20-24.8.test4/drivers/block/cciss_cmd.h
        @touch cciss.h

Because the first non-whitespace thing after the `:' contained an `=',
Make interpreted it as a target-specific variable assignment, not a
rule.  Due to the organization of the Linux kernel makefiles, it is
difficult to avoid this problem without moving the file tree to a
different place.

Following is a patch which should forbid `/' in variable names (in
most circumstances), and thus allow `=', `+=', and `?=' in path names
after the first `/'.  This does not permit `:=' in path names, whose
interpretation is much more complex.  But that problem is no worse
than before.

The patch includes a new test, variables/names_equal, to verify that
paths containing `=' can be used.  The patched code passes all the
existing tests as well.

The patch adds comments to mark a couple of cases in switches that
fall through to the next case, which is good coding style.

In addition, the patch reorganizes a "case ':'" in read.c to allow
easy insertion of code to handle `:=' when someone feels up to writing
it.  The new behavior is different than before -- previously, if a
device-letter is used on MS-DOS, the "case ':'" falls through to the
following "case '$'", which appears to be incorrect behavior, even
though it may produce the correct result in most cases.  (E.g., it
looks like the path `C:$(variable)' will not be variable-substituted
correctly.)

The patch modifies the texinfo to specify that `/' is forbidden in
variable names, and why.

Dale
----------------------------------------------------------------------
diff -r --new-file -u --exclude='*~' --exclude=work 
make-3.79.1.orig/make.texinfo make-3.79.1/make.texinfo
--- make-3.79.1.orig/make.texinfo       2000-06-20 10:00:17.000000000 -0400
+++ make-3.79.1/make.texinfo    2004-01-09 11:50:11.000000000 -0500
@@ -4000,11 +4000,13 @@
 programs to run, directories to look in for source files, directories to
 write output in, or anything else you can imagine.
 
-A variable name may be any sequence of characters not containing @samp{:},
address@hidden, @samp{=}, or leading or trailing whitespace.  However,
-variable names containing characters other than letters, numbers, and
-underscores should be avoided, as they may be given special meanings in the
-future, and with some shells they cannot be passed through the environment to a
+A variable name may be any sequence of characters not containing
address@hidden:}, @samp{#}, @samp{=}, @samp{/} or leading or trailing
+whitespace.  (@samp{/} is forbidden so that path names containing
address@hidden can be handled.)  However, variable names containing
+characters other than letters, numbers, and underscores should be
+avoided, as they may be given special meanings in the future, and with
+some shells they cannot be passed through the environment to a
 address@hidden
 (@pxref{Variables/Recursion, ,Communicating Variables to a address@hidden).
 
diff -r --new-file -u --exclude='*~' --exclude=work make-3.79.1.orig/read.c 
make-3.79.1/read.c
--- make-3.79.1.orig/read.c     2000-06-21 15:33:30.000000000 -0400
+++ make-3.79.1/read.c  2004-01-09 13:00:25.000000000 -0500
@@ -2264,6 +2264,10 @@
   enum make_word_type wtype = w_bogus;
   char *p = buffer, *beg;
   char c;
+  /* 1 if we have seen a character which cannot be part of a variable name,
+     and thus if we seen `=', it is part of the token, not a variable
+     definition.  */
+  int non_variable_character_seen = 0;
 
   /* Skip any leading whitespace.  */
   while (isblank ((unsigned char)*p))
@@ -2313,6 +2317,7 @@
           wtype = w_varassign;
           break;
         }
+      /* Falls through to next case.  */
 
     default:
       if (delim && strchr (delim, c))
@@ -2326,7 +2331,12 @@
 
   /* 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.
+     In addition, `=' is permitted if the preceding characters contain
+     a character (currently, only `/' has this role) specifically excluded from
+     variable names.
+     In addition, on MS-DOS/MS-Windows systems, `:' is permitted in certain
+     circumstances as a drive specifier.  */
 
   /* We start out assuming a static word; if we see a variable we'll
      adjust our assumptions then.  */
@@ -2343,19 +2353,28 @@
         case '\0':
         case ' ':
         case '\t':
-        case '=':
         case '#':
           goto done_word;
 
         case ':':
+         /* A word can include a colon under certain circumstances.  The
+            following clauses specify when it can.  */
+         if (0
 #if defined(__MSDOS__) || defined(WINDOWS32)
-         /* A word CAN include a colon in its drive spec.  The drive
-            spec is allowed either at the beginning of a word, or as part
-            of the archive member name, like in "libfoo.a(d:/foo/bar.o)".  */
-         if (!(p - beg >= 2
-               && (*p == '/' || *p == '\\') && isalpha ((unsigned char)p[-2])
-               && (p - beg == 2 || p[-3] == '(')))
+             /* A word can include a colon in its drive spec.  The drive
+                spec is allowed either at the beginning of a word, or as part
+                of the archive member name, like in
+                "libfoo.a(d:/foo/bar.o)".  */
+             || (p - beg >= 2
+                 && (*p == '/' || *p == '\\') && isalpha ((unsigned char)p[-2])
+                 && (p - beg == 2 || p[-3] == '('))
 #endif
+               )
+           /* If any clause is true, continue reading characters of the
+              word.  */
+           break;
+         /* If no clause is true, this `:' is a special character and should
+            not be included in the word.  */
          goto done_word;
 
         case '$':
@@ -2389,7 +2408,7 @@
 
         case '?':
         case '+':
-          if (*p == '=')
+          if (!non_variable_character_seen && *p == '=')
             goto done_word;
           break;
 
@@ -2405,7 +2424,17 @@
             }
           break;
 
+        case '=':
+          if (!non_variable_character_seen)
+           goto done_word;
+         /* Fall through to next case.  */
+
         default:
+         /* Currently, `/' is the only character specifically excluded from
+            variable names (that does not have additional syntax that is
+            already handled).  */
+         if (c == '/')
+           non_variable_character_seen = 1;
           if (delim && strchr (delim, c))
             goto done_word;
           break;
diff -r --new-file -u --exclude='*~' --exclude=work 
make-3.79.1.orig/tests/scripts/variables/names_equal 
make-3.79.1/tests/scripts/variables/names_equal
--- make-3.79.1.orig/tests/scripts/variables/names_equal        1969-12-31 
19:00:00.000000000 -0500
+++ make-3.79.1/tests/scripts/variables/names_equal     2004-01-09 
12:47:46.000000000 -0500
@@ -0,0 +1,48 @@
+#                                                                    -*-perl-*-
+
+$description = "Test that variable names may not contain `/', specifically in 
target-specific variable definitions.";
+
+$details = "";
+
+open(MAKEFILE, "> $makefile");
+
+# The Contents of the MAKEFILE ...
+
+# Prepare a makefile with rules that contain the variable assignment
+# operators `=', `+=' and `?=' within paths.
+# Note that `:=' is NOT allowed, since that case involves the messy semantics
+# of `:', which is a problem that is not addressed in this test.
+
+print MAKEFILE <<'EOF';
+all : target1 target2 target3
+
+target1 : ./=a
+       @echo processing target1
+
+./=a :
+       @echo processing ./=a
+
+target2 : ./+=a
+       @echo processing target2
+
+./+=a :
+       @echo processing ./+=a
+
+target3 : ./?=a
+       @echo processing target3
+
+./?=a :
+       @echo processing ./?=a
+EOF
+
+# END of Contents of MAKEFILE
+
+close(MAKEFILE);
+
+# TEST #1
+# -------
+
+&run_make_with_options($makefile, "", &get_logfile);
+$answer = "processing ./=a\nprocessing target1\nprocessing ./+=a\nprocessing 
target2\nprocessing ./?=a\nprocessing target3\n";
+&compare_output($answer, &get_logfile(1));
+1;
diff -r --new-file -u --exclude='*~' --exclude=work make-3.79.1.orig/variable.c 
make-3.79.1/variable.c
--- make-3.79.1.orig/variable.c 2000-05-19 12:36:08.000000000 -0400
+++ make-3.79.1/variable.c      2004-01-09 12:18:34.000000000 -0500
@@ -798,7 +798,9 @@
   while (1)
     {
       c = *p++;
-      if (c == '\0' || c == '#')
+      /* `/' is now forbidden in variable names so that paths containing `='
+        (at least, after the first `/') can be processed.  */
+      if (c == '\0' || c == '#' || c == '/')
        return 0;
       if (c == '=')
        {
----------------------------------------------------------------------




reply via email to

[Prev in Thread] Current Thread [Next in Thread]