[v1,27/30] configure: Add conditional platform builds
diff mbox series

Message ID c1326c335d81ce3fc51c44244542c89402fc98d7.1532469861.git.geoff@infradead.org
State Superseded
Headers show
Series
  • [v1,01/30] docker: Add libfdt-dev
Related show

Commit Message

Geoff Levand July 24, 2018, 10:15 p.m. UTC
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(-)

Comments

Grandbois, Brett July 30, 2018, 3:30 a.m. UTC | #1
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 $@
Geoff Levand July 30, 2018, 7:36 p.m. UTC | #2
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
Grandbois, Brett July 30, 2018, 10:58 p.m. UTC | #3
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

Patch
diff mbox series

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 $@