Am 13.07.19 um 14:46 schrieb Paul Smith:

On Wed, 2019-07-10 at 18:15 +0200, Christof Warlich wrote:
Am 10.07.19 um 14:44 schrieb Paul Smith:
Let me know how you want to proceed.
As I want to tackle the most challenging part first, I'd like to suggest to
do the copyright assignment after the technical side is in a decent shape.
But as I said, no problem to do it fist if that's more appropriate.
That's no problem.  However you prefer is fine with me.

Copyright assignment these days is pretty straightforward and can be
done via email instead of sending paper documents around like in the
old days, so it's not as long a process.

One item to note: if you work for a company which has rights to your
work, then they would need to sign off on this as well.

Cheers!

Hi,

attached is the comprehensive (and significantly reworked) patch that
implements the feature of a new internal variable ( I called it
.PREREQUISITES)
allowing to add prerequisites that do not contribute to the automatic list
variables, including the related docu and tests.

From my perspective, everything is complete and works as it should, so I
would definitely need your feedback if there is anything missing or wrong.

Otherwise (or in parallel), could you guide me to get the copyright
assignment
done so that the patch could be included?

In the hope to get some response,

Chris



diff --git a/doc/make.texi b/doc/make.texi
index 12f70c6..777c5a6 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -6627,6 +6627,77 @@ Supports dynamically loadable objects for creating custom extensions.
 Expands to a list of directories that @code{make} searches for
 included makefiles (@pxref{Include, , Including Other Makefiles}).

+@vindex .PREREQUISITES @r{(list of prerequsisites not contributing to automatic variables)}
+@item .PREREQUISITES
+This variable may be set to a list of prerequisites that are added
+to the list of existing prerequisites. But in contrast to these existing
+prerequisites, the ones listed in @code{$(.PREREQUISITES)} do not
+contribute to any of the list-type automatic variables.
+Thus, they are not added to any of the automatic variable lists @code{$^},
+@code{$+}, @code{$?}, @code{$*}, @code{$|}, @code{$(^F)}, @code{$(+F)},
+@code{$(?F)}, @code{$(*F)}, @code{$(^D)}, @code{$(+D)}, @code{$(?D)} and
+@code{$(*D)}.
+
+The rationale behind this variable is to make it easier to extend dependency
+lists of existing Makefiles. The following example illustrates this:
+
+The code below may be considered as a snippet of a large and
+possibly rather complex Makefile:
+
+@example
+myappl: main.o file1.o file2.o
+       gcc -o $@ $^
+@end example
+
+At a first glance, it lists all the relevant prerequisites, but a
+second thought reveals that this is just not true: The target
+also depends on the compiler frontend, the linker backend
+and the Makefile itself.
+
+Thus, a more complete snippet should look more like this:
+
+@example
+myappl: main.o file1.o file2.o /usr/bin/gcc /usr/bin/ld Makefile
+       gcc -o $@ $(filter %.o,$^)
+@end example
+
+Please note the need for GNU Make's $(filter) function besides the
+additional prerequisites.
+
+But for big projects, say the Linux kernel or a toolchain build,
+it would be rather laborious to change and fix all the Makefiles
+accordingly, and it is more than questionable if such patches
+would be welcomed by every project. Fortunately, with @code{.PREREQUISITES}
+at hand, the upstream Makefiles do not need to be changed at all.
+Instead, it is sufficient to assign the additional dependencies to
+a target-specific instance of @code{.PREREQUISITES} in another Makefile
+that simply includes the upstream Makefile. To continue with our
+example (and assuming the related upstream Makefile was just called
+@code{Makefile}), we could most conveniently add a @code{GNUmakefile}
+with the following content:
+
+@example
+include Makefile
+myappl: .PREREQUISITES:=/usr/bin/gcc /usr/bin/ld Makefile
+@end example
+
+Calling @code{make} now would prefer @code{GNUmakefile} over
+@code{Makefile}, thus respecting the additional prerequisites
+without affecting the automatic variable @code{$^} in the related
+recipe.
+
+So far, we have only discussed the implications of @code{.PREREQUISITES}
+as a target specific variable. But it may certainly also be defined
+globally, causing all targets that do not define a target specific instance
+of @code{.PREREQUISITES} to depend on the prerequisites listed in that
+global instance of @code{.PREREQUISITES}. But as prerequisites are usually
+targets themselves, this would create circular dependencies for those
+targets. Thus, it would be required to also define an empty target specific
+instance of @code{.PREREQUISITES} for each member in the global
+@code{.PREREQUISITES} list to avoid that make complains about circular
+dependencies. To avoid this extra noise, make isself just discards any
+of these circular dependencies.
+
 @end table

 @node Conditionals, Functions, Using Variables, Top
diff --git a/src/commands.c b/src/commands.c
index dd47851..7c08c01 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -178,7 +178,7 @@ set_file_variables (struct file *file)
     bar_len = 0;
     for (d = file->deps; d != 0; d = d->next)
       {
-        if (!d->need_2nd_expansion)
+        if (!d->need_2nd_expansion && !d->ignore_automatic_vars)
           {
             if (d->ignore_mtime)
               bar_len += strlen (dep_name (d)) + 1;
@@ -200,7 +200,7 @@ set_file_variables (struct file *file)

     qmark_len = plus_len + 1;   /* Will be this or less.  */
     for (d = file->deps; d != 0; d = d->next)
-      if (! d->ignore_mtime && ! d->need_2nd_expansion)
+      if (! d->ignore_mtime && ! d->need_2nd_expansion && ! d->ignore_automatic_vars)
         {
           const char *c = dep_name (d);

@@ -247,7 +247,7 @@ set_file_variables (struct file *file)

     for (d = file->deps; d != 0; d = d->next)
       {
-        if (d->need_2nd_expansion)
+        if (d->need_2nd_expansion || d->ignore_automatic_vars)
           continue;

         slot = hash_find_slot (&dep_hash, d);
@@ -269,7 +269,7 @@ set_file_variables (struct file *file)
       {
         const char *c;

-        if (d->need_2nd_expansion || hash_find_item (&dep_hash, d) != d)
+        if (d->need_2nd_expansion || d->ignore_automatic_vars || hash_find_item (&dep_hash, d) != d)
           continue;

         c = dep_name (d);
diff --git a/src/dep.h b/src/dep.h
index baa64df..bedf41a 100644
--- a/src/dep.h
+++ b/src/dep.h
@@ -48,7 +48,8 @@ struct nameseq
     unsigned short changed : 1;                 \
     unsigned short ignore_mtime : 1;            \
     unsigned short staticpattern : 1;           \
-    unsigned short need_2nd_expansion : 1
+    unsigned short need_2nd_expansion : 1;      \
+    unsigned short ignore_automatic_vars : 1

 struct dep
   {
diff --git a/src/file.c b/src/file.c
index c20fcf8..3811efb 100644
--- a/src/file.c
+++ b/src/file.c
@@ -653,6 +653,89 @@ reset_updating (const void *item)
   f->updating = 0;
 }

+/* Add dependencies from  .PREREQUISITES. */
+
+static void
+add_prerequisite_values (struct file *f, const struct variable *v)
+{
+  struct dep *prereqs_only = split_prereqs (variable_expand(v->value));
+  if (prereqs_only)
+    {
+      struct dep *d = prereqs_only;
+      int circular = 0;
+      while (d)
+        {
+          struct file *df = lookup_file (d->name);
+          if (df)
+            {
+              if (strcmp (f->name, d->name))
+                {
+                  d->name = NULL;
+                  d->file = df;
+                  d->ignore_automatic_vars = 1;
+                }
+              else /* Skip circular dependencies. */
+                {
+                  circular = 1;
+                  break;
+                }
+            }
+          else
+            {
+              /* Cause complain() to generate an appropriate error message. */
+              d->file = enter_file (d->name);
+              d->file->deps = NULL;
+            }
+          d = d->next;
+        }
+      if (circular)
+        free_dep_chain (prereqs_only);
+      else
+        {
+          /* Append dependencies found in target specific .PREREQUISITES to
+             existing dependencies.
+          */
+          d = prereqs_only;
+          if (f->deps)
+            {
+              d = f->deps;
+              while (d->next)
+                d = d->next;
+              d->next = prereqs_only;
+            }
+          else
+            f->deps = prereqs_only;
+        }
+    }
+}
+
+static void
+prerequisite_values (const void *item, void *arg)
+{
+  const struct variable *v = item;
+  struct file *f = arg;
+  if (!strcmp (v->name, ".PREREQUISITES"))
+    add_prerequisite_values(f, v);
+}
+
+static void
+prerequisite_targets (const void *item)
+{
+  const struct file *f = item;
+  if (f->variables != 0)
+    // FIXME: Casting away constness looks quite fishy: Is this ok?
+    hash_map_arg (&f->variables->set->table, prerequisite_values, (void *) item);
+  else
+    {
+      if (f->is_target)
+        {
+          struct variable *gv = lookup_variable(".PREREQUISITES", 14);
+          if(gv)
+            add_prerequisite_values((void *) item, gv);
+        }
+    }
+}
+
 /* For each dependency of each file, make the 'struct dep' point
    at the appropriate 'struct file' (which may have to be created).

@@ -779,6 +862,8 @@ snap_deps (void)
   if (f != 0 && f->is_target)
     not_parallel = 1;

+  /* Handle .PREREQUISITES variable. */
+  hash_map (&files, prerequisite_targets);
 #ifndef NO_MINUS_C_MINUS_O
   /* If .POSIX was defined, remove OUTPUT_OPTION to comply.  */
   /* This needs more work: what if the user sets this in the makefile?
diff --git a/tests/scripts/variables/PREREQUISITES b/tests/scripts/variables/PREREQUISITES
new file mode 100644
index 0000000..f8c4c4c
--- /dev/null
+++ b/tests/scripts/variables/PREREQUISITES
@@ -0,0 +1,131 @@
+#                                                                    -*-perl-*-
+
+$description = "Test the .PREREQUISITES special variable.";
+$details = "";
+
+# Basic test target specific .PREREQUISITES:
+run_make_test('
+DEPENDENCY_ONLY_PREREQUISITES = hi ho hey
+OTHER_DEPENDENCIES := foo bar baz
+target: .PREREQUISITES := ${DEPENDENCY_ONLY_PREREQUISITES}
+target: ${OTHER_DEPENDENCIES}
+	: ${.PREREQUISITES} $^
+.PHONY: target ${DEPENDENCY_ONLY_PREREQUISITES} ${OTHER_DEPENDENCIES}
+${DEPENDENCY_ONLY_PREREQUISITES} ${OTHER_DEPENDENCIES}:
+	touch $@
+',
+'',
+'touch foo
+touch bar
+touch baz
+touch hi
+touch ho
+touch hey
+: hi ho hey foo bar baz');
+
+# Test target specific .PREREQUISITES and pattern rules:
+unlink(qw(target.dst));
+run_make_test('
+all: target.dst
+DEPENDENCY_ONLY_PREREQUISITES = hi ho hey
+target.dst: .PREREQUISITES := ${DEPENDENCY_ONLY_PREREQUISITES}
+%.dst: %.src
+	: ${.PREREQUISITES} $^
+.PHONY: ${DEPENDENCY_ONLY_PREREQUISITES} target.src
+${DEPENDENCY_ONLY_PREREQUISITES} target.src:
+	touch $@
+',
+'',
+'touch target.src
+touch hi
+touch ho
+touch hey
+: hi ho hey target.src');
+
+# Test that global .PREREQUISITES are built first:
+run_make_test('
+.PREREQUISITES = hi ho hey
+OTHER_DEPENDENCIES := foo bar baz
+target: ${OTHER_DEPENDENCIES}
+	: ${.PREREQUISITES} $^
+.PHONY: target ${.PREREQUISITES} ${OTHER_DEPENDENCIES}
+${.PREREQUISITES} ${OTHER_DEPENDENCIES}:
+	touch $@
+',
+'',
+'touch hi
+touch ho
+touch hey
+touch foo
+touch bar
+touch baz
+: hi ho hey foo bar baz');
+
+# Test that target specific .PREREQUISITES override global .PREREQUISITES:
+run_make_test('
+.PREREQUISITES = tick tack
+DEPENDENCY_ONLY_PREREQUISITES = hi ho hey
+OTHER_DEPENDENCIES := foo bar baz
+target: .PREREQUISITES := ${DEPENDENCY_ONLY_PREREQUISITES}
+target: ${OTHER_DEPENDENCIES}
+	: ${.PREREQUISITES} $^
+.PHONY: target ${DEPENDENCY_ONLY_PREREQUISITES} ${OTHER_DEPENDENCIES} ${.PREREQUISITES}
+${DEPENDENCY_ONLY_PREREQUISITES} ${OTHER_DEPENDENCIES} ${.PREREQUISITES}:
+	touch $@
+',
+'',
+'touch tick
+touch tack
+touch foo
+touch bar
+touch baz
+touch hi
+touch ho
+touch hey
+: hi ho hey foo bar baz');
+
+# Test error reporting of global .PREREQUISITES:
+unlink(qw(tick tack));
+run_make_test('
+.PREREQUISITES = tick tack
+.PHONY: all
+all:
+	: ${.PREREQUISITES} $^
+',
+'',
+"make: *** No rule to make target 'tick', needed by 'all'.  Stop.",
+512);
+
+# Test error reporting of global .PREREQUISITES and keep-going:
+unlink(qw(tick tack));
+run_make_test('
+.PREREQUISITES = tick tack
+.PHONY: all
+all:
+	: ${.PREREQUISITES} $^
+',
+'-k',
+"make: *** No rule to make target 'tick', needed by 'all'.
+make: *** No rule to make target 'tack', needed by 'all'.
+make: Target 'all' not remade because of errors.",
+512);
+
+# Test error reporting of target specific .PREREQUISITES and keep-going:
+unlink(qw(tick tack));
+run_make_test('
+all: .PREREQUISITES = tick tack
+.PHONY: all
+all:
+	: ${.PREREQUISITES} $^
+',
+'-k',
+"make: *** No rule to make target 'tick', needed by 'all'.
+make: *** No rule to make target 'tack', needed by 'all'.
+make: Target 'all' not remade because of errors.",
+512);
+
+# Cleanup:
+unlink(qw(hi ho hey foo bar baz tick tack target.src));
+
+# This tells the test driver that the perl test script executed properly.
+1;
_______________________________________________
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

Reply via email to