Message ID | 1386784684-6546-1-git-send-email-rjbarnet@rockwellcollins.com |
---|---|
State | Superseded |
Headers | show |
Hi Ryan, On Wed, Dec 11, 2013 at 6:58 PM, Ryan Barnett <rjbarnet@rockwellcollins.com> wrote: > Adding support for specifying multiple directories in > BR2_GLOBAL_PATCH_DIR. This will allow for a layered approach for the > patching of a package. > --- > Config.in | 20 ++++++++++++-------- > package/pkg-generic.mk | 5 ++++- > 2 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/Config.in b/Config.in > index 2b401cb..be9a352 100644 > --- a/Config.in > +++ b/Config.in > @@ -461,18 +461,22 @@ config BR2_PACKAGE_OVERRIDE_FILE > Buildroot documentation for more details on this feature. > > config BR2_GLOBAL_PATCH_DIR > - string "global patch directory" > + string "global patch directories" > help > - You may specify a directory containing global package patches. > - For a specific version <packageversion> of a specific package > - <packagename>, patches are applied as follows. > + You may specify a space seperated list of one or more directories separated > + containing global package patches. For a specific version > + <packageversion> of a specific package <packagename>, patches are > + applied as follows: > > - First, the default Buildroot patch set for the package is applied. > + First, the default Buildroot patch set for the package is applied > + from 'package/<packagename>'. This is not really correct (although the current code isn't correct either, as you noticed) for packages not residing in package/ (like linux or bootloaders). So I'd rather write something like: ... from the package's directory in Buildroot. > > - If the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename>/<packageversion> > - exists, then all *.patch files in the directory will be applied. > + Then for every directory - <global-patch-dir> - that exists in > + BR2_GLOBAL_PATCH_DIR, if the directory > + <global-patch-dir>/<packagename>/<packageversion>/ exists, then all > + *.patch files in this directory will be applied. > > - Otherwise, if the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename> exists, > + Otherwise, if the directory <global-patch-dir>/<packagename> exists, > then all *.patch files in the directory will be applied. > > endmenu > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 1fce71b..53d96c9 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -134,8 +134,11 @@ endif > # The RAWNAME variable is the lowercased package name, which allows to > # find the package directory (typically package/<pkgname>) and the > # prefix of the patches > +# > +# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined > $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION) > -$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME) > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(if $(BR2_GLOBAL_PATCH_DIR),$(foreach pdir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),$(pdir)/$(RAWNAME))) > $(BUILD_DIR)/%/.stamp_patched: > @$(call step_start,patch) > @$(call MESSAGE,"Patching") Otherwise, looks good. Best regards, Thomas
On 11/12/13 18:58, Ryan Barnett wrote: > Adding support for specifying multiple directories in > BR2_GLOBAL_PATCH_DIR. This will allow for a layered approach for the > patching of a package. [snip] > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 1fce71b..53d96c9 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -134,8 +134,11 @@ endif > # The RAWNAME variable is the lowercased package name, which allows to > # find the package directory (typically package/<pkgname>) and the > # prefix of the patches > +# > +# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined > $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION) > -$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME) > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(if $(BR2_GLOBAL_PATCH_DIR),$(foreach pdir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),$(pdir)/$(RAWNAME))) I guess you meant to avoid the "overhead" of the foreach when BR2_GLOBAL_PATCH_DIR is empty? Unfortunately, the condition will always be true because it is "" when empty, and "" is non-empty... So you'd have to qstrip it first. But anyway, the overhead of foreach is nothing, so I think it's both more efficient and easier to read if the condition is removed. Minor detail: I would have used $(addsuffix /$(RAWNAME),$(BR2_GLOBAL_PATCH_DIR)) but I guess that's a matter of taste. Regards, Arnout > $(BUILD_DIR)/%/.stamp_patched: > @$(call step_start,patch) > @$(call MESSAGE,"Patching") >
Arnout, Arnout Vandecappelle <arnout@mind.be> wrote on 12/12/2013 04:11:17 PM: > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > > index 1fce71b..53d96c9 100644 > > --- a/package/pkg-generic.mk > > +++ b/package/pkg-generic.mk > > @@ -134,8 +134,11 @@ endif > > # The RAWNAME variable is the lowercased package name, which allows to > > # find the package directory (typically package/<pkgname>) and the > > # prefix of the patches > > +# > > +# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined > > $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION) > > -$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME) > > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) > > +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(if $(BR2_GLOBAL_PATCH_DIR),$(foreach pdir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),$(pdir)/$(RAWNAME))) > > I guess you meant to avoid the "overhead" of the foreach when > BR2_GLOBAL_PATCH_DIR is empty? Unfortunately, the condition will always > be true because it is "" when empty, and "" is non-empty... So you'd have > to qstrip it first. But anyway, the overhead of foreach is nothing, so I > think it's both more efficient and easier to read if the condition is > removed. I apologies for not putting a version on my patches. I was in a rush to submit a second version and forgot the step of adding a new version title. Lesson learned (since Thomas Petazzoni) I wasn't attempting so save the overhead with this check. Rather, I was trying to prevent that when BR2_GLOBAL_PATCH_DIR is empty, from not generating "/$(RAWNAME)" when BR2_GLOBAL_PATCH_DIR is empty which is happening with current implementation of BR2_GLOBAL_PATCH_DIR. > > Minor detail: I would have used > $(addsuffix /$(RAWNAME),$(BR2_GLOBAL_PATCH_DIR)) > but I guess that's a matter of taste. I do prefer this - thanks for the suggestion and I will update with this in mind. This will also make this much simpler as it I can use addsuffix and it will handle the case where BR2_GLOBAL_PATCH_DIR is empty. I will submit a v3 of this with your suggestions in mind. Thanks, -Ryan
diff --git a/Config.in b/Config.in index 2b401cb..be9a352 100644 --- a/Config.in +++ b/Config.in @@ -461,18 +461,22 @@ config BR2_PACKAGE_OVERRIDE_FILE Buildroot documentation for more details on this feature. config BR2_GLOBAL_PATCH_DIR - string "global patch directory" + string "global patch directories" help - You may specify a directory containing global package patches. - For a specific version <packageversion> of a specific package - <packagename>, patches are applied as follows. + You may specify a space seperated list of one or more directories + containing global package patches. For a specific version + <packageversion> of a specific package <packagename>, patches are + applied as follows: - First, the default Buildroot patch set for the package is applied. + First, the default Buildroot patch set for the package is applied + from 'package/<packagename>'. - If the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename>/<packageversion> - exists, then all *.patch files in the directory will be applied. + Then for every directory - <global-patch-dir> - that exists in + BR2_GLOBAL_PATCH_DIR, if the directory + <global-patch-dir>/<packagename>/<packageversion>/ exists, then all + *.patch files in this directory will be applied. - Otherwise, if the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename> exists, + Otherwise, if the directory <global-patch-dir>/<packagename> exists, then all *.patch files in the directory will be applied. endmenu diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 1fce71b..53d96c9 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -134,8 +134,11 @@ endif # The RAWNAME variable is the lowercased package name, which allows to # find the package directory (typically package/<pkgname>) and the # prefix of the patches +# +# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION) -$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME) +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(if $(BR2_GLOBAL_PATCH_DIR),$(foreach pdir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),$(pdir)/$(RAWNAME))) $(BUILD_DIR)/%/.stamp_patched: @$(call step_start,patch) @$(call MESSAGE,"Patching")