[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Re-enable grub-extras
|
From: |
Colin Watson |
|
Subject: |
Re: [PATCH] Re-enable grub-extras |
|
Date: |
Thu, 23 Sep 2010 14:08:20 +0100 |
|
User-agent: |
Mutt/1.5.18 (2008-05-17) |
On Tue, Sep 21, 2010 at 07:35:30PM +0100, Colin Watson wrote:
> Here's a patch to re-enable grub-extras by allowing extra modules to
> provide Autogen definitions files. A few things to note:
Vladimir commented on this on IRC, noting three problems. In reverse
order of complexity:
1) The GRUB_CONTRIB check in autogen.sh should also check for
grub-core/contrib.
Fixed.
2) What's the problem with renaming g2hdr.bin to g2hdr.img?
I know of at least one piece of software which expects the current file
name: win32-loader, written by Robert and maintained in Debian. It
probably wouldn't be completely awful to change this, but it would be
nice not to need to. That said, perhaps g2hdr.bin could be a symlink to
g2hdr.img (which could be done in Makefile.common). I've updated my
patch to do this, and removed the extension-override facility from
gentpl.py.
3) It would be nice if extra modules could add new sources to existing
targets defined in one of the main definitions files: for example, it
would be useful to be able to add zfs to libgrub.a.
This was the hard bit that took me a day or two to fix.
Vladimir's original suggestion on IRC was to create a separate
libgrubzfs.a and arrange to link the utilities with that as well. When
I looked into this, I decided that I didn't like this approach because
it simply pushes the problem of appending to an existing list up a
level; as far as the Autogen definitions go, there isn't a significant
difference between appending to grub_probe_LDADD and appending to
libgrub_a_SOURCES. I would prefer to solve this some other way.
My preferred approach is to try not to expose any of the difficulties in
the definitions file, so that it can just look like this:
library = {
name = libgrub.a;
common = contrib/zfs/zfs.c;
common = contrib/zfs/zfs_lzjb.c;
common = contrib/zfs/zfs_sha256.c;
common = contrib/zfs/zfs_fletcher.c;
cppflags = '$(ZFS_CPPFLAGS)';
cflags = '$(ZFS_CFLAGS)';
};
Doing this actually turned out to be very difficult because Autogen's
native macros aren't powerful enough: they can only really emit text
rather than using temporary storage, which means that you can't do
things like "look for all the library blocks with the same name and
concatenate them" or "append to _SOURCES variables if we've seen them
before". Some Automake variables can be handled with simple appends
while some of them have some boilerplate only at the start which
shouldn't be repeated; Automake variables must be initialised exactly
once on all possible intersections of conditional paths; and we should
also avoid generating duplicate rules. All these constraints make it a
hard problem.
Fortunately, Autogen supports embedding bits of Guile, which makes the
problem merely hard rather than impossible. I tried a few different
approaches, and the one that I found to be most practical is in the
following patch: effectively we remember whether we've seen a given
target before and treat it slightly differently, and furthermore we emit
initialisations for non-global variables to avoid problems with
Automake's restrictions on variable initialisation.
For now, I only implemented this for 'library' blocks, so something that
wants to compile extra files into the GRUB kernel for example will be
out of luck. If people feel this is an onerous restriction then it's
fairly easy to fix; I was just trying not to change too much at once,
particularly for paths I wasn't going to test.
Doing this involved feeding all the files relevant to a given Automake
input stream to Autogen in one go, so I eliminated Makefile.gcry.am.
This was an exercise in programming in six languages at once (Python,
Autogen native macros, Guile, Autogen definitions, Automake, and make).
I found it to be a bit much, and my impression is that Autogen got in
the way more than it helped. I suggest that after 1.99 we should look
at having a Python program generate Automake input directly, which would
avoid many contortions and allow us to improve the output (it could
merge the multiple _SOURCES lines into just one per conditional path,
for instance). The .def file format is of course just fine as it is,
and could easily be preserved.
> * Extra modules may also provide Makefile.common with literal Automake
> input, for those cases where it's too hard to cram things into
> Autogen.
I also added Makefile.core.common and Makefile.util.common (starting to
run out of good names) for literal Automake input that should only be
used in grub-core or at the top level respectively. ntldr-img needed
the former.
Once again, the required patches to grub-extras modules are attached.
2010-09-22 Colin Watson <address@hidden>
Re-enable grub-extras.
* autogen.sh: Create symlinks to ${GRUB_CONTRIB} if necessary to
avoid confusing Automake. Run autogen only twice, once for the top
level and once for grub-core. Add Makefile.util.def and
Makefile.core.def from extra modules to the appropriate autogen
invocations. If Makefile.common exists in an extra module, include
it in both Makefile.util.am and grub-core/Makefile.core.am;
similarly, include any Makefile.util.common file in Makefile.util.am
and any Makefile.core.common file in grub-core/Makefile.core.am.
* conf/Makefile.common ($(top_srcdir)/grub-core/Makefile.core.am):
Depend on $(top_srcdir)/grub-core/Makefile.gcry.def.
($(top_srcdir)/grub-core/Makefile.gcry.def): Remove.
* grub-core/Makefile.am: Remove inclusion of Makefile.gcry.am.
* gentpl.py (gvar_add): Turn GVARS into a set.
(global_variable_initializers): Sort global variables on output.
(vars_init): New function.
(first_time): Likewise.
(library): Ensure that non-global variable initialisations are
emitted before the first time we emit code for a library block.
Append to variables rather than setting them. Only emit
noinst_LIBRARIES, BUILT_SOURCES, and CLEANFILES the first time for
each conditional path.
(program): installdir() emits an Autogen macro, so must be passed to
var_add rather than gvar_add.
(data): Likewise.
(script): Likewise.
(rules): New function, centralising handling for different target
types. Set up Guile association lists for first_time and vars_init,
and send most output to a diversion so that variable initialisations
can be emitted first.
(module_rules): Use new rules function.
(kernel_rules): Likewise.
(image_rules): Likewise.
(library_rules): Likewise.
(program_rules): Likewise.
(script_rules): Likewise.
(data_rules): Likewise.
* configure.ac: Add AC_PROG_LN_S, for the benefit of ntldr-img.
* .bzrignore: Add contrib and grub-core/contrib. Remove
grub-core/Makefile.gcry.am.
=== modified file '.bzrignore'
--- .bzrignore 2010-09-20 13:03:47 +0000
+++ .bzrignore 2010-09-22 13:07:15 +0000
@@ -104,9 +104,10 @@ grub-core/lib/libgcrypt-grub
**/.deps-core
**/.dirstamp
Makefile.util.am
+contrib
grub-core/Makefile.core.am
-grub-core/Makefile.gcry.am
grub-core/Makefile.gcry.def
+grub-core/contrib
grub-core/genmod.sh
grub-core/gensyminfo.sh
grub-core/*.module
=== modified file 'autogen.sh'
--- autogen.sh 2010-08-23 08:37:29 +0000
+++ autogen.sh 2010-09-23 10:36:52 +0000
@@ -14,9 +14,50 @@ echo "Creating Makefile.tpl..."
python gentpl.py | sed -e '/^$/{N;/^\n$/D;}' > Makefile.tpl
echo "Running autogen..."
-autogen -T Makefile.tpl Makefile.util.def | sed -e '/^$/{N;/^\n$/D;}' >
Makefile.util.am
-autogen -T Makefile.tpl grub-core/Makefile.core.def | sed -e
'/^$/{N;/^\n$/D;}' > grub-core/Makefile.core.am
-autogen -T Makefile.tpl grub-core/Makefile.gcry.def | sed -e
'/^$/{N;/^\n$/D;}' > grub-core/Makefile.gcry.am
+
+# Automake doesn't like including files from a path outside the project.
+rm -f contrib grub-core/contrib
+if [ "x${GRUB_CONTRIB}" != x ]; then
+ [ "${GRUB_CONTRIB}" = contrib ] || ln -s "${GRUB_CONTRIB}" contrib
+ [ "${GRUB_CONTRIB}" = grub-core/contrib ] || ln -s ../contrib
grub-core/contrib
+fi
+
+UTIL_DEFS=Makefile.util.def
+CORE_DEFS='grub-core/Makefile.core.def grub-core/Makefile.gcry.def'
+
+for extra in contrib/*/Makefile.util.def; do
+ if test -e "$extra"; then
+ UTIL_DEFS="$UTIL_DEFS $extra"
+ fi
+done
+
+for extra in contrib/*/Makefile.core.def; do
+ if test -e "$extra"; then
+ CORE_DEFS="$CORE_DEFS $extra"
+ 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
+
+for extra in contrib/*/Makefile.common; do
+ if test -e "$extra"; then
+ echo "include $extra" >> Makefile.util.am
+ echo "include $extra" >> grub-core/Makefile.core.am
+ fi
+done
+
+for extra in contrib/*/Makefile.util.common; do
+ if test -e "$extra"; then
+ echo "include $extra" >> Makefile.util.am
+ fi
+done
+
+for extra in contrib/*/Makefile.core.common; do
+ if test -e "$extra"; then
+ echo "include $extra" >> grub-core/Makefile.core.am
+ fi
+done
echo "Saving timestamps..."
echo timestamp > stamp-h.in
=== modified file 'conf/Makefile.common'
--- conf/Makefile.common 2010-09-21 12:37:50 +0000
+++ conf/Makefile.common 2010-09-22 13:09:44 +0000
@@ -146,11 +146,7 @@ $(top_srcdir)/Makefile.util.am: $(top_sr
mv address@hidden $@
.PRECIOUS: $(top_srcdir)/grub-core/Makefile.core.am
-$(top_srcdir)/grub-core/Makefile.core.am:
$(top_srcdir)/grub-core/Makefile.core.def $(top_srcdir)/Makefile.tpl
- autogen -T $(top_srcdir)/Makefile.tpl $< | sed -e '/^$$/{N;/^\\n$$/D;}'
> address@hidden || (rm -f address@hidden; exit 1)
- mv address@hidden $@
-
-.PRECIOUS: $(top_srcdir)/grub-core/Makefile.gcry.am
-$(top_srcdir)/grub-core/Makefile.gcry.am:
$(top_srcdir)/grub-core/Makefile.gcry.def $(top_srcdir)/Makefile.tpl
- autogen -T $(top_srcdir)/Makefile.tpl $< | sed -e '/^$$/{N;/^\\n$$/D;}'
> address@hidden || (rm -f address@hidden; exit 1)
+$(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
+ if [ "x$$GRUB_CONTRIB" != x ]; then echo "You need to run ./autogen.sh
manually." >&2; exit 1; fi
+ autogen -T $(top_srcdir)/Makefile.tpl
$(top_srcdir)/grub-core/Makefile.core.def
$(top_srcdir)/grub-core/Makefile.gcry.def | sed -e '/^$$/{N;/^\\n$$/D;}' >
address@hidden || (rm -f address@hidden; exit 1)
mv address@hidden $@
=== modified file 'configure.ac'
--- configure.ac 2010-09-21 09:42:30 +0000
+++ configure.ac 2010-09-22 12:11:35 +0000
@@ -235,6 +235,7 @@ AC_PROG_LEX
AC_PROG_YACC
AC_PROG_MAKE_SET
AC_PROG_MKDIR_P
+AC_PROG_LN_S
if test "x$LEX" = "x:"; then
AC_MSG_ERROR([flex is not found])
=== modified file 'gentpl.py'
--- gentpl.py 2010-09-19 13:24:45 +0000
+++ gentpl.py 2010-09-22 21:01:22 +0000
@@ -70,16 +70,15 @@ for platform in GRUB_PLATFORMS:
#
# Global variables
#
-GVARS = []
+GVARS = set()
def gvar_add(var, value):
- if var not in GVARS:
- GVARS.append(var)
+ GVARS.add(var)
return var + " += " + value + "\n"
def global_variable_initializers():
r = ""
- for var in GVARS:
+ for var in sorted(GVARS):
r += var + " ?= \n"
return r
@@ -87,6 +86,16 @@ def global_variable_initializers():
# 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)
+
def var_set(var, value):
return var + " = " + value + "\n"
@@ -257,6 +266,15 @@ def platform_ccasflags(p): return platfo
def platform_stripflags(p): return platform_specific_values(p, "_stripflags",
"stripflags")
def platform_objcopyflags(p): return platform_specific_values(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")
@@ -341,18 +359,25 @@ fi
def library(platform):
r = set_canonical_name_suffix("")
- r += gvar_add("noinst_LIBRARIES", "[+ name +]")
- r += var_set(cname() + "_SOURCES", platform_sources(platform))
- r += var_set("nodist_" + cname() + "_SOURCES",
platform_nodist_sources(platform))
- r += var_set(cname() + "_CFLAGS", "$(AM_CFLAGS) $(CFLAGS_LIBRARY) " +
platform_cflags(platform))
- r += var_set(cname() + "_CPPFLAGS", "$(AM_CPPFLAGS) $(CPPFLAGS_LIBRARY) "
+ platform_cppflags(platform))
- r += var_set(cname() + "_CCASFLAGS", "$(AM_CCASFLAGS) $(CCASFLAGS_LIBRARY)
" + platform_ccasflags(platform))
- # r += var_set(cname() + "_DEPENDENCIES", platform_dependencies(platform)
+ " " + platform_ldadd(platform))
- r += gvar_add("EXTRA_DIST", extra_dist())
- r += gvar_add("BUILT_SOURCES", "$(nodist_" + cname() + "_SOURCES)")
- r += gvar_add("CLEANFILES", "$(nodist_" + cname() + "_SOURCES)")
+ 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("EXTRA_DIST", 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"):
@@ -376,7 +401,7 @@ def program(platform, test=False):
r += gvar_add("check_PROGRAMS", "[+ name +]")
r += gvar_add("TESTS", "[+ name +]")
r += "[+ ELSE +]"
- r += gvar_add(installdir() + "_PROGRAMS", "[+ name +]")
+ r += var_add(installdir() + "_PROGRAMS", "[+ name +]")
r += "[+ IF mansection +]" + manpage() + "[+ ENDIF +]"
r += "[+ ENDIF +]"
@@ -397,7 +422,7 @@ def program(platform, test=False):
def data(platform):
r = gvar_add("EXTRA_DIST", platform_sources(platform))
r += gvar_add("EXTRA_DIST", extra_dist())
- r += gvar_add(installdir() + "_DATA", platform_sources(platform))
+ r += var_add(installdir() + "_DATA", platform_sources(platform))
return r
def script(platform):
@@ -405,7 +430,7 @@ def script(platform):
r += gvar_add("check_SCRIPTS", "[+ name +]")
r += gvar_add ("TESTS", "[+ name +]")
r += "[+ ELSE +]"
- r += gvar_add(installdir() + "_SCRIPTS", "[+ name +]")
+ r += var_add(installdir() + "_SCRIPTS", "[+ name +]")
r += "[+ IF mansection +]" + manpage() + "[+ ENDIF +]"
r += "[+ ENDIF +]"
@@ -418,33 +443,43 @@ chmod a+x [+ name +]
r += gvar_add("dist_noinst_DATA", platform_sources(platform))
return r
+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 "[+ FOR module +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, module(p))) + "[+
ENDFOR +]"
+ return rules("module", module)
def kernel_rules():
- return "[+ FOR kernel +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, kernel(p))) + "[+
ENDFOR +]"
+ return rules("kernel", kernel)
def image_rules():
- return "[+ FOR image +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, image(p))) + "[+
ENDFOR +]"
+ return rules("image", image)
def library_rules():
- return "[+ FOR library +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, library(p))) + "[+
ENDFOR +]"
+ return rules("library", library)
def program_rules():
- return "[+ FOR program +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, program(p))) + "[+
ENDFOR +]"
+ return rules("program", program)
def script_rules():
- return "[+ FOR script +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, script(p))) + "[+
ENDFOR +]"
+ return rules("script", script)
def data_rules():
- return "[+ FOR data +]" + foreach_enabled_platform(
- lambda p: under_platform_specific_conditionals(p, data(p))) + "[+
ENDFOR +]"
+ return rules("data", data)
print "[+ AutoGen5 template +]\n"
a = module_rules()
=== modified file 'grub-core/Makefile.am'
--- grub-core/Makefile.am 2010-09-19 20:22:43 +0000
+++ grub-core/Makefile.am 2010-09-22 13:11:11 +0000
@@ -51,7 +51,6 @@ grub_script.yy.c: grub_script.yy.h
CLEANFILES += grub_script.yy.c grub_script.yy.h
include $(srcdir)/Makefile.core.am
-include $(srcdir)/Makefile.gcry.am
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/cache.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h
--
Colin Watson address@hidden