[PATCH 0/3] Eliminate the use of Autogen

[PATCH 0/3] Eliminate the use of Autogen

[Colin Watson]

I already mentioned this on IRC, but I thought it'd be helpful to send
it here as well.

This patch series (also in the cjwatson/new-autogen git branch)
eliminates the use of Autogen and instead generates Automake input
directly from gentpl.py.

Autogen's definitions files were useful, and I've maintained
compatibility with the relevant subset of them, but the template
language was cumbersome; beyond a certain level of complexity it became
necessary to use the Guile extensions, and the impedance mismatch
between those and the Python template generation was considerable.  It's
simpler, much faster, and very much less confusing to generate
Makefile.*.am directly.

I've checked that this generates almost identical output, even when the
modules from grub-extras are in use.  The differences amount to a few
unimportant things such as removal of trailing newlines, and the
movement of a declarations block for libgnulib.a a bit further up the
file; they are short enough to be easily reviewable and not significant
enough to be worth contorting the generator code to avoid.

Colin Watson (3):
  Generate Makefile.*.am directly from gentpl.py, eliminating the use of
    Autogen.  The Autogen definitions files remain intact as they offer
    a useful abstraction.
  Show file name in error messages from AutogenParser.
  Handle #if/#endif and C-style comments in AutoGen definitions files.

 .gitignore               |    1 -
 ChangeLog                |    6 +
 INSTALL                  |    1 -
 autogen.sh               |   11 +-
 conf/Makefile.common     |   15 +-
 conf/Makefile.extra-dist |    1 -
 gentpl.py                | 1050 ++++++++++++++++++++++++++++------------------
 7 files changed, 651 insertions(+), 434 deletions(-)

-- 
1.8.4.3

[PATCH 1/3] Generate Makefile.*.am directly from gentpl.py, eliminating the use of Autogen. The Autogen definitions files remain intact as they offer a useful abstraction.

[Colin Watson]

---
 .gitignore               |    1 -
 ChangeLog                |    6 +
 INSTALL                  |    1 -
 autogen.sh               |   11 +-
 conf/Makefile.common     |   15 +-
 conf/Makefile.extra-dist |    1 -
 gentpl.py                | 1007 +++++++++++++++++++++++++++-------------------
 7 files changed, 608 insertions(+), 434 deletions(-)

diff --git a/.gitignore b/.gitignore
index dc93061..2292cc9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -159,7 +159,6 @@ GPATH
 GRTAGS
 GSYMS
 GTAGS
-Makefile.tpl
 compile
 depcomp
 mdate-sh
diff --git a/ChangeLog b/ChangeLog
index 279cc23..88b3c83 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2013-11-26  Colin Watson  <cjwatson@ubuntu.com>
+
+	Generate Makefile.*.am directly from gentpl.py, eliminating the use
+	of Autogen.  The Autogen definitions files remain intact as they
+	offer a useful abstraction.
+
 2013-11-26  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	Add PCI command activation to all PCI drivers as required for coreboot
diff --git a/INSTALL b/INSTALL
index e81a095..cb9df30 100644
--- a/INSTALL
+++ b/INSTALL
@@ -55,7 +55,6 @@ need the following.
 * Python 2.5.2 or later
 * Autoconf 2.60 or later
 * Automake 1.10.1 or later
-* Autogen 5.10 or later
 
 Prerequisites for make-check:
 
diff --git a/autogen.sh b/autogen.sh
index 10fe68f..7424428 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -8,8 +8,6 @@ unset LC_ALL
 find . -iname '*.[ch]' ! -ipath './grub-core/lib/libgcrypt-grub/*' ! -ipath './build-aux/*' ! -ipath './grub-core/lib/libgcrypt/src/misc.c' ! -ipath './grub-core/lib/libgcrypt/src/global.c' ! -ipath './grub-core/lib/libgcrypt/src/secmem.c'  ! -ipath './util/grub-gen-widthspec.c' ! -ipath './util/grub-gen-asciih.c' |sort > po/POTFILES.in
 find util -iname '*.in' ! -name Makefile.in  |sort > po/POTFILES-shell.in
 
-autogen --version >/dev/null || exit 1
-
 echo "Importing unicode..."
 python util/import_unicode.py unicode/UnicodeData.txt unicode/BidiMirroring.txt unicode/ArabicShaping.txt grub-core/unidata.c
 
@@ -32,10 +30,7 @@ for x in mpi-asm-defs.h mpih-add1.c mpih-sub1.c mpih-mul1.c mpih-mul2.c mpih-mul
     ln -s generic/"$x" grub-core/lib/libgcrypt-grub/mpi/"$x"
 done
 
-echo "Creating Makefile.tpl..."
-python gentpl.py | sed -e '/^$/{N;/^\n$/D;}' > Makefile.tpl
-
-echo "Running autogen..."
+echo "Generating Automake input..."
 
 # Automake doesn't like including files from a path outside the project.
 rm -f contrib grub-core/contrib
@@ -59,8 +54,8 @@ for extra in contrib/*/Makefile.core.def; do
   fi
 done
 
-cat $UTIL_DEFS | autogen -T Makefile.tpl | sed -e '/^$/{N;/^\n$/D;}' > Makefile.util.am
-cat $CORE_DEFS | autogen -T Makefile.tpl | sed -e '/^$/{N;/^\n$/D;}' > grub-core/Makefile.core.am
+python gentpl.py $UTIL_DEFS > Makefile.util.am
+python gentpl.py $CORE_DEFS > grub-core/Makefile.core.am
 
 for extra in contrib/*/Makefile.common; do
   if test -e "$extra"; then
diff --git a/conf/Makefile.common b/conf/Makefile.common
index 9f0d1d6..d887df1 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -131,20 +131,15 @@ EXTRA_DIST =
 CLEANFILES =
 BUILT_SOURCES =
 
-# Rules for autogen definition files
-
-.PRECIOUS: $(top_srcdir)/Makefile.tpl
-$(top_srcdir)/Makefile.tpl: $(top_srcdir)/gentpl.py
-	python $< | sed -e '/^$$/{N;/^\\n$$/D;}' > $@.new || (rm -f $@.new; exit 1)
-	mv $@.new $@
+# Rules for Automake input
 
 .PRECIOUS: $(top_srcdir)/Makefile.util.am
-$(top_srcdir)/Makefile.util.am: $(top_srcdir)/Makefile.util.def $(top_srcdir)/Makefile.utilgcry.def $(top_srcdir)/Makefile.tpl
-	cat $(top_srcdir)/Makefile.util.def $(top_srcdir)/Makefile.utilgcry.def | autogen -T $(top_srcdir)/Makefile.tpl | sed -e '/^$$/{N;/^\\n$$/D;}' > $@.new || (rm -f $@.new; exit 1)
+$(top_srcdir)/Makefile.util.am: $(top_srcdir)/gentpl.py $(top_srcdir)/Makefile.util.def $(top_srcdir)/Makefile.utilgcry.def
+	python $^ > $@.new || (rm -f $@.new; exit 1)
 	mv $@.new $@
 
 .PRECIOUS: $(top_srcdir)/grub-core/Makefile.core.am
-$(top_srcdir)/grub-core/Makefile.core.am: $(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/grub-core/Makefile.gcry.def $(top_srcdir)/Makefile.tpl
+$(top_srcdir)/grub-core/Makefile.core.am: $(top_srcdir)/gentpl.py $(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/grub-core/Makefile.gcry.def
 	if [ "x$$GRUB_CONTRIB" != x ]; then echo "You need to run ./autogen.sh manually." >&2; exit 1; fi
-	cat $(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/grub-core/Makefile.gcry.def | autogen -T $(top_srcdir)/Makefile.tpl | sed -e '/^$$/{N;/^\\n$$/D;}' > $@.new || (rm -f $@.new; exit 1)
+	python $^ > $@.new || (rm -f $@.new; exit 1)
 	mv $@.new $@
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 72b00c0..e0c915c 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -2,7 +2,6 @@ EXTRA_DIST += autogen.sh
 EXTRA_DIST += geninit.sh
 
 EXTRA_DIST += gentpl.py
-EXTRA_DIST += Makefile.tpl
 EXTRA_DIST += Makefile.util.def
 EXTRA_DIST += Makefile.utilgcry.def
 
diff --git a/gentpl.py b/gentpl.py
index c135c5c..f0f4493 100644
--- a/gentpl.py
+++ b/gentpl.py
@@ -1,6 +1,6 @@
 #! /usr/bin/python
 #  GRUB  --  GRand Unified Bootloader
-#  Copyright (C) 2010,2011  Free Software Foundation, Inc.
+#  Copyright (C) 2010,2011,2012,2013  Free Software Foundation, Inc.
 #
 #  GRUB is free software: you can redistribute it and/or modify
 #  it under the terms of the GNU General Public License as published by
@@ -15,8 +15,13 @@
 #  You should have received a copy of the GNU General Public License
 #  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
 
+__metaclass__ = type
+
+from optparse import OptionParser
+import re
+
 #
-# This is the python script used to generate Makefile.tpl
+# This is the python script used to generate Makefile.*.am
 #
 
 GRUB_PLATFORMS = [ "emu", "i386_pc", "i386_efi", "i386_qemu", "i386_coreboot",
@@ -97,80 +102,365 @@ for platform in GRUB_PLATFORMS:
         RMAP[platform] = smaller + [ k ] + bigger
 
 #
+# Input
+#
+
+# We support a subset of the AutoGen definitions file syntax.  Specifically,
+# compound names are disallowed; C-style comments and preprocessing
+# directives are disallowed; and shell-generated strings, Scheme-generated
+# strings, and here strings are disallowed.
+
+class AutogenToken:
+    (autogen, definitions, eof, var_name, other_name, string, number,
+     semicolon, equals, comma, lbrace, rbrace, lbracket, rbracket) = range(14)
+
+class AutogenState:
+    (init, need_def, need_tpl, need_semi, need_name, have_name, need_value,
+     need_idx, need_rbracket, indx_name, have_value, done) = range(12)
+
+class AutogenParseError(Exception):
+    def __init__(self, message, line):
+        super(AutogenParseError, self).__init__(message)
+        self.line = line
+
+    def __str__(self):
+        return (
+            super(AutogenParseError, self).__str__() +
+            " at line %d" % self.line)
+
+class AutogenDefinition(list):
+    def __getitem__(self, key):
+        try:
+            return super(AutogenDefinition, self).__getitem__(key)
+        except TypeError:
+            for name, value in self:
+                if name == key:
+                    return value
+
+    def __contains__(self, key):
+        for name, value in self:
+            if name == key:
+                return True
+        return False
+
+    def get(self, key, default):
+        for name, value in self:
+            if name == key:
+                return value
+        else:
+            return default
+
+    def find_all(self, key):
+        for name, value in self:
+            if name == key:
+                yield value
+
+class AutogenParser:
+    def __init__(self):
+        self.definitions = AutogenDefinition()
+        self.def_stack = [("", self.definitions)]
+        self.curdef = None
+        self.new_name = None
+        self.cur_line = 0
+
+    @staticmethod
+    def is_unquotable_char(c):
+        return (ord(c) in range(ord("!"), ord("~") + 1) and
+                c not in "#,;<=>[\\]`{}?*'\"()")
+
+    @staticmethod
+    def is_value_name_char(c):
+        return c in ":^-_" or c.isalnum()
+
+    def read_tokens(self, f):
+        data = f.read()
+        end = len(data)
+        offset = 0
+        while offset < end:
+            while offset < end and data[offset].isspace():
+                if data[offset] == "\n":
+                    self.cur_line += 1
+                offset += 1
+            if offset >= end:
+                break
+            c = data[offset]
+            if c == "{":
+                yield AutogenToken.lbrace, c
+                offset += 1
+            elif c == "=":
+                yield AutogenToken.equals, c
+                offset += 1
+            elif c == "}":
+                yield AutogenToken.rbrace, c
+                offset += 1
+            elif c == "[":
+                yield AutogenToken.lbracket, c
+                offset += 1
+            elif c == "]":
+                yield AutogenToken.rbracket, c
+                offset += 1
+            elif c == ";":
+                yield AutogenToken.semicolon, c
+                offset += 1
+            elif c == ",":
+                yield AutogenToken.comma, c
+                offset += 1
+            elif c in ("'", '"'):
+                s = []
+                while True:
+                    offset += 1
+                    if offset >= end:
+                        raise AutogenParseError(
+                            "EOF in quoted string", self.cur_line)
+                    if data[offset] == "\n":
+                        self.cur_line += 1
+                    if data[offset] == "\\":
+                        offset += 1
+                        if offset >= end:
+                            raise AutogenParseError(
+                                "EOF in quoted string", self.cur_line)
+                        if data[offset] == "\n":
+                            self.cur_line += 1
+                        # Proper escaping unimplemented; this can be filled
+                        # out if needed.
+                        s.append("\\")
+                        s.append(data[offset])
+                    elif data[offset] == c:
+                        offset += 1
+                        break
+                    else:
+                        s.append(data[offset])
+                yield AutogenToken.string, "".join(s)
+            elif (c.isdigit() or
+                  (c == "-" and offset < end - 1 and
+                   data[offset + 1].isdigit())):
+                end_number = offset + 1
+                while end_number < end and data[end_number].isdigit():
+                    end_number += 1
+                yield AutogenToken.number, data[offset:end_number]
+                offset = end_number
+            elif self.is_unquotable_char(c):
+                end_name = offset
+                while (end_name < end and
+                       self.is_value_name_char(data[end_name])):
+                    end_name += 1
+                if end_name < end and self.is_unquotable_char(data[end_name]):
+                    while (end_name < end and
+                           self.is_unquotable_char(data[end_name])):
+                        end_name += 1
+                    yield AutogenToken.other_name, data[offset:end_name]
+                    offset = end_name
+                else:
+                    s = data[offset:end_name]
+                    if s.lower() == "autogen":
+                        yield AutogenToken.autogen, s
+                    elif s.lower() == "definitions":
+                        yield AutogenToken.definitions, s
+                    else:
+                        yield AutogenToken.var_name, s
+                    offset = end_name
+            else:
+                raise AutogenParseError(
+                    "Invalid input character '%s'" % c, self.cur_line)
+        yield AutogenToken.eof, None
+
+    def do_need_name_end(self, token):
+        if len(self.def_stack) > 1:
+            raise AutogenParseError(
+                "Definition blocks were left open", self.cur_line)
+
+    def do_need_name_var_name(self, token):
+        self.new_name = token
+
+    def do_end_block(self, token):
+        if len(self.def_stack) <= 1:
+            raise AutogenParseError("Too many close braces", self.cur_line)
+        new_name, parent_def = self.def_stack.pop()
+        parent_def.append((new_name, self.curdef))
+        self.curdef = parent_def
+
+    def do_empty_val(self, token):
+        self.curdef.append((self.new_name, ""))
+
+    def do_str_value(self, token):
+        self.curdef.append((self.new_name, token))
+
+    def do_start_block(self, token):
+        self.def_stack.append((self.new_name, self.curdef))
+        self.curdef = AutogenDefinition()
+
+    def do_indexed_name(self, token):
+        self.new_name = token
+
+    def read_definitions(self, f):
+        self.curdef = self.definitions
+        self.cur_line = 0
+        state = AutogenState.init
+
+        # The following transition table was reduced from the Autogen
+        # documentation:
+        #   info -f autogen -n 'Full Syntax'
+        transitions = {
+            AutogenState.init: {
+                AutogenToken.autogen: (AutogenState.need_def, None),
+            },
+            AutogenState.need_def: {
+                AutogenToken.definitions: (AutogenState.need_tpl, None),
+            },
+            AutogenState.need_tpl: {
+                AutogenToken.var_name: (AutogenState.need_semi, None),
+                AutogenToken.other_name: (AutogenState.need_semi, None),
+                AutogenToken.string: (AutogenState.need_semi, None),
+            },
+            AutogenState.need_semi: {
+                AutogenToken.semicolon: (AutogenState.need_name, None),
+            },
+            AutogenState.need_name: {
+                AutogenToken.autogen: (AutogenState.need_def, None),
+                AutogenToken.eof: (AutogenState.done, self.do_need_name_end),
+                AutogenToken.var_name: (
+                    AutogenState.have_name, self.do_need_name_var_name),
+                AutogenToken.rbrace: (
+                    AutogenState.have_value, self.do_end_block),
+            },
+            AutogenState.have_name: {
+                AutogenToken.semicolon: (
+                    AutogenState.need_name, self.do_empty_val),
+                AutogenToken.equals: (AutogenState.need_value, None),
+                AutogenToken.lbracket: (AutogenState.need_idx, None),
+            },
+            AutogenState.need_value: {
+                AutogenToken.var_name: (
+                    AutogenState.have_value, self.do_str_value),
+                AutogenToken.other_name: (
+                    AutogenState.have_value, self.do_str_value),
+                AutogenToken.string: (
+                    AutogenState.have_value, self.do_str_value),
+                AutogenToken.number: (
+                    AutogenState.have_value, self.do_str_value),
+                AutogenToken.lbrace: (
+                    AutogenState.need_name, self.do_start_block),
+            },
+            AutogenState.need_idx: {
+                AutogenToken.var_name: (
+                    AutogenState.need_rbracket, self.do_indexed_name),
+                AutogenToken.number: (
+                    AutogenState.need_rbracket, self.do_indexed_name),
+            },
+            AutogenState.need_rbracket: {
+                AutogenToken.rbracket: (AutogenState.indx_name, None),
+            },
+            AutogenState.indx_name: {
+                AutogenToken.semicolon: (
+                    AutogenState.need_name, self.do_empty_val),
+                AutogenToken.equals: (AutogenState.need_value, None),
+            },
+            AutogenState.have_value: {
+                AutogenToken.semicolon: (AutogenState.need_name, None),
+                AutogenToken.comma: (AutogenState.need_value, None),
+            },
+        }
+
+        for code, token in self.read_tokens(f):
+            if code in transitions[state]:
+                state, handler = transitions[state][code]
+                if handler is not None:
+                    handler(token)
+            else:
+                raise AutogenParseError(
+                    "Parse error in state %s: unexpected token '%s'" % (
+                        state, token), self.cur_line)
+            if state == AutogenState.done:
+                break
+
+defparser = AutogenParser()
+
+#
+# Output
+#
+
+outputs = {}
+
+def output(s, section=''):
+    if s == "":
+        return
+    outputs.setdefault(section, [])
+    outputs[section].append(s)
+
+def write_output(section=''):
+    for s in outputs.get(section, []):
+        print s,
+
+#
 # Global variables
 #
-GVARS = set()
 
 def gvar_add(var, value):
-    GVARS.add(var)
-    return var + " += " + value + "\n"
-
-def global_variable_initializers():
-    r = ""
-    for var in sorted(GVARS):
-        r += var + " ?= \n"
-    return r
+    output(var + " += " + value + "\n")
 
 #
 # Per PROGRAM/SCRIPT variables 
 #
 
-def vars_init(*var_list):
-    r = "[+ IF (if (not (assoc-ref seen-vars (get \".name\"))) \"seen\") +]"
-    r += "[+ (out-suspend \"v\") +]"
-    for var in var_list:
-        r += var + "  = \n"
-    r += "[+ (out-resume \"v\") +]"
-    r += "[+ (set! seen-vars (assoc-set! seen-vars (get \".name\") 0)) +]"
-    r += "[+ ENDIF +]"
-    return first_time(r)
+seen_vars = set()
+
+def vars_init(defn, *var_list):
+    name = defn['name']
+
+    if name not in seen_target and name not in seen_vars:
+        for var in var_list:
+            output(var + "  = \n", section='decl')
+        seen_vars.add(name)
 
 def var_set(var, value):
-    return var + "  = " + value + "\n"
+    output(var + "  = " + value + "\n")
 
 def var_add(var, value):
-    return var + " += " + value + "\n"
+    output(var + " += " + value + "\n")
 
 #
-# Autogen constructs
+# Variable names and rules
 #
 
-def set_canonical_name_suffix(suffix): return "[+ % name `export cname=$(echo %s" + suffix + " | sed -e 's/[^0-9A-Za-z@_]/_/g')` +]"
-def cname(): return "[+ % name `echo $cname` +]"
+canonical_name_re = re.compile(r'[^0-9A-Za-z@_]')
+canonical_name_suffix = ""
+
+def set_canonical_name_suffix(suffix):
+    global canonical_name_suffix
+    canonical_name_suffix = suffix
+
+def cname(defn):
+    return canonical_name_re.sub('_', defn['name'] + canonical_name_suffix)
 
 def rule(target, source, cmd):
     if cmd[0] == "\n":
-        return "\n" + target + ": " + source + cmd.replace("\n", "\n\t") + "\n"
+        output("\n" + target + ": " + source + cmd.replace("\n", "\n\t") + "\n")
     else:
-        return "\n" + target + ": " + source + "\n\t" + cmd.replace("\n", "\n\t") + "\n"
+        output("\n" + target + ": " + source + "\n\t" + cmd.replace("\n", "\n\t") + "\n")
 
 #
-# Template for keys with platform names as values, for example:
+# Handle keys with platform names as values, for example:
 #
 # kernel = {
 #   nostrip = emu;
 #   ...
 # }
 #
-def if_platform_tagged(platform, tag, snippet_if, snippet_else=None):
-    r = ""
-    r += "[+ IF " + tag + " defined +]"
-    r += "[+ FOR " + tag + " +][+ CASE " + tag + " +]"
-    for group in RMAP[platform]:
-        r += "[+ = \"" + group + "\" +]" + snippet_if
-
-    if snippet_else != None: r += "[+ * +]" + snippet_else
-    r += "[+ ESAC +][+ ENDFOR +]"
-
-    if snippet_else == None:
-        r += "[+ ENDIF +]"
-        return r
+def platform_tagged(defn, platform, tag):
+    for value in defn.find_all(tag):
+        for group in RMAP[platform]:
+            if value == group:
+                return True
+    return False
 
-    r += "[+ ELSE +]" + snippet_else + "[+ ENDIF +]"
-    return r
+def if_platform_tagged(defn, platform, tag, snippet_if, snippet_else=None):
+    if platform_tagged(defn, platform, tag):
+        return snippet_if
+    elif snippet_else is not None:
+        return snippet_else
 
 #
-# Template for tagged values
+# Handle tagged values
 #
 # module = {
 #   extra_dist = ...
@@ -178,11 +468,14 @@ def if_platform_tagged(platform, tag, snippet_if, snippet_else=None):
 #   ...
 # };
 #
-def foreach_value(tag, closure):
-    return "[+ FOR " + tag + " +]" + closure("[+ ." + tag + " +]") + "[+ ENDFOR +]"
+def foreach_value(defn, tag, closure):
+    r = []
+    for value in defn.find_all(tag):
+        r.append(closure(value))
+    return ''.join(r)
 
 #
-# Template for handling best matched values for a platform, for example:
+# Handle best matched values for a platform, for example:
 #
 # module = {
 #   cflags = '-Wall';
@@ -190,34 +483,21 @@ def foreach_value(tag, closure):
 #   ...
 # }
 #
-def foreach_platform_specific_value(platform, suffix, nonetag, closure):
-    r = ""
+def foreach_platform_specific_value(defn, platform, suffix, nonetag, closure):
+    r = []
     for group in RMAP[platform]:
-        gtag = group + suffix
-
-        if group == RMAP[platform][0]:
-            r += "[+ IF " + gtag + " +]"
-        else:
-            r += "[+ ELIF " + gtag + " +]"
-
-        r += "[+ FOR " + gtag + " +]" + closure("[+ ." + gtag + " +]") + "[+ ENDFOR +]"
-    r += "[+ ELSE +][+ FOR " + nonetag + " +]" + closure("[+ ." + nonetag + " +]") + "[+ ENDFOR +][+ ENDIF +]"
-    return r
-
-#
-# Returns autogen code that defines an autogen macro using the
-# definition given in the 'snippet'.
-#
-def define_autogen_macro(name, snippet):
-    r = ""
-    r += "[+ DEFINE " + name + " +]"
-    r += snippet
-    r += "[+ ENDDEF +]\n"
-    return r
+        values = list(defn.find_all(group + suffix))
+        if values:
+            for value in values:
+                r.append(closure(value))
+            break
+    else:
+        for value in defn.find_all(nonetag):
+            r.append(closure(value))
+    return ''.join(r)
 
 #
-# Template for handling values from sum of all groups for a platform,
-# for example:
+# Handle values from sum of all groups for a platform, for example:
 #
 # module = {
 #   common = kern/misc.c;
@@ -225,18 +505,20 @@ def define_autogen_macro(name, snippet):
 #   ...
 # }
 #
-def foreach_platform_value (platform, suffix, closure):
-    r = ""
+def foreach_platform_value(defn, platform, suffix, closure):
+    r = []
     for group in RMAP[platform]:
-        gtag = group + suffix
+        for value in defn.find_all(group + suffix):
+            r.append(closure(value))
+    return ''.join(r)
 
-        r += "[+ IF " + gtag + " +]"
-        r += "[+ FOR " + gtag + " +]" + closure("[+ ." + gtag + " +]") + "[+ ENDFOR +]"
-        r += "[+ ENDIF +]"
-    return r
+def platform_conditional(platform, closure):
+    output("\nif COND_" + platform + "\n")
+    closure(platform)
+    output("endif\n")
 
 #
-# Template for gaurding with platform specific "enable" keys, for example:
+# Handle guarding with platform-specific "enable" keys, for example:
 #
 #  module = {
 #    name = pci;
@@ -251,20 +533,17 @@ def foreach_platform_value (platform, suffix, closure):
 #    enable = i386_coreboot;
 #  };
 #
-def foreach_enabled_platform(closure):
-    r = "[+ IF - enable undefined +]"
-    for platform in GRUB_PLATFORMS:
-        r += "\nif COND_" + platform + "\n" + closure(platform) + "endif\n"
-    r += "[+ ELSE +]"
-    for platform in GRUB_PLATFORMS:
-        x = "\nif COND_" + platform + "\n" + closure(platform) + "endif\n"
-        r += if_platform_tagged(platform, "enable", x)
-    r += "[+ ENDIF +]"
-    return r
+def foreach_enabled_platform(defn, closure):
+    if 'enable' in defn:
+        for platform in GRUB_PLATFORMS:
+            if platform_tagged(defn, platform, "enable"):
+               platform_conditional(platform, closure)
+    else:
+        for platform in GRUB_PLATFORMS:
+            platform_conditional(platform, closure)
 
 #
-# Template for gaurding with platform specific automake conditionals,
-# for example:
+# Handle guarding with platform-specific automake conditionals, for example:
 #
 #  module = {
 #    name = usb;
@@ -277,349 +556,251 @@ def foreach_enabled_platform(closure):
 #    emu_condition = COND_GRUB_EMU_USB;
 #  };
 #
-def define_macro_for_platform_conditionals_if_statement(p):
-    return define_autogen_macro(
-        "if_" + p + "_conditionals",
-        foreach_platform_specific_value(platform, "_condition", "condition", lambda cond: "if " + cond + "\n"))
-def define_macro_for_platform_conditionals_endif_statement(p):
-    return define_autogen_macro(
-        "endif_" + p + "_conditionals",
-        foreach_platform_specific_value(platform, "_condition", "condition", lambda cond: "endif " + cond + "\n"))
-def under_platform_specific_conditionals(platform, snippet):
-    r  = "[+ if_" + platform + "_conditionals +]"
-    r += snippet
-    r += "[+ endif_" + platform + "_conditionals +]"
-    return r
-
-def platform_specific_values(platform, suffix, nonetag):
-    return foreach_platform_specific_value(platform, suffix, nonetag,
+def under_platform_specific_conditionals(defn, platform, closure):
+    output(foreach_platform_specific_value(defn, platform, "_condition", "condition", lambda cond: "if " + cond + "\n"))
+    closure(defn, platform)
+    output(foreach_platform_specific_value(defn, platform, "_condition", "condition", lambda cond: "endif " + cond + "\n"))
+
+def platform_specific_values(defn, platform, suffix, nonetag):
+    return foreach_platform_specific_value(defn, platform, suffix, nonetag,
                                            lambda value: value + " ")
 
-def platform_values(platform, suffix):
-    return foreach_platform_value(platform, suffix, lambda value: value + " ")
-
-def extra_dist():
-    return foreach_value("extra_dist", lambda value: value + " ")
-
-def define_macro_for_platform_sources(p):
-    return define_autogen_macro(
-        "get_" + p + "_sources",
-        platform_values(p, ""))
-def define_macro_for_platform_nodist_sources(p):
-    return define_autogen_macro(
-        "get_" + p + "_nodist_sources",
-        platform_values(p, "_nodist"))
-def platform_sources(p): return "[+ get_" + p + "_sources +]"
-def platform_nodist_sources(p): return "[+ get_" + p + "_nodist_sources +]"
-
-#
-# Returns Autogen code which defines the autogen macros that collect
-# platform specific values for cflags, ldflags, etc. tags.
-#
-def define_macro_for_platform_startup(p):
-    return define_autogen_macro(
-        "get_" + p + "_startup",
-        platform_specific_values(p, "_startup", "startup"))
-def define_macro_for_platform_cflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_cflags",
-        platform_specific_values(p, "_cflags", "cflags"))
-def define_macro_for_platform_ldadd(p):
-    return define_autogen_macro(
-        "get_" + p + "_ldadd",
-        platform_specific_values(p, "_ldadd", "ldadd"))
-def define_macro_for_platform_dependencies(p):
-    return define_autogen_macro(
-        "get_" + p + "_dependencies",
-        platform_specific_values(p, "_dependencies", "dependencies"))
-def define_macro_for_platform_ldflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_ldflags",
-        platform_specific_values(p, "_ldflags", "ldflags"))
-def define_macro_for_platform_cppflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_cppflags",
-        platform_specific_values(p, "_cppflags", "cppflags"))
-def define_macro_for_platform_ccasflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_ccasflags",
-        platform_specific_values(p, "_ccasflags", "ccasflags"))
-def define_macro_for_platform_stripflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_stripflags",
-        platform_specific_values(p, "_stripflags", "stripflags"))
-def define_macro_for_platform_objcopyflags(p):
-    return define_autogen_macro(
-        "get_" + p + "_objcopyflags",
-        platform_specific_values(p, "_objcopyflags", "objcopyflags"))
-#
-# Autogen calls to invoke the above macros.
-#
-def platform_startup(p): return "[+ get_" + p + "_startup +]"
-def platform_ldadd(p): return "[+ get_" + p + "_ldadd +]"
-def platform_dependencies(p): return "[+ get_" + p + "_dependencies +]"
-def platform_cflags(p): return "[+ get_" + p + "_cflags +]"
-def platform_ldflags(p): return "[+ get_" + p + "_ldflags +]"
-def platform_cppflags(p): return "[+ get_" + p + "_cppflags +]"
-def platform_ccasflags(p): return "[+ get_" + p + "_ccasflags +]"
-def platform_stripflags(p): return "[+ get_" + p + "_stripflags +]"
-def platform_objcopyflags(p): return "[+ get_" + p + "_objcopyflags +]"
+def platform_values(defn, platform, suffix):
+    return foreach_platform_value(defn, platform, suffix, lambda value: value + " ")
+
+def extra_dist(defn):
+    return foreach_value(defn, "extra_dist", lambda value: value + " ")
+
+def platform_sources(defn, p): return platform_values(defn, p, "")
+def platform_nodist_sources(defn, p): return platform_values(defn, p, "_nodist")
+
+def platform_startup(defn, p): return platform_specific_values(defn, p, "_startup", "startup")
+def platform_ldadd(defn, p): return platform_specific_values(defn, p, "_ldadd", "ldadd")
+def platform_dependencies(defn, p): return platform_specific_values(defn, p, "_dependencies", "dependencies")
+def platform_cflags(defn, p): return platform_specific_values(defn, p, "_cflags", "cflags")
+def platform_ldflags(defn, p): return platform_specific_values(defn, p, "_ldflags", "ldflags")
+def platform_cppflags(defn, p): return platform_specific_values(defn, p, "_cppflags", "cppflags")
+def platform_ccasflags(defn, p): return platform_specific_values(defn, p, "_ccasflags", "ccasflags")
+def platform_stripflags(defn, p): return platform_specific_values(defn, p, "_stripflags", "stripflags")
+def platform_objcopyflags(defn, p): return platform_specific_values(defn, p, "_objcopyflags", "objcopyflags")
 
 #
 # Emit snippet only the first time through for the current name.
 #
-def first_time(snippet):
-    r = "[+ IF (if (not (assoc-ref seen-target (get \".name\"))) \"seen\") +]"
-    r += snippet
-    r += "[+ ENDIF +]"
-    return r
-
-def module(platform):
-    r = set_canonical_name_suffix(".module")
-
-    r += gvar_add("platform_PROGRAMS", "[+ name +].module")
-    r += gvar_add("MODULE_FILES", "[+ name +].module$(EXEEXT)")
-
-    r += var_set(cname() + "_SOURCES", platform_sources(platform) + " ## platform sources")
-    r += var_set("nodist_" + cname() + "_SOURCES", platform_nodist_sources(platform) + " ## platform nodist sources")
-    r += var_set(cname() + "_LDADD", platform_ldadd(platform))
-    r += var_set(cname() + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_MODULE) " + platform_cflags(platform))
-    r += var_set(cname() + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_MODULE) " + platform_ldflags(platform))
-    r += var_set(cname() + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_MODULE) " + platform_cppflags(platform))
-    r += var_set(cname() + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_MODULE) " + platform_ccasflags(platform))
-    r += var_set(cname() + "_DEPENDENCIES", "$(TARGET_OBJ2ELF) " + platform_dependencies(platform))
-
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    r += gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)")
-    r += gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)")
-
-    r += gvar_add("MOD_FILES", "[+ name +].mod")
-    r += gvar_add("MARKER_FILES", "[+ name +].marker")
-    r += gvar_add("CLEANFILES", "[+ name +].marker")
-    r += """
-[+ name +].marker: $(""" + cname() + """_SOURCES) $(nodist_""" + cname() + """_SOURCES)
-	$(TARGET_CPP) -DGRUB_LST_GENERATOR $(CPPFLAGS_MARKER) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(""" + cname() + """_CPPFLAGS) $(CPPFLAGS) $^ > $@.new || (rm -f $@; exit 1)
+seen_target = set()
+
+def first_time(defn, snippet):
+    if defn['name'] not in seen_target:
+        return snippet
+    return ''
+
+def module(defn, platform):
+    name = defn['name']
+    set_canonical_name_suffix(".module")
+
+    gvar_add("platform_PROGRAMS", name + ".module")
+    gvar_add("MODULE_FILES", name + ".module$(EXEEXT)")
+
+    var_set(cname(defn) + "_SOURCES", platform_sources(defn, platform) + " ## platform sources")
+    var_set("nodist_" + cname(defn) + "_SOURCES", platform_nodist_sources(defn, platform) + " ## platform nodist sources")
+    var_set(cname(defn) + "_LDADD", platform_ldadd(defn, platform))
+    var_set(cname(defn) + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_MODULE) " + platform_cflags(defn, platform))
+    var_set(cname(defn) + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_MODULE) " + platform_ldflags(defn, platform))
+    var_set(cname(defn) + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_MODULE) " + platform_cppflags(defn, platform))
+    var_set(cname(defn) + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_MODULE) " + platform_ccasflags(defn, platform))
+    var_set(cname(defn) + "_DEPENDENCIES", "$(TARGET_OBJ2ELF) " + platform_dependencies(defn, platform))
+
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+    gvar_add("BUILT_SOURCES", "$(nodist_" + cname(defn) + "_SOURCES)")
+    gvar_add("CLEANFILES", "$(nodist_" + cname(defn) + "_SOURCES)")
+
+    gvar_add("MOD_FILES", name + ".mod")
+    gvar_add("MARKER_FILES", name + ".marker")
+    gvar_add("CLEANFILES", name + ".marker")
+    output("""
+""" + name + """.marker: $(""" + cname(defn) + """_SOURCES) $(nodist_""" + cname(defn) + """_SOURCES)
+	$(TARGET_CPP) -DGRUB_LST_GENERATOR $(CPPFLAGS_MARKER) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(""" + cname(defn) + """_CPPFLAGS) $(CPPFLAGS) $^ > $@.new || (rm -f $@; exit 1)
 	grep 'MARKER' $@.new > $@; rm -f $@.new
-"""
-    return r
-
-def kernel(platform):
-    r = set_canonical_name_suffix(".exec")
-    r += gvar_add("platform_PROGRAMS", "[+ name +].exec")
-    r += var_set(cname() + "_SOURCES", platform_startup(platform))
-    r += var_add(cname() + "_SOURCES", platform_sources(platform))
-    r += var_set("nodist_" + cname() + "_SOURCES", platform_nodist_sources(platform) + " ## platform nodist sources")
-    r += var_set(cname() + "_LDADD", platform_ldadd(platform))
-    r += var_set(cname() + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_KERNEL) " + platform_cflags(platform))
-    r += var_set(cname() + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_KERNEL) " + platform_ldflags(platform))
-    r += var_set(cname() + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_KERNEL) " + platform_cppflags(platform))
-    r += var_set(cname() + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_KERNEL) " + platform_ccasflags(platform))
-    r += var_set(cname() + "_STRIPFLAGS", "$(AM_STRIPFLAGS) $(STRIPFLAGS_KERNEL) " + platform_stripflags(platform))
-    r += var_set(cname() + "_DEPENDENCIES", "$(TARGET_OBJ2ELF)")
-
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    r += gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)")
-    r += gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)")
-
-    r += gvar_add("platform_DATA", "[+ name +].img")
-    r += gvar_add("CLEANFILES", "[+ name +].img")
-    r += rule("[+ name +].img", "[+ name +].exec$(EXEEXT)",
-              if_platform_tagged(platform, "nostrip",
+""")
+
+def kernel(defn, platform):
+    name = defn['name']
+    set_canonical_name_suffix(".exec")
+    gvar_add("platform_PROGRAMS", name + ".exec")
+    var_set(cname(defn) + "_SOURCES", platform_startup(defn, platform))
+    var_add(cname(defn) + "_SOURCES", platform_sources(defn, platform))
+    var_set("nodist_" + cname(defn) + "_SOURCES", platform_nodist_sources(defn, platform) + " ## platform nodist sources")
+    var_set(cname(defn) + "_LDADD", platform_ldadd(defn, platform))
+    var_set(cname(defn) + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_KERNEL) " + platform_cflags(defn, platform))
+    var_set(cname(defn) + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_KERNEL) " + platform_ldflags(defn, platform))
+    var_set(cname(defn) + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_KERNEL) " + platform_cppflags(defn, platform))
+    var_set(cname(defn) + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_KERNEL) " + platform_ccasflags(defn, platform))
+    var_set(cname(defn) + "_STRIPFLAGS", "$(AM_STRIPFLAGS) $(STRIPFLAGS_KERNEL) " + platform_stripflags(defn, platform))
+    var_set(cname(defn) + "_DEPENDENCIES", "$(TARGET_OBJ2ELF)")
+
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+    gvar_add("BUILT_SOURCES", "$(nodist_" + cname(defn) + "_SOURCES)")
+    gvar_add("CLEANFILES", "$(nodist_" + cname(defn) + "_SOURCES)")
+
+    gvar_add("platform_DATA", name + ".img")
+    gvar_add("CLEANFILES", name + ".img")
+    rule(name + ".img", name + ".exec$(EXEEXT)",
+         if_platform_tagged(defn, platform, "nostrip",
 """if test x$(TARGET_APPLE_LINKER) = x1; then \
      $(TARGET_OBJCONV) -f$(TARGET_MODULE_FORMAT) -nr:_grub_mod_init:grub_mod_init -nr:_grub_mod_fini:grub_mod_fini -ed2022 -wd1106 -nu -nd $< $@; \
    elif test ! -z '$(TARGET_OBJ2ELF)'; then \
      cp $< $@.bin; $(TARGET_OBJ2ELF) $@.bin && cp $@.bin $@ || (rm -f $@.bin; exit 1); \
    else cp $< $@; fi""",
 """if test x$(TARGET_APPLE_LINKER) = x1; then \
-  $(TARGET_STRIP) -S -x $(""" + cname() + """) -o $@.bin $<; \
+  $(TARGET_STRIP) -S -x $(""" + cname(defn) + """) -o $@.bin $<; \
   $(TARGET_OBJCONV) -f$(TARGET_MODULE_FORMAT) -nr:_grub_mod_init:grub_mod_init -nr:_grub_mod_fini:grub_mod_fini -ed2022 -ed2016 -wd1106 -nu -nd $@.bin $@; \
-else """  + "$(TARGET_STRIP) $(" + cname() + "_STRIPFLAGS) -o $@ $<; \
+else """  + "$(TARGET_STRIP) $(" + cname(defn) + "_STRIPFLAGS) -o $@ $<; \
 fi"""))
-    return r
-
-def image(platform):
-    r = set_canonical_name_suffix(".image")
-    r += gvar_add("platform_PROGRAMS", "[+ name +].image")
-    r += var_set(cname() + "_SOURCES", platform_sources(platform))
-    r += var_set("nodist_" + cname() + "_SOURCES", platform_nodist_sources(platform) + "## platform nodist sources")
-    r += var_set(cname() + "_LDADD", platform_ldadd(platform))
-    r += var_set(cname() + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_IMAGE) " + platform_cflags(platform))
-    r += var_set(cname() + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_IMAGE) " + platform_ldflags(platform))
-    r += var_set(cname() + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_IMAGE) " + platform_cppflags(platform))
-    r += var_set(cname() + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_IMAGE) " + platform_ccasflags(platform))
-    r += var_set(cname() + "_OBJCOPYFLAGS", "$(OBJCOPYFLAGS_IMAGE) " + platform_objcopyflags(platform))
-    # r += var_set(cname() + "_DEPENDENCIES", platform_dependencies(platform) + " " + platform_ldadd(platform))
-
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    r += gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)")
-    r += gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)")
-
-    r += gvar_add("platform_DATA", "[+ name +].img")
-    r += gvar_add("CLEANFILES", "[+ name +].img")
-    r += rule("[+ name +].img", "[+ name +].image$(EXEEXT)", """
+
+def image(defn, platform):
+    name = defn['name']
+    set_canonical_name_suffix(".image")
+    gvar_add("platform_PROGRAMS", name + ".image")
+    var_set(cname(defn) + "_SOURCES", platform_sources(defn, platform))
+    var_set("nodist_" + cname(defn) + "_SOURCES", platform_nodist_sources(defn, platform) + "## platform nodist sources")
+    var_set(cname(defn) + "_LDADD", platform_ldadd(defn, platform))
+    var_set(cname(defn) + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_IMAGE) " + platform_cflags(defn, platform))
+    var_set(cname(defn) + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_IMAGE) " + platform_ldflags(defn, platform))
+    var_set(cname(defn) + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_IMAGE) " + platform_cppflags(defn, platform))
+    var_set(cname(defn) + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_IMAGE) " + platform_ccasflags(defn, platform))
+    var_set(cname(defn) + "_OBJCOPYFLAGS", "$(OBJCOPYFLAGS_IMAGE) " + platform_objcopyflags(defn, platform))
+    # var_set(cname(defn) + "_DEPENDENCIES", platform_dependencies(defn, platform) + " " + platform_ldadd(defn, platform))
+
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+    gvar_add("BUILT_SOURCES", "$(nodist_" + cname(defn) + "_SOURCES)")
+    gvar_add("CLEANFILES", "$(nodist_" + cname(defn) + "_SOURCES)")
+
+    gvar_add("platform_DATA", name + ".img")
+    gvar_add("CLEANFILES", name + ".img")
+    rule(name + ".img", name + ".image$(EXEEXT)", """
 if test x$(TARGET_APPLE_LINKER) = x1; then \
   $(MACHO2IMG) $< $@; \
 else \
-  $(TARGET_OBJCOPY) $(""" + cname() + """_OBJCOPYFLAGS) --strip-unneeded -R .note -R .comment -R .note.gnu.build-id -R .reginfo -R .rel.dyn -R .note.gnu.gold-version $< $@; \
+  $(TARGET_OBJCOPY) $(""" + cname(defn) + """_OBJCOPYFLAGS) --strip-unneeded -R .note -R .comment -R .note.gnu.build-id -R .reginfo -R .rel.dyn -R .note.gnu.gold-version $< $@; \
 fi
 """)
-    return r
-
-def library(platform):
-    r = set_canonical_name_suffix("")
-
-    r += vars_init(cname() + "_SOURCES",
-                   "nodist_" + cname() + "_SOURCES",
-                   cname() + "_CFLAGS",
-                   cname() + "_CPPFLAGS",
-                   cname() + "_CCASFLAGS")
-    #              cname() + "_DEPENDENCIES")
-
-    r += first_time(gvar_add("noinst_LIBRARIES", "[+ name +]"))
-    r += var_add(cname() + "_SOURCES", platform_sources(platform))
-    r += var_add("nodist_" + cname() + "_SOURCES", platform_nodist_sources(platform))
-    r += var_add(cname() + "_CFLAGS", first_time("$(AM_CFLAGS) $(CFLAGS_LIBRARY) ") + platform_cflags(platform))
-    r += var_add(cname() + "_CPPFLAGS", first_time("$(AM_CPPFLAGS) $(CPPFLAGS_LIBRARY) ") + platform_cppflags(platform))
-    r += var_add(cname() + "_CCASFLAGS", first_time("$(AM_CCASFLAGS) $(CCASFLAGS_LIBRARY) ") + platform_ccasflags(platform))
-    # r += var_add(cname() + "_DEPENDENCIES", platform_dependencies(platform) + " " + platform_ldadd(platform))
-
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    r += first_time(gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)"))
-    r += first_time(gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)"))
-    return r
-
-def installdir(default="bin"):
-    return "[+ IF installdir +][+ installdir +][+ ELSE +]" + default + "[+ ENDIF +]"
-
-def manpage(adddeps):
-    r  = "if COND_MAN_PAGES\n"
-    r += gvar_add("man_MANS", "[+ name +].[+ mansection +]\n")
-    r += rule("[+ name +].[+ mansection +]", "[+ name +] " + adddeps, """
-chmod a+x [+ name +]
-PATH=$(builddir):$$PATH pkgdatadir=$(builddir) $(HELP2MAN) --section=[+ mansection +] -i $(top_srcdir)/docs/man/[+ name +].h2m -o $@ [+ name +]
+
+def library(defn, platform):
+    name = defn['name']
+    set_canonical_name_suffix("")
+
+    vars_init(defn,
+              cname(defn) + "_SOURCES",
+              "nodist_" + cname(defn) + "_SOURCES",
+              cname(defn) + "_CFLAGS",
+              cname(defn) + "_CPPFLAGS",
+              cname(defn) + "_CCASFLAGS")
+    #         cname(defn) + "_DEPENDENCIES")
+
+    if name not in seen_target:
+        gvar_add("noinst_LIBRARIES", name)
+    var_add(cname(defn) + "_SOURCES", platform_sources(defn, platform))
+    var_add("nodist_" + cname(defn) + "_SOURCES", platform_nodist_sources(defn, platform))
+    var_add(cname(defn) + "_CFLAGS", first_time(defn, "$(AM_CFLAGS) $(CFLAGS_LIBRARY) ") + platform_cflags(defn, platform))
+    var_add(cname(defn) + "_CPPFLAGS", first_time(defn, "$(AM_CPPFLAGS) $(CPPFLAGS_LIBRARY) ") + platform_cppflags(defn, platform))
+    var_add(cname(defn) + "_CCASFLAGS", first_time(defn, "$(AM_CCASFLAGS) $(CCASFLAGS_LIBRARY) ") + platform_ccasflags(defn, platform))
+    # var_add(cname(defn) + "_DEPENDENCIES", platform_dependencies(defn, platform) + " " + platform_ldadd(defn, platform))
+
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+    if name not in seen_target:
+        gvar_add("BUILT_SOURCES", "$(nodist_" + cname(defn) + "_SOURCES)")
+        gvar_add("CLEANFILES", "$(nodist_" + cname(defn) + "_SOURCES)")
+
+def installdir(defn, default="bin"):
+    return defn.get('installdir', default)
+
+def manpage(defn, adddeps):
+    name = defn['name']
+    mansection = defn['mansection']
+
+    output("if COND_MAN_PAGES\n")
+    gvar_add("man_MANS", name + "." + mansection)
+    rule(name + "." + mansection, name + " " + adddeps, """
+chmod a+x """ + name + """
+PATH=$(builddir):$$PATH pkgdatadir=$(builddir) $(HELP2MAN) --section=""" + mansection + """ -i $(top_srcdir)/docs/man/""" + name + """.h2m -o $@ """ + name + """
 """)
-    r += gvar_add("CLEANFILES", "[+ name +].[+ mansection +]")
-    r += "endif\n"
-    return r
-
-def program(platform, test=False):
-    r = set_canonical_name_suffix("")
-
-    r += "[+ IF testcase defined +]"
-    r += gvar_add("check_PROGRAMS", "[+ name +]")
-    r += gvar_add("TESTS", "[+ name +]")
-    r += "[+ ELSE +]"
-    r += var_add(installdir() + "_PROGRAMS", "[+ name +]")
-    r += "[+ IF mansection +]" + manpage("") + "[+ ENDIF +]"
-    r += "[+ ENDIF +]"
-
-    r += var_set(cname() + "_SOURCES", platform_sources(platform))
-    r += var_set("nodist_" + cname() + "_SOURCES", platform_nodist_sources(platform))
-    r += var_set(cname() + "_LDADD", platform_ldadd(platform))
-    r += var_set(cname() + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_PROGRAM) " + platform_cflags(platform))
-    r += var_set(cname() + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_PROGRAM) " + platform_ldflags(platform))
-    r += var_set(cname() + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_PROGRAM) " + platform_cppflags(platform))
-    r += var_set(cname() + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_PROGRAM) " + platform_ccasflags(platform))
-    # r += var_set(cname() + "_DEPENDENCIES", platform_dependencies(platform) + " " + platform_ldadd(platform))
-
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    r += gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)")
-    r += gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)")
-    return r
-
-def data(platform):
-    r  = var_add("dist_" + installdir() + "_DATA", platform_sources(platform))
-    r += gvar_add("dist_noinst_DATA", extra_dist())
-    return r
-
-def script(platform):
-    r  = "[+ IF testcase defined +]"
-    r += gvar_add("check_SCRIPTS", "[+ name +]")
-    r += gvar_add ("TESTS", "[+ name +]")
-    r += "[+ ELSE +]"
-    r += var_add(installdir() + "_SCRIPTS", "[+ name +]")
-    r += "[+ IF mansection +]" + manpage("grub-mkconfig_lib") + "[+ ENDIF +]"
-    r += "[+ ENDIF +]"
-
-    r += rule("[+ name +]", "$(top_builddir)/config.status " + platform_sources(platform) + platform_dependencies(platform), """
-(for x in """ + platform_sources(platform) + """; do cat $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-
-chmod a+x [+ name +]
+    gvar_add("CLEANFILES", name + "." + mansection)
+    output("endif\n")
+
+def program(defn, platform, test=False):
+    name = defn['name']
+    set_canonical_name_suffix("")
+
+    if 'testcase' in defn:
+        gvar_add("check_PROGRAMS", name)
+        gvar_add("TESTS", name)
+    else:
+        var_add(installdir(defn) + "_PROGRAMS", name)
+        if 'mansection' in defn:
+            manpage(defn, "")
+
+    var_set(cname(defn) + "_SOURCES", platform_sources(defn, platform))
+    var_set("nodist_" + cname(defn) + "_SOURCES", platform_nodist_sources(defn, platform))
+    var_set(cname(defn) + "_LDADD", platform_ldadd(defn, platform))
+    var_set(cname(defn) + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_PROGRAM) " + platform_cflags(defn, platform))
+    var_set(cname(defn) + "_LDFLAGS", "$(AM_LDFLAGS) $(LDFLAGS_PROGRAM) " + platform_ldflags(defn, platform))
+    var_set(cname(defn) + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_PROGRAM) " + platform_cppflags(defn, platform))
+    var_set(cname(defn) + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_PROGRAM) " + platform_ccasflags(defn, platform))
+    # var_set(cname(defn) + "_DEPENDENCIES", platform_dependencies(defn, platform) + " " + platform_ldadd(defn, platform))
+
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+    gvar_add("BUILT_SOURCES", "$(nodist_" + cname(defn) + "_SOURCES)")
+    gvar_add("CLEANFILES", "$(nodist_" + cname(defn) + "_SOURCES)")
+
+def data(defn, platform):
+    var_add("dist_" + installdir(defn) + "_DATA", platform_sources(defn, platform))
+    gvar_add("dist_noinst_DATA", extra_dist(defn))
+
+def script(defn, platform):
+    name = defn['name']
+
+    if 'testcase' in defn:
+        gvar_add("check_SCRIPTS", name)
+        gvar_add ("TESTS", name)
+    else:
+        var_add(installdir(defn) + "_SCRIPTS", name)
+        if 'mansection' in defn:
+            manpage(defn, "grub-mkconfig_lib")
+
+    rule(name, "$(top_builddir)/config.status " + platform_sources(defn, platform) + platform_dependencies(defn, platform), """
+(for x in """ + platform_sources(defn, platform) + """; do cat $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-
+chmod a+x """ + name + """
 """)
 
-    r += gvar_add("CLEANFILES", "[+ name +]")
-    r += gvar_add("EXTRA_DIST", extra_dist())
-    r += gvar_add("dist_noinst_DATA", platform_sources(platform))
-    return r
+    gvar_add("CLEANFILES", name)
+    gvar_add("EXTRA_DIST", extra_dist(defn))
+    gvar_add("dist_noinst_DATA", platform_sources(defn, platform))
 
 def rules(target, closure):
-    # Create association lists for the benefit of first_time and vars_init.
-    r = "[+ (define seen-target '()) +]"
-    r += "[+ (define seen-vars '()) +]"
-    # Most output goes to a diversion.  This allows us to emit variable
-    # initializations before everything else.
-    r += "[+ (out-push-new) +]"
-
-    r += "[+ FOR " + target + " +]"
-    r += foreach_enabled_platform(
-        lambda p: under_platform_specific_conditionals(p, closure(p)))
-    # Remember that we've seen this target.
-    r += "[+ (set! seen-target (assoc-set! seen-target (get \".name\") 0)) +]"
-    r += "[+ ENDFOR +]"
-    r += "[+ (out-pop #t) +]"
-    return r
-
-def module_rules():
-    return rules("module", module)
-
-def kernel_rules():
-    return rules("kernel", kernel)
-
-def image_rules():
-    return rules("image", image)
-
-def library_rules():
-    return rules("library", library)
-
-def program_rules():
-    return rules("program", program)
-
-def script_rules():
-    return rules("script", script)
-
-def data_rules():
-    return rules("data", data)
-
-a = module_rules()
-b = kernel_rules()
-c = image_rules()
-d = library_rules()
-e = program_rules()
-f = script_rules()
-g = data_rules()
-z = global_variable_initializers()
-
-print ("[+ AutoGen5 template +]\n")
-for p in GRUB_PLATFORMS:
-    print (define_macro_for_platform_sources(p))
-    print (define_macro_for_platform_nodist_sources(p))
-
-    print (define_macro_for_platform_startup(p))
-    print (define_macro_for_platform_cflags(p))
-    print (define_macro_for_platform_ldadd(p))
-    print (define_macro_for_platform_dependencies(p))
-    print (define_macro_for_platform_ldflags(p))
-    print (define_macro_for_platform_cppflags(p))
-    print (define_macro_for_platform_ccasflags(p))
-    print (define_macro_for_platform_stripflags(p))
-    print (define_macro_for_platform_objcopyflags(p))
-
-    print (define_macro_for_platform_conditionals_if_statement(p))
-    print (define_macro_for_platform_conditionals_endif_statement(p))
-# print z # initializer for all vars
-print (a)
-print (b)
-print (c)
-print (d)
-print (e)
-print (f)
-print (g)
+    seen_target.clear()
+    seen_vars.clear()
+
+    for defn in defparser.definitions.find_all(target):
+        foreach_enabled_platform(
+            defn,
+            lambda p: under_platform_specific_conditionals(defn, p, closure))
+        # Remember that we've seen this target.
+        seen_target.add(defn['name'])
+
+parser = OptionParser(usage="%prog DEFINITION-FILES")
+_, args = parser.parse_args()
+
+for arg in args:
+    with open(arg) as f:
+        defparser.read_definitions(f)
+
+rules("module", module)
+rules("kernel", kernel)
+rules("image", image)
+rules("library", library)
+rules("program", program)
+rules("script", script)
+rules("data", data)
+
+write_output(section='decl')
+write_output()
-- 
1.8.4.3

[PATCH 2/3] Show file name in error messages from AutogenParser.

[Colin Watson]

---
 gentpl.py | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/gentpl.py b/gentpl.py
index f0f4493..5bee43a 100644
--- a/gentpl.py
+++ b/gentpl.py
@@ -119,14 +119,15 @@ class AutogenState:
      need_idx, need_rbracket, indx_name, have_value, done) = range(12)
 
 class AutogenParseError(Exception):
-    def __init__(self, message, line):
+    def __init__(self, message, path, line):
         super(AutogenParseError, self).__init__(message)
+        self.path = path
         self.line = line
 
     def __str__(self):
         return (
             super(AutogenParseError, self).__str__() +
-            " at line %d" % self.line)
+            " at file %s line %d" % (self.path, self.line))
 
 class AutogenDefinition(list):
     def __getitem__(self, key):
@@ -161,6 +162,7 @@ class AutogenParser:
         self.def_stack = [("", self.definitions)]
         self.curdef = None
         self.new_name = None
+        self.cur_path = None
         self.cur_line = 0
 
     @staticmethod
@@ -172,6 +174,9 @@ class AutogenParser:
     def is_value_name_char(c):
         return c in ":^-_" or c.isalnum()
 
+    def error(self, message):
+        raise AutogenParseError(message, self.cur_file, self.cur_line)
+
     def read_tokens(self, f):
         data = f.read()
         end = len(data)
@@ -210,15 +215,13 @@ class AutogenParser:
                 while True:
                     offset += 1
                     if offset >= end:
-                        raise AutogenParseError(
-                            "EOF in quoted string", self.cur_line)
+                        self.error("EOF in quoted string")
                     if data[offset] == "\n":
                         self.cur_line += 1
                     if data[offset] == "\\":
                         offset += 1
                         if offset >= end:
-                            raise AutogenParseError(
-                                "EOF in quoted string", self.cur_line)
+                            self.error("EOF in quoted string")
                         if data[offset] == "\n":
                             self.cur_line += 1
                         # Proper escaping unimplemented; this can be filled
@@ -260,21 +263,19 @@ class AutogenParser:
                         yield AutogenToken.var_name, s
                     offset = end_name
             else:
-                raise AutogenParseError(
-                    "Invalid input character '%s'" % c, self.cur_line)
+                self.error("Invalid input character '%s'" % c)
         yield AutogenToken.eof, None
 
     def do_need_name_end(self, token):
         if len(self.def_stack) > 1:
-            raise AutogenParseError(
-                "Definition blocks were left open", self.cur_line)
+            self.error("Definition blocks were left open")
 
     def do_need_name_var_name(self, token):
         self.new_name = token
 
     def do_end_block(self, token):
         if len(self.def_stack) <= 1:
-            raise AutogenParseError("Too many close braces", self.cur_line)
+            self.error("Too many close braces")
         new_name, parent_def = self.def_stack.pop()
         parent_def.append((new_name, self.curdef))
         self.curdef = parent_def
@@ -292,7 +293,7 @@ class AutogenParser:
     def do_indexed_name(self, token):
         self.new_name = token
 
-    def read_definitions(self, f):
+    def read_definitions_file(self, f):
         self.curdef = self.definitions
         self.cur_line = 0
         state = AutogenState.init
@@ -367,12 +368,17 @@ class AutogenParser:
                 if handler is not None:
                     handler(token)
             else:
-                raise AutogenParseError(
+                self.error(
                     "Parse error in state %s: unexpected token '%s'" % (
-                        state, token), self.cur_line)
+                        state, token))
             if state == AutogenState.done:
                 break
 
+    def read_definitions(self, path):
+        self.cur_file = path
+        with open(path) as f:
+            self.read_definitions_file(f)
+
 defparser = AutogenParser()
 
 #
@@ -791,8 +797,7 @@ parser = OptionParser(usage="%prog DEFINITION-FILES")
 _, args = parser.parse_args()
 
 for arg in args:
-    with open(arg) as f:
-        defparser.read_definitions(f)
+    defparser.read_definitions(arg)
 
 rules("module", module)
 rules("kernel", kernel)
-- 
1.8.4.3

[PATCH 3/3] Handle #if/#endif and C-style comments in AutoGen definitions files.

[Colin Watson]

---
 gentpl.py | 46 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/gentpl.py b/gentpl.py
index 5bee43a..0c5eabb 100644
--- a/gentpl.py
+++ b/gentpl.py
@@ -106,9 +106,11 @@ for platform in GRUB_PLATFORMS:
 #
 
 # We support a subset of the AutoGen definitions file syntax.  Specifically,
-# compound names are disallowed; C-style comments and preprocessing
-# directives are disallowed; and shell-generated strings, Scheme-generated
-# strings, and here strings are disallowed.
+# compound names are disallowed; some preprocessing directives are
+# disallowed (though #if/#endif are allowed; note that, like AutoGen, #if
+# skips everything to the next #endif regardless of the value of the
+# conditional); and shell-generated strings, Scheme-generated strings, and
+# here strings are disallowed.
 
 class AutogenToken:
     (autogen, definitions, eof, var_name, other_name, string, number,
@@ -189,7 +191,27 @@ class AutogenParser:
             if offset >= end:
                 break
             c = data[offset]
-            if c == "{":
+            if c == "#":
+                offset += 1
+                try:
+                    end_directive = data.index("\n", offset)
+                    directive = data[offset:end_directive]
+                    offset = end_directive
+                except ValueError:
+                    directive = data[offset:]
+                    offset = end
+                name, value = directive.split(None, 1)
+                if name == "if":
+                    try:
+                        end_if = data.index("\n#endif", offset)
+                        new_offset = end_if + len("\n#endif")
+                        self.cur_line += data[offset:new_offset].count("\n")
+                        offset = new_offset
+                    except ValueError:
+                        self.error("#if without matching #endif")
+                else:
+                    self.error("Unhandled directive '#%s'" % name)
+            elif c == "{":
                 yield AutogenToken.lbrace, c
                 offset += 1
             elif c == "=":
@@ -234,6 +256,22 @@ class AutogenParser:
                     else:
                         s.append(data[offset])
                 yield AutogenToken.string, "".join(s)
+            elif c == "/":
+                offset += 1
+                if data[offset] == "*":
+                    offset += 1
+                    try:
+                        end_comment = data.index("*/", offset)
+                        new_offset = end_comment + len("*/")
+                        self.cur_line += data[offset:new_offset].count("\n")
+                        offset = new_offset
+                    except ValueError:
+                        self.error("/* without matching */")
+                elif data[offset] == "/":
+                    try:
+                        offset = data.index("\n", offset)
+                    except ValueError:
+                        pass
             elif (c.isdigit() or
                   (c == "-" and offset < end - 1 and
                    data[offset + 1].isdigit())):
-- 
1.8.4.3