[Buildroot] [PATCH] new variable <pkg>_CONFIG_FIXUP

Thomas Petazzoni thomas.petazzoni at free-electrons.com
Mon Jan 7 22:10:37 UTC 2013

Dear Stefan Fröberg,

On Tue,  8 Jan 2013 00:01:18 +0200, Stefan Fröberg wrote:
> Signed-off-by: Stefan Fröberg <stefan.froberg at petroprogram.com>

Please add more details to your commit log. I.e, basically, most of your
introduction e-mail contents should go here.

Remember: the contents of your introduction e-mail are not preserved in
the Git history. On the opposite, the commit log is preserved forever
in the Git history, so we want the details to be here.

> ---
>  package/pkg-generic.mk |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index a570ad7..a410ad1 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>  	@$(call MESSAGE,"Installing to staging directory")
>  	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
> +	@$(call MESSAGE,"Fixing package configuration files")

Please show the message only if there are actually files to fixup. Or
maybe just don't display any message at all. Fixing those files is kind
of a detail.

> +	for file in $($(PKG)_CONFIG_FIXUP); do \
> +		if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \

No, we should not silently error out if the file doesn't exist, because
it's hiding a mistake of package .mk file. Instead, if the file doesn't
exist, we should loudly error out.

Also, please include an update to the Buildroot manual, either as part
of this patch, or as a separate patch.

Thanks for your work!

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.

More information about the buildroot mailing list