| Message ID | 1425147313-17380-2-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Thomas, On Sat, Feb 28, 2015 at 07:15:07PM +0100, Thomas Petazzoni wrote: > +# We really on the generic package infrastructure to download and s/really/rely/ > +# apply remote patches (downloaded from ftp, http or https). For local > +# patches, we have a custom post-patch hook below. > +LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) baruch
Hello Thomas, [snip] > +define LINUX_APPLY_LOCAL_PATCHES > + for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \ > + if test -d $$p ; then \ > $(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \ [snip] I know, it's not related to your patch but linux patches doesn't follow the new naming convention here. Do we want to keep the "linux-" prefix since linux is not really a package or get rid of it ? It would be convenient to use linux patches generated by git format-patch command like for other packages. Best regards, Romain
Dear Romain Naour, On Sun, 15 Mar 2015 15:59:07 +0100, Romain Naour wrote: > I know, it's not related to your patch but linux patches doesn't follow the new > naming convention here. > > Do we want to keep the "linux-" prefix since linux is not really a package or > get rid of it ? linux is really a package. > It would be convenient to use linux patches generated by git format-patch > command like for other packages. You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR. Regarding whether we should switch from linux-*.patch to *.patch, I don't know. It might break the build for people having various patches in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should break the build for them at some point, and get to a situation where we are consistent. Best regards, Thomas
Hello Thomas, Le 15/03/2015 16:14, Thomas Petazzoni a écrit : > Dear Romain Naour, > > On Sun, 15 Mar 2015 15:59:07 +0100, Romain Naour wrote: > >> I know, it's not related to your patch but linux patches doesn't follow the new >> naming convention here. >> >> Do we want to keep the "linux-" prefix since linux is not really a package or >> get rid of it ? > > linux is really a package. humm... yes (hide) :) > >> It would be convenient to use linux patches generated by git format-patch >> command like for other packages. > > You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR. > > Regarding whether we should switch from linux-*.patch to *.patch, I > don't know. It might break the build for people having various patches > in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should > break the build for them at some point, and get to a situation where we > are consistent. I use BR2_LINUX_KERNEL_PATCH which point to a directory that contains all patches for the kernel build for my target at work. I already made the mistake to forget to rename a new patch with the "linux-" prefix, because I added it like I do for other packages. But sure, it would break the build if we decide to fix this. Best regards, Romain > > Best regards, > > Thomas >
Hi Thomas, Le 28/02/2015 19:15, Thomas Petazzoni a écrit : > The linux package has a special handling of patches, with quite a bit > of legacy in it. A problem caused by this special handling is that the > linux package calls directly the DOWNLOAD_WGET macro, which means that > the package infrastructure isn't aware of which patches get > downloaded, and it prevents doing changes inside the package download > infrastructure. > > This commit changes the handling of patches in the linux package in > the following way: > > * The LINUX_PATCHES variable is kept as is: it lists all the patches > mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This mentionned -> mentioned > option can contain http://, ftp://, https:// URLs, path to local > files or local directories. > > This variable is *not* used by the generic package infrastructure, > so it is purely internal to the Linux package. > > * The LINUX_PATCH variable is now filled in with the list of patches > that should be downloaded. It is derived from LINUX_PATCHES by > filtering the patches that have http://, ftp:// or https:// in > their path. Since <pkg>_PATCH is handled by the package > infrastructure, it means that those patches are now automatically > downloaded and applied by the package infrastructure. > > * The LINUX_APPLY_PATCHES hook is renamed to > LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of > applying local patches: remote patches are handled by > LINUX_PATCH. The implementation of the hook is changed to filter > out the patches that have already taken care of by LINUX_PATCH, so > that we only iterate through the list of local patches or local > patch directories. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- Reviewed-by: Romain Naour <romain.naour@openwide.fr> Partially tested with BR2_LINUX_KERNEL_PATCH which point to a directory that contains all patches for the kernel to be build. > linux/linux.mk | 25 +++++++++---------------- > 1 file changed, 9 insertions(+), 16 deletions(-) > > diff --git a/linux/linux.mk b/linux/linux.mk > index fc90fc5..d591e1e 100644 > --- a/linux/linux.mk > +++ b/linux/linux.mk > @@ -40,6 +40,11 @@ endif > > LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH)) > > +# We really on the generic package infrastructure to download and really -> rely (as noted by Baruch) > +# apply remote patches (downloaded from ftp, http or https). For local > +# patches, we have a custom post-patch hook below. > +LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) > + > LINUX_INSTALL_IMAGES = YES > LINUX_DEPENDENCIES += host-kmod host-lzop > > @@ -144,20 +149,9 @@ else > LINUX_IMAGE_PATH = $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME) > endif # BR2_LINUX_KERNEL_VMLINUX > > -define LINUX_DOWNLOAD_PATCHES > - $(if $(LINUX_PATCHES), > - @$(call MESSAGE,"Download additional patches")) > - $(foreach patch,$(filter ftp://% http://% https://%,$(LINUX_PATCHES)),\ > - $(call DOWNLOAD_WGET,$(patch),$(notdir $(patch)))$(sep)) > -endef > - > -LINUX_POST_DOWNLOAD_HOOKS += LINUX_DOWNLOAD_PATCHES > - > -define LINUX_APPLY_PATCHES > - for p in $(LINUX_PATCHES) ; do \ > - if echo $$p | grep -q -E "^ftp://|^http://|^https://" ; then \ > - $(APPLY_PATCHES) $(@D) $(DL_DIR) `basename $$p` ; \ > - elif test -d $$p ; then \ > +define LINUX_APPLY_LOCAL_PATCHES > + for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \ > + if test -d $$p ; then \ > $(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \ > else \ > $(APPLY_PATCHES) $(@D) `dirname $$p` `basename $$p` ; \ > @@ -165,8 +159,7 @@ define LINUX_APPLY_PATCHES > done > endef > > -LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES > - > +LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES > > ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y) > KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig >
On 15/03/15 16:14, Thomas Petazzoni wrote: >> > It would be convenient to use linux patches generated by git format-patch >> > command like for other packages. > You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR. > > Regarding whether we should switch from linux-*.patch to *.patch, I > don't know. It might break the build for people having various patches > in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should > break the build for them at some point, and get to a situation where we > are consistent. I would say, break the build. But in a separate patch of course :-) Regards, Arnout
On 28/02/15 19:15, Thomas Petazzoni wrote: > The linux package has a special handling of patches, with quite a bit > of legacy in it. A problem caused by this special handling is that the > linux package calls directly the DOWNLOAD_WGET macro, which means that > the package infrastructure isn't aware of which patches get > downloaded, and it prevents doing changes inside the package download > infrastructure. > > This commit changes the handling of patches in the linux package in > the following way: > > * The LINUX_PATCHES variable is kept as is: it lists all the patches > mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This > option can contain http://, ftp://, https:// URLs, path to local > files or local directories. > > This variable is *not* used by the generic package infrastructure, > so it is purely internal to the Linux package. > > * The LINUX_PATCH variable is now filled in with the list of patches > that should be downloaded. It is derived from LINUX_PATCHES by > filtering the patches that have http://, ftp:// or https:// in > their path. Since <pkg>_PATCH is handled by the package > infrastructure, it means that those patches are now automatically > downloaded and applied by the package infrastructure. > > * The LINUX_APPLY_PATCHES hook is renamed to > LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of > applying local patches: remote patches are handled by > LINUX_PATCH. The implementation of the hook is changed to filter > out the patches that have already taken care of by LINUX_PATCH, so > that we only iterate through the list of local patches or local > patch directories. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> It's time for this one to be applied. There are of course the two spelling corrections noted by Romain and Baruch, but I'm sure you can fix those while applying. Regards, Arnout
diff --git a/linux/linux.mk b/linux/linux.mk index fc90fc5..d591e1e 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -40,6 +40,11 @@ endif LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH)) +# We really on the generic package infrastructure to download and +# apply remote patches (downloaded from ftp, http or https). For local +# patches, we have a custom post-patch hook below. +LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) + LINUX_INSTALL_IMAGES = YES LINUX_DEPENDENCIES += host-kmod host-lzop @@ -144,20 +149,9 @@ else LINUX_IMAGE_PATH = $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME) endif # BR2_LINUX_KERNEL_VMLINUX -define LINUX_DOWNLOAD_PATCHES - $(if $(LINUX_PATCHES), - @$(call MESSAGE,"Download additional patches")) - $(foreach patch,$(filter ftp://% http://% https://%,$(LINUX_PATCHES)),\ - $(call DOWNLOAD_WGET,$(patch),$(notdir $(patch)))$(sep)) -endef - -LINUX_POST_DOWNLOAD_HOOKS += LINUX_DOWNLOAD_PATCHES - -define LINUX_APPLY_PATCHES - for p in $(LINUX_PATCHES) ; do \ - if echo $$p | grep -q -E "^ftp://|^http://|^https://" ; then \ - $(APPLY_PATCHES) $(@D) $(DL_DIR) `basename $$p` ; \ - elif test -d $$p ; then \ +define LINUX_APPLY_LOCAL_PATCHES + for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \ + if test -d $$p ; then \ $(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \ else \ $(APPLY_PATCHES) $(@D) `dirname $$p` `basename $$p` ; \ @@ -165,8 +159,7 @@ define LINUX_APPLY_PATCHES done endef -LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES - +LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y) KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
The linux package has a special handling of patches, with quite a bit of legacy in it. A problem caused by this special handling is that the linux package calls directly the DOWNLOAD_WGET macro, which means that the package infrastructure isn't aware of which patches get downloaded, and it prevents doing changes inside the package download infrastructure. This commit changes the handling of patches in the linux package in the following way: * The LINUX_PATCHES variable is kept as is: it lists all the patches mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This option can contain http://, ftp://, https:// URLs, path to local files or local directories. This variable is *not* used by the generic package infrastructure, so it is purely internal to the Linux package. * The LINUX_PATCH variable is now filled in with the list of patches that should be downloaded. It is derived from LINUX_PATCHES by filtering the patches that have http://, ftp:// or https:// in their path. Since <pkg>_PATCH is handled by the package infrastructure, it means that those patches are now automatically downloaded and applied by the package infrastructure. * The LINUX_APPLY_PATCHES hook is renamed to LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of applying local patches: remote patches are handled by LINUX_PATCH. The implementation of the hook is changed to filter out the patches that have already taken care of by LINUX_PATCH, so that we only iterate through the list of local patches or local patch directories. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- linux/linux.mk | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-)