diff mbox

Added local package support.

Message ID 1343247448-19993-1-git-send-email-avishorp@gmail.com
State Superseded
Headers show

Commit Message

Avishay Orpaz July 25, 2012, 8:17 p.m. UTC
From: Avishay O <avishorp@gmail.com>

Local packages are packages that are local to the build and not distributed with buildroot.
They are placed in a directory named "locals" (by default) and follow the same rules
as other packages (Config.in, <pkgname>.mk, patches ..). Their configuration options
are automatically collected and placed under the "Local Packages" menu in x/menu/.../config.
---
 Makefile          |   48 ++++++++++++++++++++++++++++++------------------
 package/Config.in |    4 ++++
 2 files changed, 34 insertions(+), 18 deletions(-)

Comments

Arnout Vandecappelle July 25, 2012, 11:18 p.m. UTC | #1
On 07/25/12 22:17, Avishay Orpaz wrote:
> From: Avishay O<avishorp@gmail.com>
>
> Local packages are packages that are local to the build and not distributed with buildroot.
> They are placed in a directory named "locals" (by default) and follow the same rules
> as other packages (Config.in,<pkgname>.mk, patches ..). Their configuration options
> are automatically collected and placed under the "Local Packages" menu in x/menu/.../config.

  I'm going to give comments on this patch, even though I'm not sure if I would
want to have this feature in buildroot.

> ---
>   Makefile          |   48 ++++++++++++++++++++++++++++++------------------
>   package/Config.in |    4 ++++
>   2 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 639fdaa..e9342e9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -192,6 +192,7 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
>
>   BUILD_DIR:=$(BASE_DIR)/build
>
> +LOCAL_PACKAGES_DIR:=locals

  Please use <space>=<space> for variable assignments.  The BUILD_DIR above
is historical.

  Is there an added value to define a variable for this?

  Also, I'd call the directory 'local' rather than 'locals'.

>
>   ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>
> @@ -315,6 +316,9 @@ endif
>
>   include package/*/*.mk
>
> +# Include local packages
> +include $(LOCAL_PACKAGES_DIR)/*/*.mk

  Can you verify if this still works when the LOCAL_PACKAGES_DIR is
empty or doesn't exist?

> +
>   include boot/common.mk
>   include target/Makefile.in
>   include linux/linux.mk
> @@ -565,43 +569,51 @@ COMMON_CONFIG_ENV = \
>   	KCONFIG_TRISTATE=$(BUILD_DIR)/buildroot-config/tristate.config \
>   	BUILDROOT_CONFIG=$(CONFIG_DIR)/.config
>
> -xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile
> +LOCALS_CONFIG_FILE = .locals.Config.in
> +
> +$(LOCALS_CONFIG_FILE):
> +	rm -f $(@)
> +	find $(LOCAL_PACKAGES_DIR)  \
> +	-regex '^$(LOCAL_PACKAGES_DIR)/[^/]*/Config.in$$' \
> +	-exec cat {} \;>>  $@ || true

  There is no real need to generate this file automatically.  I'd keep it
manually maintained, like for the normal packages.

  Also we normally don't create _any_ file outside $(O), so that the buildroot
source itself can be read-only.  This file is of course a special case,
because it belongs with the locals directory.

> +
> +xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
> -gconfig: $(BUILD_DIR)/buildroot-config/gconf outputmakefile
> +gconfig: $(BUILD_DIR)/buildroot-config/gconf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) srctree=$(TOPDIR) $<  $(CONFIG_CONFIG_IN)
>
> -menuconfig: $(BUILD_DIR)/buildroot-config/mconf outputmakefile
> +menuconfig: $(BUILD_DIR)/buildroot-config/mconf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
> -nconfig: $(BUILD_DIR)/buildroot-config/nconf outputmakefile
> +nconfig: $(BUILD_DIR)/buildroot-config/nconf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
> -config: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +config: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
> -oldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +oldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --oldconfig $(CONFIG_CONFIG_IN)
>
> -randconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +randconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --randconfig $(CONFIG_CONFIG_IN)
>
> -allyesconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +allyesconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --allyesconfig $(CONFIG_CONFIG_IN)
>
> -allnoconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +allnoconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --allnoconfig $(CONFIG_CONFIG_IN)
>
> -randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>   	@$(COMMON_CONFIG_ENV) \
> @@ -609,7 +621,7 @@ randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>   		$<  --randconfig $(CONFIG_CONFIG_IN)
>   	@rm -f $(CONFIG_DIR)/.config.nopkg
>
> -allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>   	@$(COMMON_CONFIG_ENV) \
> @@ -617,7 +629,7 @@ allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>   		$<  --allyesconfig $(CONFIG_CONFIG_IN)
>   	@rm -f $(CONFIG_DIR)/.config.nopkg
>
> -allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>   	@$(COMMON_CONFIG_ENV) \
> @@ -625,19 +637,19 @@ allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>   		$<  --allnoconfig $(CONFIG_CONFIG_IN)
>   	@rm -f $(CONFIG_DIR)/.config.nopkg
>
> -silentoldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +silentoldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	$(COMMON_CONFIG_ENV) $<  --silentoldconfig $(CONFIG_CONFIG_IN)
>
> -defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --defconfig$(if $(BR2_DEFCONFIG),=$(BR2_DEFCONFIG)) $(CONFIG_CONFIG_IN)
>
> -%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(TOPDIR)/configs/%_defconfig outputmakefile
> +%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(TOPDIR)/configs/%_defconfig outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN)
>
> -savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
> +savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
>   	@mkdir -p $(BUILD_DIR)/buildroot-config
>   	@$(COMMON_CONFIG_ENV) $<  --savedefconfig=$(CONFIG_DIR)/defconfig $(CONFIG_CONFIG_IN)
>
> @@ -664,7 +676,7 @@ endif
>   clean:
>   	rm -rf $(STAGING_DIR) $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
>   		$(STAMP_DIR) $(BUILD_DIR) $(TOOLCHAIN_DIR) $(BASE_DIR)/staging \
> -		$(LEGAL_INFO_DIR)
> +		$(LEGAL_INFO_DIR) $(LOCALS_CONFIG_FILE)

  This shouldn't be cleaned in the clean target, but in the distclean target.

>
>   distclean: clean
>   ifeq ($(DL_DIR),$(TOPDIR)/dl)
> @@ -760,5 +772,5 @@ print-version:
>
>   include docs/manual/manual.mk
>
> -.PHONY: $(noconfig_targets)
> +.PHONY: $(noconfig_targets) $(LOCALS_CONFIG_FILE)
>
> diff --git a/package/Config.in b/package/Config.in
> index f664b8e..06a779c 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -695,4 +695,8 @@ source "package/vim/Config.in"
>   endif
>   endmenu
>
> +menu "Local Packages"
> +source ".locals.Config.in"
> +endmenu
> +
>   endmenu

.locals.Config.in should also be included in the .gitignore.

  Regards,
  Arnout
Arnout Vandecappelle July 27, 2012, 8:08 a.m. UTC | #2
[Please keep the list in CC]

On 07/26/12 16:56, Avishay Orpaz wrote:
> Hi Arnout, thanks for the comments. I'll repost the patch when fixed.
>
> 2012/7/26 Arnout Vandecappelle <arnout@mind.be <mailto:arnout@mind.be>>
>
>     On 07/25/12 22:17, Avishay Orpaz wrote:
>
>         From: Avishay O<avishorp@gmail.com <mailto:avishorp@gmail.com>>
>
>         Local packages are packages that are local to the build and not distributed with buildroot.
>         They are placed in a directory named "locals" (by default) and follow the same rules
>         as other packages (Config.in,<pkgname>.mk, patches ..). Their configuration options
>         are automatically collected and placed under the "Local Packages" menu in x/menu/.../config.
>
>
>       I'm going to give comments on this patch, even though I'm not sure if I would
>     want to have this feature in buildroot.
>
>
> Why? I think it's very convinient to leave buildroot untouched (and easy to upgrade) and put the system specific
> packages in a different place.

  That is currently usually done by (manually) adding a packages/<company name>/
directory which contains all the local packages, with a manually created
Config.in.  So what this patch adds is mainly to build Config.in automatically.

>Anyway, if you don't intend to include the patch please let me know so I won't bother to
> make corrections.

  That's why I said immediately that I'm not a big fan of this feature.

  What do the others think?

  Regards,
  Arnout


  [Quoting the rest of the message because it was not posted on the list]

>
>
>
>         ---
>            Makefile          |   48 ++++++++++++++++++++++++++++++------------------
>            package/Config.in |    4 ++++
>            2 files changed, 34 insertions(+), 18 deletions(-)
>
>         diff --git a/Makefile b/Makefile
>         index 639fdaa..e9342e9 100644
>         --- a/Makefile
>         +++ b/Makefile
>         @@ -192,6 +192,7 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
>
>            BUILD_DIR:=$(BASE_DIR)/build
>
>         +LOCAL_PACKAGES_DIR:=locals
>
>
>       Please use <space>=<space> for variable assignments.  The BUILD_DIR above
>     is historical.
>
> OK.
>
>       Is there an added value to define a variable for this?
>
> I thinks it's good to keep things configurable when possible. I'm actually using such method to build multiple systems
> with a single buildroot copy.
>
>       Also, I'd call the directory 'local' rather than 'locals'.
>
> OK.
>
>
>
>            ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>
>         @@ -315,6 +316,9 @@ endif
>
>            include package/*/*.mk
>
>         +# Include local packages
>         +include $(LOCAL_PACKAGES_DIR)/*/*.mk
>
>
>       Can you verify if this still works when the LOCAL_PACKAGES_DIR is
>     empty or doesn't exist?
>
> It does.
>
>
>         +
>            include boot/common.mk <http://common.mk>
>            include target/Makefile.in
>            include linux/linux.mk <http://linux.mk>
>         @@ -565,43 +569,51 @@ COMMON_CONFIG_ENV = \
>                  KCONFIG_TRISTATE=$(BUILD_DIR)/__buildroot-config/tristate.__config \
>                  BUILDROOT_CONFIG=$(CONFIG_DIR)__/.config
>
>         -xconfig: $(BUILD_DIR)/buildroot-config/__qconf outputmakefile
>         +LOCALS_CONFIG_FILE = .locals.Config.in <http://locals.Config.in>
>         +
>         +$(LOCALS_CONFIG_FILE):
>         +       rm -f $(@)
>         +       find $(LOCAL_PACKAGES_DIR)  \
>         +       -regex '^$(LOCAL_PACKAGES_DIR)/[^/]*/__Config.in$$' \
>         +       -exec cat {} \;>>  $@ || true
>
>
>       There is no real need to generate this file automatically.  I'd keep it
>     manually maintained, like for the normal packages.
>
> It's for convenience. The local package directory is very dynamic. In my opinion, if you want to add a package to the
> project just put it there and it will show up.
>
>       Also we normally don't create _any_ file outside $(O), so that the buildroot
>     source itself can be read-only.  This file is of course a special case,
>     because it belongs with the locals directory.
>
> The problem is that the kconfig configuration files don't expand environment variables, so it's impossible to include a
> Config.in file from a variable directory. Anyway, the .config is created in the invocation directory.
>
>
>         +
>         +xconfig: $(BUILD_DIR)/buildroot-config/__qconf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
>         -gconfig: $(BUILD_DIR)/buildroot-config/__gconf outputmakefile
>         +gconfig: $(BUILD_DIR)/buildroot-config/__gconf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) srctree=$(TOPDIR) $<  $(CONFIG_CONFIG_IN)
>
>         -menuconfig: $(BUILD_DIR)/buildroot-config/__mconf outputmakefile
>         +menuconfig: $(BUILD_DIR)/buildroot-config/__mconf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
>         -nconfig: $(BUILD_DIR)/buildroot-config/__nconf outputmakefile
>         +nconfig: $(BUILD_DIR)/buildroot-config/__nconf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
>         -config: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +config: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  $(CONFIG_CONFIG_IN)
>
>         -oldconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +oldconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --oldconfig $(CONFIG_CONFIG_IN)
>
>         -randconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +randconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --randconfig $(CONFIG_CONFIG_IN)
>
>         -allyesconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +allyesconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --allyesconfig $(CONFIG_CONFIG_IN)
>
>         -allnoconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +allnoconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --allnoconfig $(CONFIG_CONFIG_IN)
>
>         -randpackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +randpackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>                  @$(COMMON_CONFIG_ENV) \
>         @@ -609,7 +621,7 @@ randpackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>                          $<  --randconfig $(CONFIG_CONFIG_IN)
>                  @rm -f $(CONFIG_DIR)/.config.nopkg
>
>         -allyespackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +allyespackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>                  @$(COMMON_CONFIG_ENV) \
>         @@ -617,7 +629,7 @@ allyespackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>                          $<  --allyesconfig $(CONFIG_CONFIG_IN)
>                  @rm -f $(CONFIG_DIR)/.config.nopkg
>
>         -allnopackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +allnopackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config>  $(CONFIG_DIR)/.config.nopkg
>                  @$(COMMON_CONFIG_ENV) \
>         @@ -625,19 +637,19 @@ allnopackageconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>                          $<  --allnoconfig $(CONFIG_CONFIG_IN)
>                  @rm -f $(CONFIG_DIR)/.config.nopkg
>
>         -silentoldconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +silentoldconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  $(COMMON_CONFIG_ENV) $<  --silentoldconfig $(CONFIG_CONFIG_IN)
>
>         -defconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +defconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --defconfig$(if $(BR2_DEFCONFIG),=$(BR2___DEFCONFIG)) $(CONFIG_CONFIG_IN)
>
>         -%_defconfig: $(BUILD_DIR)/buildroot-config/__conf $(TOPDIR)/configs/%_defconfig outputmakefile
>         +%_defconfig: $(BUILD_DIR)/buildroot-config/__conf $(TOPDIR)/configs/%_defconfig outputmakefile
>         $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --defconfig=$(TOPDIR)/configs/__$@ $(CONFIG_CONFIG_IN)
>
>         -savedefconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile
>         +savedefconfig: $(BUILD_DIR)/buildroot-config/__conf outputmakefile $(LOCALS_CONFIG_FILE)
>                  @mkdir -p $(BUILD_DIR)/buildroot-config
>                  @$(COMMON_CONFIG_ENV) $<  --savedefconfig=$(CONFIG_DIR)/__defconfig $(CONFIG_CONFIG_IN)
>
>         @@ -664,7 +676,7 @@ endif
>            clean:
>                  rm -rf $(STAGING_DIR) $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
>                          $(STAMP_DIR) $(BUILD_DIR) $(TOOLCHAIN_DIR) $(BASE_DIR)/staging \
>         -               $(LEGAL_INFO_DIR)
>         +               $(LEGAL_INFO_DIR) $(LOCALS_CONFIG_FILE)
>
>
>       This shouldn't be cleaned in the clean target, but in the distclean target.
>
> The $(LOCALS_CONFIG_FILE) should be considered as a runtime temporary file, not a configuration file. It's not like
> .config or so.
>
>
>
>            distclean: clean
>            ifeq ($(DL_DIR),$(TOPDIR)/dl)
>         @@ -760,5 +772,5 @@ print-version:
>
>            include docs/manual/manual.mk <http://manual.mk>
>
>         -.PHONY: $(noconfig_targets)
>         +.PHONY: $(noconfig_targets) $(LOCALS_CONFIG_FILE)
>
>         diff --git a/package/Config.in b/package/Config.in
>         index f664b8e..06a779c 100644
>         --- a/package/Config.in
>         +++ b/package/Config.in
>         @@ -695,4 +695,8 @@ source "package/vim/Config.in"
>            endif
>            endmenu
>
>         +menu "Local Packages"
>         +source ".locals.Config.in <http://locals.Config.in>"
>         +endmenu
>         +
>            endmenu
>
>
>     .locals.Config.in <http://locals.Config.in> should also be included in the .gitignore.
>
> OK.
>
>       Regards,
>       Arnout
>     --
>     Arnout Vandecappelle                               arnout at mind be
>     Senior Embedded Software Architect                 +32-16-286540
>     Essensium/Mind http://www.mind.be
>     G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
>     LinkedIn profile: http://www.linkedin.com/in/__arnoutvandecappelle <http://www.linkedin.com/in/arnoutvandecappelle>
>     GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
>
Thomas Petazzoni July 27, 2012, 8:12 a.m. UTC | #3
Le Fri, 27 Jul 2012 10:08:13 +0200,
Arnout Vandecappelle <arnout@mind.be> a écrit :

> >       I'm going to give comments on this patch, even though I'm not
> > sure if I would want to have this feature in buildroot.
> >
> >
> > Why? I think it's very convinient to leave buildroot untouched (and
> > easy to upgrade) and put the system specific packages in a
> > different place.
> 
>   That is currently usually done by (manually) adding a
> packages/<company name>/ directory which contains all the local
> packages, with a manually created Config.in.  So what this patch adds
> is mainly to build Config.in automatically.
> 
> >Anyway, if you don't intend to include the patch please let me know
> >so I won't bother to
> > make corrections.
> 
>   That's why I said immediately that I'm not a big fan of this
> feature.
> 
>   What do the others think?

I am also a bit skeptical, but it's not yet a final and definitive
opinion. Custom packages is one thing, but one also very often need to
put stuff in board/ to keep kernel config, patches and various other
stuff, though it's true nothing prevent you to use $(TOPDIR)/../ to
keep track of all those things.

Thomas
Simon Dawson July 27, 2012, 8:48 a.m. UTC | #4
On 27 July 2012 09:12, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> I am also a bit skeptical, but it's not yet a final and definitive
> opinion. Custom packages is one thing, but one also very often need to
> put stuff in board/ to keep kernel config, patches and various other
> stuff, though it's true nothing prevent you to use $(TOPDIR)/../ to
> keep track of all those things.

I can sort of see that this might be useful. But I wonder if there's a
more subtle sociological reason not to add the local package support.
Specifically, does it encourage a culture of users building up
stockpiles of local packages, because that's perceived to be "how you
add support for a package in Buildroot"? If so, then users might be
less likely to push their new packages upstream to Buildroot, and the
community as a whole loses out.

Maybe that's overthinking it somewhat though.

Simon.
Richard Braun July 27, 2012, 8:53 a.m. UTC | #5
On Fri, Jul 27, 2012 at 10:12:53AM +0200, Thomas Petazzoni wrote:
> Le Fri, 27 Jul 2012 10:08:13 +0200,
> Arnout Vandecappelle <arnout@mind.be> a écrit :
> >   What do the others think?
> 
> I am also a bit skeptical, but it's not yet a final and definitive
> opinion. Custom packages is one thing, but one also very often need to
> put stuff in board/ to keep kernel config, patches and various other
> stuff, though it's true nothing prevent you to use $(TOPDIR)/../ to
> keep track of all those things.

I don't really see the point of the feature either. Keeping custom files
in board and local packages in package/<company>/ is very easy to do and
should suit most people fine. Local packages (those that use the local
method) have one purpose : ease software development through the
buildroot environment. I don't think people spend that much time on the
.mk or Config.in files when using local packages. They spend time on
the packaged software itself.
Avishay Orpaz July 29, 2012, 7:02 a.m. UTC | #6
Let me try to make my point with an example. Let's say I'm designing a
linux based digital camera. The software stack will probably include a
bunch of standard software component (kernel, busybox, flash utils etc.),
but there would also be at least one software component that is unique to
the design, and not intended to be shared with other designs - for example,
the GUI implementation of that specific model. This is the kind of software
I expect to see in the "local" directory. Of course this software component
can be put in the "package" directory, but I would think of buildroot as a
tool, which should not be modified and can be easily upgraded.

Regarding to the comment that other files in other directories may need to
be customized - it's very easy to put those files in any directory using
make variables according to one's project organization preference.

Committed or not, I really appreciate everyone's comments.


2012/7/27 Richard Braun <rbraun@sceen.net>

> On Fri, Jul 27, 2012 at 10:12:53AM +0200, Thomas Petazzoni wrote:
> > Le Fri, 27 Jul 2012 10:08:13 +0200,
> > Arnout Vandecappelle <arnout@mind.be> a écrit :
> > >   What do the others think?
> >
> > I am also a bit skeptical, but it's not yet a final and definitive
> > opinion. Custom packages is one thing, but one also very often need to
> > put stuff in board/ to keep kernel config, patches and various other
> > stuff, though it's true nothing prevent you to use $(TOPDIR)/../ to
> > keep track of all those things.
>
> I don't really see the point of the feature either. Keeping custom files
> in board and local packages in package/<company>/ is very easy to do and
> should suit most people fine. Local packages (those that use the local
> method) have one purpose : ease software development through the
> buildroot environment. I don't think people spend that much time on the
> .mk or Config.in files when using local packages. They spend time on
> the packaged software itself.
>
> --
> Richard Braun
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Arnout Vandecappelle July 29, 2012, 3:56 p.m. UTC | #7
On 07/29/12 09:02, Avishay Orpaz wrote:
> Let me try to make my point with an example. Let's say I'm designing a linux based digital camera. The software stack
> will probably include a bunch of standard software component (kernel, busybox, flash utils etc.), but there would also
> be at least one software component that is unique to the design, and not intended to be shared with other designs - for
> example, the GUI implementation of that specific model. This is the kind of software I expect to see in the "local"
> directory. Of course this software component can be put in the "package" directory, but I would think of buildroot as a
> tool, which should not be modified and can be easily upgraded.

  But putting it in a subdirectory 'local' within the buildroot directory still
has the same problem...

  As for ease of upgrading buildroot when there is a package/<company name>
directory: Peter Korsgaard (the main buildroot maintainer) does exactly that
at his work, and I don't think he has any issue with it.  You just need one
additional line at the top of package/Config.in to include
package/<company name>/Config.in


  That said, I would also like the possibility to extend buildroot with "local"
packages, board-specific files, rootfs skeletons, etc.  That gives my customers
the possibility to fully separate the open source stuff from the custom stuff.
(I know I'm changing my opinion here, but only idiots never change their mind :-)
But then, the local directory should be completely outside the buildroot directory.
And I still don't really like the automatic creation of the Config.in file.
I'm not sure what could be the alternative, though, because as you mention
the source-ing of a Config.in can't be done conditionally or using a variable
name.  Perhaps Kconfig itself should be extended to support that?


> Regarding to the comment that other files in other directories may need to be customized - it's very easy to put those
> files in any directory using make variables according to one's project organization preference.

  Or better yet, define a default project directory layout that sets
paths like BR2_LINUX_KERNEL_PATCH automatically.


  Regards,
  Arnout
Tzu-Jung Lee July 29, 2012, 4:34 p.m. UTC | #8
On Sun, Jul 29, 2012 at 11:56 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 07/29/12 09:02, Avishay Orpaz wrote:
>>
>> Let me try to make my point with an example. Let's say I'm designing a
>> linux based digital camera. The software stack
>> will probably include a bunch of standard software component (kernel,
>> busybox, flash utils etc.), but there would also
>> be at least one software component that is unique to the design, and not
>> intended to be shared with other designs - for
>> example, the GUI implementation of that specific model. This is the kind
>> of software I expect to see in the "local"
>> directory. Of course this software component can be put in the "package"
>> directory, but I would think of buildroot as a
>> tool, which should not be modified and can be easily upgraded.
>
>
>  But putting it in a subdirectory 'local' within the buildroot directory
> still
> has the same problem...
>
>  As for ease of upgrading buildroot when there is a package/<company name>
> directory: Peter Korsgaard (the main buildroot maintainer) does exactly that
> at his work, and I don't think he has any issue with it.  You just need one
> additional line at the top of package/Config.in to include
> package/<company name>/Config.in
>
>
>  That said, I would also like the possibility to extend buildroot with
> "local"
> packages, board-specific files, rootfs skeletons, etc.  That gives my
> customers
> the possibility to fully separate the open source stuff from the custom
> stuff.
> (I know I'm changing my opinion here, but only idiots never change their
> mind :-)
> But then, the local directory should be completely outside the buildroot
> directory.
> And I still don't really like the automatic creation of the Config.in file.
> I'm not sure what could be the alternative, though, because as you mention
> the source-ing of a Config.in can't be done conditionally or using a
> variable
> name.  Perhaps Kconfig itself should be extended to support that?
>
>
>
>> Regarding to the comment that other files in other directories may need to
>> be customized - it's very easy to put those
>> files in any directory using make variables according to one's project
>> organization preference.
>

Cannot agree with you more!
Looks we all sought to find a way to separate the upstream /open tree
with local/closed tree.
And the main problem as you indicated is the Kconf can't be done
conditionally or using a  variable

I submit another small patch set to support the "Overlay".
It is small, because most of the customization can be done outside the
buildroot.

Regards,
Roy
Avishay Orpaz July 29, 2012, 8:09 p.m. UTC | #9
2012/7/29 Arnout Vandecappelle <arnout@mind.be>

> On 07/29/12 09:02, Avishay Orpaz wrote:
>
>> Let me try to make my point with an example. Let's say I'm designing a
>> linux based digital camera. The software stack
>> will probably include a bunch of standard software component (kernel,
>> busybox, flash utils etc.), but there would also
>> be at least one software component that is unique to the design, and not
>> intended to be shared with other designs - for
>> example, the GUI implementation of that specific model. This is the kind
>> of software I expect to see in the "local"
>> directory. Of course this software component can be put in the "package"
>> directory, but I would think of buildroot as a
>> tool, which should not be modified and can be easily upgraded.
>>
>
>  But putting it in a subdirectory 'local' within the buildroot directory
> still
> has the same problem...
>

That's exactly why I made the local directory name configurable. All the
other stuff is already configurable.


>
>  As for ease of upgrading buildroot when there is a package/<company name>
> directory: Peter Korsgaard (the main buildroot maintainer) does exactly
> that
> at his work, and I don't think he has any issue with it.  You just need one
> additional line at the top of package/Config.in to include
> package/<company name>/Config.in
>
>
>  That said, I would also like the possibility to extend buildroot with
> "local"
> packages, board-specific files, rootfs skeletons, etc.  That gives my
> customers
> the possibility to fully separate the open source stuff from the custom
> stuff.
> (I know I'm changing my opinion here, but only idiots never change their
> mind :-)
> But then, the local directory should be completely outside the buildroot
> directory.
> And I still don't really like the automatic creation of the Config.in file.
> I'm not sure what could be the alternative, though, because as you mention
> the source-ing of a Config.in can't be done conditionally or using a
> variable
> name.  Perhaps Kconfig itself should be extended to support that?
>
>
>
>  Regarding to the comment that other files in other directories may need
>> to be customized - it's very easy to put those
>> files in any directory using make variables according to one's project
>> organization preference.
>>
>
>  Or better yet, define a default project directory layout that sets
> paths like BR2_LINUX_KERNEL_PATCH automatically.
>
>
>
>  Regards,
>  Arnout
>
> --
> Arnout Vandecappelle                               arnout at mind be
> Senior Embedded Software Architect                 +32-16-286540
> Essensium/Mind                                     http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR
> Leuven
> LinkedIn profile: http://www.linkedin.com/in/**arnoutvandecappelle<http://www.linkedin.com/in/arnoutvandecappelle>
>
> GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
>
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 639fdaa..e9342e9 100644
--- a/Makefile
+++ b/Makefile
@@ -192,6 +192,7 @@  $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 
 BUILD_DIR:=$(BASE_DIR)/build
 
+LOCAL_PACKAGES_DIR:=locals
 
 ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
@@ -315,6 +316,9 @@  endif
 
 include package/*/*.mk
 
+# Include local packages
+include $(LOCAL_PACKAGES_DIR)/*/*.mk
+
 include boot/common.mk
 include target/Makefile.in
 include linux/linux.mk
@@ -565,43 +569,51 @@  COMMON_CONFIG_ENV = \
 	KCONFIG_TRISTATE=$(BUILD_DIR)/buildroot-config/tristate.config \
 	BUILDROOT_CONFIG=$(CONFIG_DIR)/.config
 
-xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile
+LOCALS_CONFIG_FILE = .locals.Config.in
+
+$(LOCALS_CONFIG_FILE):
+	rm -f $(@)
+	find $(LOCAL_PACKAGES_DIR)  \
+	-regex '^$(LOCAL_PACKAGES_DIR)/[^/]*/Config.in$$' \
+	-exec cat {} \; >> $@ || true
+
+xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-gconfig: $(BUILD_DIR)/buildroot-config/gconf outputmakefile
+gconfig: $(BUILD_DIR)/buildroot-config/gconf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) srctree=$(TOPDIR) $< $(CONFIG_CONFIG_IN)
 
-menuconfig: $(BUILD_DIR)/buildroot-config/mconf outputmakefile
+menuconfig: $(BUILD_DIR)/buildroot-config/mconf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-nconfig: $(BUILD_DIR)/buildroot-config/nconf outputmakefile
+nconfig: $(BUILD_DIR)/buildroot-config/nconf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-config: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+config: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-oldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+oldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --oldconfig $(CONFIG_CONFIG_IN)
 
-randconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+randconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --randconfig $(CONFIG_CONFIG_IN)
 
-allyesconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allyesconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --allyesconfig $(CONFIG_CONFIG_IN)
 
-allnoconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allnoconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --allnoconfig $(CONFIG_CONFIG_IN)
 
-randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) \
@@ -609,7 +621,7 @@  randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		$< --randconfig $(CONFIG_CONFIG_IN)
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 
-allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) \
@@ -617,7 +629,7 @@  allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		$< --allyesconfig $(CONFIG_CONFIG_IN)
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 
-allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@grep -v BR2_PACKAGE_ $(CONFIG_DIR)/.config > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) \
@@ -625,19 +637,19 @@  allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		$< --allnoconfig $(CONFIG_CONFIG_IN)
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 
-silentoldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+silentoldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	$(COMMON_CONFIG_ENV) $< --silentoldconfig $(CONFIG_CONFIG_IN)
 
-defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(BR2_DEFCONFIG),=$(BR2_DEFCONFIG)) $(CONFIG_CONFIG_IN)
 
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(TOPDIR)/configs/%_defconfig outputmakefile
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(TOPDIR)/configs/%_defconfig outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN)
 
-savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile $(LOCALS_CONFIG_FILE)
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --savedefconfig=$(CONFIG_DIR)/defconfig $(CONFIG_CONFIG_IN)
 
@@ -664,7 +676,7 @@  endif
 clean:
 	rm -rf $(STAGING_DIR) $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
 		$(STAMP_DIR) $(BUILD_DIR) $(TOOLCHAIN_DIR) $(BASE_DIR)/staging \
-		$(LEGAL_INFO_DIR)
+		$(LEGAL_INFO_DIR) $(LOCALS_CONFIG_FILE)
 
 distclean: clean
 ifeq ($(DL_DIR),$(TOPDIR)/dl)
@@ -760,5 +772,5 @@  print-version:
 
 include docs/manual/manual.mk
 
-.PHONY: $(noconfig_targets)
+.PHONY: $(noconfig_targets) $(LOCALS_CONFIG_FILE)
 
diff --git a/package/Config.in b/package/Config.in
index f664b8e..06a779c 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -695,4 +695,8 @@  source "package/vim/Config.in"
 endif
 endmenu
 
+menu "Local Packages"
+source ".locals.Config.in"
+endmenu
+
 endmenu