Go ahead for both trunk and extras
On 09/23/2010 03:08 PM, Colin Watson wrote:
> 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 <cjwat...@ubuntu.com>
>
> 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 $...@.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)/Makefile.tpl
> - autogen -T $(top_srcdir)/Makefile.tpl $< | sed -e '/^$$/{N;/^\\n$$/D;}'
> > $...@.new || (rm -f $...@.new; exit 1)
> - mv $...@.new $@
> -
> -.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;}'
> > $...@.new || (rm -f $...@.new; 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;}' >
> $...@.new || (rm -f $...@.new; exit 1)
> mv $...@.new $@
>
> === 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
>
> -- Regards Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
