Message ID | c1326c335d81ce3fc51c44244542c89402fc98d7.1532469861.git.geoff@infradead.org |
---|---|
State | Superseded |
Headers | show |
Series | [v1,01/30] docker: Add libfdt-dev | expand |
I like the direction this is going. One suggestion I might make is that as platforms are effectively independent modules anyway with their own registration interface, maybe also have an explicit 'use these platform files' configure option? Then it would be easy for developers to add in their custom platforms at build time without having to modify configure/makefiles. So on the command line something like: configure --enable-explicit-platforms=foo,/some/abs/path/bar.c --enable-explicit-platform-libs=baz eventually turns into something like this: discover_platform_ro_SOURCES += discover/platform-foo.c /some/abs/path/bar.c discover_platform_ro_LDFLAGS += -lbaz After a bit more configure/Makefile magic that I know I'm glossing over, but this is just to illustrate the idea for comment. The libs is of course if your platform module requires something outside the main lib list, depending on how far to take this. On 25/07/18 08:15, Geoff Levand wrote: > Add configure --enable-platform-XXX options to allow specifying > which platform support to build. > > --enable-platform-auto, the default, will use the host > triplet to guess which platforms to build. > --enable-platform-all will build all platforms. > > Signed-off-by: Geoff Levand <geoff@infradead.org> > --- > configure.ac | 38 ++++++++++++++++++++++++++++++++++++++ > discover/Makefile.am | 11 +++++++---- > 2 files changed, 45 insertions(+), 4 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 408a569..5b7c7b5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -303,6 +303,44 @@ AS_IF( > ] > ) > > +# platform choice > +AC_ARG_ENABLE( > + [platform-all], > + [AS_HELP_STRING( > + [--enable-platform-all], > + [build support for all platforms [default=no]] > + )] > +) > +AS_IF( > + [test "x$enable_platform_all" = "xyes"], > + [enable_platform_powerpc="yes"; enable_platform_ps3="yes"] > +) > + > +AC_ARG_ENABLE( > + [platform-auto], > + [AS_HELP_STRING( > + [--enable-platform-auto], > + [auto detect platform support to build [default=yes]] > + )], > + [], > + [enable_platform_auto="yes"] > +) > +AS_IF( > + [test "x$enable_platform_auto" = "xyes"], > + [AS_CASE([$host], > + [powerpc*-*-*], [enable_platform_powerpc="yes"], > + )] > +) > + > +AC_ARG_ENABLE( > + [platform-powerpc], > + [AS_HELP_STRING( > + [--enable-platform-powerpc], > + [build support for powerpc platforms [default=no]] > + )] > +) > +AM_CONDITIONAL([PLATFORM_POWERPC], [test "x$enable_platform_powerpc" = "xyes"]) > + > AC_ARG_ENABLE( > [platform-ps3], > [AS_HELP_STRING( > diff --git a/discover/Makefile.am b/discover/Makefile.am > index f9625ec..2b7c794 100644 > --- a/discover/Makefile.am > +++ b/discover/Makefile.am > @@ -80,11 +80,11 @@ discover_platform_ro_SOURCES = \ > discover/ipmi.h \ > discover/dt.c \ > discover/dt.h \ > - discover/hostboot.h \ > - discover/platform-powerpc.c > + discover/hostboot.h > > -discover_platform_ro_CPPFLAGS = \ > - $(AM_CPPFLAGS) > +if PLATFORM_POWERPC > +discover_platform_ro_SOURCES += discover/platform-powerpc.c > +endif > > # Build dummy last to put it at the end of the platforms section. > discover_platform_ro_SOURCES += discover/platform-dummy.c > @@ -99,5 +99,8 @@ discover_platform_ro_LDFLAGS = \ > > endif > > +discover_platform_ro_CPPFLAGS = \ > + $(AM_CPPFLAGS) > + > discover_platform_ro_LINK = \ > $(LD) -r -o $@
Hi Brett, On 07/29/2018 08:30 PM, Brett Grandbois wrote: > I like the direction this is going. One suggestion I might make is that as platforms are effectively independent modules anyway with their own registration interface, maybe also have an explicit 'use these platform files' configure option? Then it would be easy for developers to add in their custom platforms at build time without having to modify configure/makefiles. > > So on the command line something like: > > configure --enable-explicit-platforms=foo,/some/abs/path/bar.c --enable-explicit-platform-libs=baz > > eventually turns into something like this: > > discover_platform_ro_SOURCES += discover/platform-foo.c /some/abs/path/bar.c > > discover_platform_ro_LDFLAGS += -lbaz > > After a bit more configure/Makefile magic that I know I'm glossing over, but this is just to illustrate the idea for comment. > > The libs is of course if your platform module requires something outside the main lib list, depending on how far to take this. What you suggest does not lend itself to autoconf/automake. I don't think there would be enough use of it to warrant creating a custom config system. See: https://www.gnu.org/software/autoconf/manual/autoconf.html#Site-Configuration -Geoff
On 31/07/18 05:36, Geoff Levand wrote: > Hi Brett, > > On 07/29/2018 08:30 PM, Brett Grandbois wrote: >> I like the direction this is going. One suggestion I might make is that as platforms are effectively independent modules anyway with their own registration interface, maybe also have an explicit 'use these platform files' configure option? Then it would be easy for developers to add in their custom platforms at build time without having to modify configure/makefiles. >> >> So on the command line something like: >> >> configure --enable-explicit-platforms=foo,/some/abs/path/bar.c --enable-explicit-platform-libs=baz >> >> eventually turns into something like this: >> >> discover_platform_ro_SOURCES += discover/platform-foo.c /some/abs/path/bar.c >> >> discover_platform_ro_LDFLAGS += -lbaz >> >> After a bit more configure/Makefile magic that I know I'm glossing over, but this is just to illustrate the idea for comment. >> >> The libs is of course if your platform module requires something outside the main lib list, depending on how far to take this. > What you suggest does not lend itself to autoconf/automake. I would disagree with that. Autotools just sits on top of bash (or some shell) at the end of the day so doing text processing wouldn't be an issue. In this particular case look at the signed-boot configure input processing, which takes input strings and makes decisions based on them. Once you get a string you can easily separate it via whatever delimiters you specify and then they can be exported into whatever make variables you need. Now whether you want to do that or not is a different discussion, which is below. > I don't think there would be enough use of it to warrant > creating a custom config system. See: Now that is more of the crux of the discussion. What is the intended use case here? Is there not enough demand for this sort of thing because there currently isn't a simple mechanism to achieve it? Is it a case of 'if you build it, they will come'? Or are you correct and there's a limited audience for this? I don't know the answers to those questions, I can just make observations based on how I'm using it or intending to use it. My vision of the platforms is sort of the petitboot equivalent of external kernel modules. They give an adopter a means to customize the configuration to fit their particular application so a simpler way to achieve that would mean to me then simpler means for developers to adopt petitboot to fit their platform. So if the decision is not to go this sort of route, I'm OK with that, I'm just making some feature suggestions. In either case it wouldn't need to be part of this patch series anyway as it goes above and beyond the intent here. > > https://www.gnu.org/software/autoconf/manual/autoconf.html#Site-Configuration > > -Geoff
diff --git a/configure.ac b/configure.ac index 408a569..5b7c7b5 100644 --- a/configure.ac +++ b/configure.ac @@ -303,6 +303,44 @@ AS_IF( ] ) +# platform choice +AC_ARG_ENABLE( + [platform-all], + [AS_HELP_STRING( + [--enable-platform-all], + [build support for all platforms [default=no]] + )] +) +AS_IF( + [test "x$enable_platform_all" = "xyes"], + [enable_platform_powerpc="yes"; enable_platform_ps3="yes"] +) + +AC_ARG_ENABLE( + [platform-auto], + [AS_HELP_STRING( + [--enable-platform-auto], + [auto detect platform support to build [default=yes]] + )], + [], + [enable_platform_auto="yes"] +) +AS_IF( + [test "x$enable_platform_auto" = "xyes"], + [AS_CASE([$host], + [powerpc*-*-*], [enable_platform_powerpc="yes"], + )] +) + +AC_ARG_ENABLE( + [platform-powerpc], + [AS_HELP_STRING( + [--enable-platform-powerpc], + [build support for powerpc platforms [default=no]] + )] +) +AM_CONDITIONAL([PLATFORM_POWERPC], [test "x$enable_platform_powerpc" = "xyes"]) + AC_ARG_ENABLE( [platform-ps3], [AS_HELP_STRING( diff --git a/discover/Makefile.am b/discover/Makefile.am index f9625ec..2b7c794 100644 --- a/discover/Makefile.am +++ b/discover/Makefile.am @@ -80,11 +80,11 @@ discover_platform_ro_SOURCES = \ discover/ipmi.h \ discover/dt.c \ discover/dt.h \ - discover/hostboot.h \ - discover/platform-powerpc.c + discover/hostboot.h -discover_platform_ro_CPPFLAGS = \ - $(AM_CPPFLAGS) +if PLATFORM_POWERPC +discover_platform_ro_SOURCES += discover/platform-powerpc.c +endif # Build dummy last to put it at the end of the platforms section. discover_platform_ro_SOURCES += discover/platform-dummy.c @@ -99,5 +99,8 @@ discover_platform_ro_LDFLAGS = \ endif +discover_platform_ro_CPPFLAGS = \ + $(AM_CPPFLAGS) + discover_platform_ro_LINK = \ $(LD) -r -o $@
Add configure --enable-platform-XXX options to allow specifying which platform support to build. --enable-platform-auto, the default, will use the host triplet to guess which platforms to build. --enable-platform-all will build all platforms. Signed-off-by: Geoff Levand <geoff@infradead.org> --- configure.ac | 38 ++++++++++++++++++++++++++++++++++++++ discover/Makefile.am | 11 +++++++---- 2 files changed, 45 insertions(+), 4 deletions(-)