[Buildroot] [PATCH v3] package/psplash: add support for missing configure options

[Arnout Vandecappelle]

On 11/12/2021 10:51, Arnout Vandecappelle wrote:
>
>
> On 16/11/2021 11:45, Kory Maincent wrote:
>> Add support to three configure options:
>> --enable-startup-msg: Enable text banner output on startup
>> --enable-progress-bar: Enable progress bar
>> --enable-img-fullscreen: Enable the logo image in fullscreen mode

  I forgot to mention before: the commit message should say explicitly that 
startup-msg and progress-bar default to enabled, so they have the corresponding 
default in Config.in.



  Regards,
  Arnout


>>
>> Signed-off-by: Kory Maincent <kory.maincent at bootlin.com>
>> ---
>>
>> Change since v1:
>> Remove the crap .config.in.swp
>>
>> Change since v2:
>> Use both side of enable/disable configure option condition.
>> Use positive options in Kconfig.
>>
>>
>>   package/psplash/Config.in  | 17 +++++++++++++++++
>>   package/psplash/psplash.mk | 18 ++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/package/psplash/Config.in b/package/psplash/Config.in
>> index da99b264e0..edd30dcb4d 100644
>> --- a/package/psplash/Config.in
>> +++ b/package/psplash/Config.in
>> @@ -35,6 +35,23 @@ config BR2_PACKAGE_PSPLASH_IMAGE
>>         Use a personalized png image as boot splash.
>>         Let it empty if you want to keep the psplash default image.
>>   +config BR2_PACKAGE_PSPLASH_FULL_SCREEN
>> +    bool "use fullscreen mode"
>> +    help
>> +      Enable the psplash image in fullscreen mode.
>> +
>> +config BR2_PACKAGE_PSPLASH_STARTUP_MSG
>> +    bool "enable startup message"
>> +    default y
>> +    help
>> +      Enable text banner output on startup.
>> +
>> +config BR2_PACKAGE_PSPLASH_PROGRESS_BAR
>> +    bool "enable progress bar"
>> +    default y
>> +    help
>> +      Enable the management of the progress bar.
>> +
>>   endif
>>     comment "psplash needs a toolchain w/ wchar"
>> diff --git a/package/psplash/psplash.mk b/package/psplash/psplash.mk
>> index 5e61f2392f..e5b212381a 100644
>> --- a/package/psplash/psplash.mk
>> +++ b/package/psplash/psplash.mk
>> @@ -18,6 +18,24 @@ else
>>   PSPLASH_CONF_OPTS += --without-systemd
>>   endif
>>   +ifeq ($(BR2_PACKAGE_PSPLASH_FULL_SCREEN),y)
>> +PSPLASH_CONF_OPTS += --enable-img-fullscreen
>> +else
>> +PSPLASH_CONF_OPTS += --disable-img-fullscreen
>
>  Have you actually tested this?
>
>  I haven't, but I see this in configure.ac:
>
> AC_ARG_ENABLE([img-fullscreen],
>     AS_HELP_STRING([--enable-img-fullscreen], [Enable the logo image in 
> fullscreen mode)]),
>     [img_fullscreen=true],
>     [img_fullscreen=false])
> AS_IF([test x$img_fullscreen = xtrue], [
>     EXTRA_GCC_FLAGS="$EXTRA_GCC_FLAGS -DPSPLASH_IMG_FULLSCREEN=1"
> ])
>
> i.a. AC_ARG_ENABLE is used incorrectly. That means that both 
> --enable-img-fullscreen and --disable-image-fullscreen will enable the option 
> (because the action-if-present is img_fullscreen=true), while if nothing is 
> set it will be disbled.
>
>  Same for the other two options, but there the logic is inverted (i.e. it's 
> disabled if present and enabled if not).
>
>  I don't know if psplash is still an active project; if so, I would propose to 
> fix this in configure.ac and send the patch upstream. The fix is simple:
>
>
> AC_ARG_ENABLE([img-fullscreen],
>     AS_HELP_STRING([--enable-img-fullscreen], [Enable the logo image in 
> fullscreen mode)]))
> AS_IF([test "x$enable_img_fullscreen" != "xno"], [
>     EXTRA_GCC_FLAGS="$EXTRA_GCC_FLAGS -DPSPLASH_IMG_FULLSCREEN=1"
> ])
>
>
>  and for enable-startup-msg:
>
> AC_ARG_ENABLE([startup-msg],
>     AS_HELP_STRING([--disable-startup-msg], [Disable text banner output on 
> startup]),
>     [],[enable_startup_msg=yes])
> AS_IF([test "x$enable_startup_msg" = "xno"], [
>     EXTRA_GCC_FLAGS="$EXTRA_GCC_FLAGS -DPSPLASH_DISABLE_STARTUP_MSG"
> ])
>
> (note: you have to use "no" because that is the value that is set when 
> --disable-foo is given. You shouldn't use anything else because 
> --enable-foo=no or --enable-foo=yes are also allowed).
>
>
>  Regards,
>  Arnout
>
>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_PSPLASH_STARTUP_MSG),y)
>> +PSPLASH_CONF_OPTS += --enable-startup-msg
>> +else
>> +PSPLASH_CONF_OPTS += --disable-startup-msg
>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_PSPLASH_PROGRESS_BAR),y)
>> +PSPLASH_CONF_OPTS += --enable-progress-bar
>> +else
>> +PSPLASH_CONF_OPTS += --disable-progress-bar
>> +endif
>> +
>>   PSPLASH_IMAGE = $(call qstrip,$(BR2_PACKAGE_PSPLASH_IMAGE))
>>     ifneq ($(PSPLASH_IMAGE),)
>>