Message ID | 1426762651-7734-1-git-send-email-steven@uplinklabs.net |
---|---|
State | Rejected |
Headers | show |
Hi Steven, On Thu, Mar 19, 2015 at 03:57:31AM -0700, Steven Noonan wrote: > # Source taken from the Linux kernel tree > -PERF_SOURCE = > PERF_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) > +PERF_SOURCE = linux-$(PERF_VERSION).tar.xz > +ifeq ($(findstring x2.6.,x$(PERF_VERSION)),x2.6.) > +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6 > +else > +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x What about v4.x? > +endif baruch
On Thu, Mar 19, 2015 at 4:02 AM, Baruch Siach <baruch@tkos.co.il> wrote: > Hi Steven, > > On Thu, Mar 19, 2015 at 03:57:31AM -0700, Steven Noonan wrote: >> # Source taken from the Linux kernel tree >> -PERF_SOURCE = >> PERF_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) >> +PERF_SOURCE = linux-$(PERF_VERSION).tar.xz >> +ifeq ($(findstring x2.6.,x$(PERF_VERSION)),x2.6.) >> +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6 >> +else >> +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x > > What about v4.x? Good callout. Have there been related changes submitted for the linux-headers and kernel packages?
Hi Steven, On Thu, Mar 19, 2015 at 04:04:38AM -0700, Steven Noonan wrote: > On Thu, Mar 19, 2015 at 4:02 AM, Baruch Siach <baruch@tkos.co.il> wrote: > > On Thu, Mar 19, 2015 at 03:57:31AM -0700, Steven Noonan wrote: > >> # Source taken from the Linux kernel tree > >> -PERF_SOURCE = > >> PERF_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) > >> +PERF_SOURCE = linux-$(PERF_VERSION).tar.xz > >> +ifeq ($(findstring x2.6.,x$(PERF_VERSION)),x2.6.) > >> +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6 > >> +else > >> +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x > > > > What about v4.x? > > Good callout. Have there been related changes submitted for the > linux-headers and kernel packages? In case the maintainers decide this is the way to go about perf (which I doubt), then we should move this code to a common place instead of duplicating it. While at it we should also fix the v4.x case. baruch
Dear Steven Noonan, On Thu, 19 Mar 2015 03:57:31 -0700, Steven Noonan wrote: > This is necessary for introducing patches. > > Signed-off-by: Steven Noonan <steven@uplinklabs.net> Can you describe a little bit more what is happening here, and why we are doing this? In some cases, the current dependency of the perf package on building a Linux kernel is a bit annoying: when I do kernel development, I tend to build my kernel outside of Buildroot. But I still would like to be able to use Buildroot to build tools like perf. So in some sense, what you're proposing here makes some sense. But it is not very efficient to extract twice the entire kernel source code, and there is the issue of code duplication pointed out by Baruch. Thanks, Thomas
On Thu, Mar 19, 2015 at 5:15 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Steven Noonan, > > On Thu, 19 Mar 2015 03:57:31 -0700, Steven Noonan wrote: >> This is necessary for introducing patches. >> >> Signed-off-by: Steven Noonan <steven@uplinklabs.net> > > Can you describe a little bit more what is happening here, and why we > are doing this? > > In some cases, the current dependency of the perf package on building a > Linux kernel is a bit annoying: when I do kernel development, I tend to > build my kernel outside of Buildroot. But I still would like to be able > to use Buildroot to build tools like perf. So in some sense, what > you're proposing here makes some sense. > > But it is not very efficient to extract twice the entire kernel source > code, and there is the issue of code duplication pointed out by Baruch. I agree, it's ugly. But if we intend to apply patches on perf it makes sense that it really has its own tree. For example, in my upcoming x32 patch series: http://git.uplinklabs.net/snoonan/projects/buildroot.git/commit/?h=x32&id=36f8702bebd0466d4ab2bc680f8087dfab1529e0 Is there a better way to handle patching the perf code base, or am I taking the right approach here by having it own its own source tree? If we really care about the efficiency of the extraction we can cut it down to the bits that perf itself really needs (tools directory, some of the top-level files), but that feels kind of weird too.
Dear Steven Noonan, On Thu, 19 Mar 2015 03:57:31 -0700, Steven Noonan wrote: > This is necessary for introducing patches. > > Signed-off-by: Steven Noonan <steven@uplinklabs.net> > --- > > v2: > - Running 'make source' made me realize that the PERF_SOURCE variable needed > to be added, pointing to the same location as e.g. linux-headers. > > package/perf/perf.mk | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/package/perf/perf.mk b/package/perf/perf.mk > index f35abc9..2177ac8 100644 > --- a/package/perf/perf.mk > +++ b/package/perf/perf.mk > @@ -5,10 +5,15 @@ > ################################################################################ > > # Source taken from the Linux kernel tree > -PERF_SOURCE = > PERF_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) > +PERF_SOURCE = linux-$(PERF_VERSION).tar.xz > +ifeq ($(findstring x2.6.,x$(PERF_VERSION)),x2.6.) > +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6 > +else > +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x > +endif Unfortunately, this works only if an upstream kernel tarball is selected. However, that's not really the case in many situations: a lot of people use tarballs from custom locations, or a kernel coming from a Git repository. And for all those cases, your definition of PERF_SITE will not work. The simplest solution is quite probably to apply your perf patches to your kernel tree. You can use BR2_GLOBAL_PATCH_DIR to apply custom patches to your kernel tree. Best regards, Thomas
diff --git a/package/perf/perf.mk b/package/perf/perf.mk index f35abc9..2177ac8 100644 --- a/package/perf/perf.mk +++ b/package/perf/perf.mk @@ -5,10 +5,15 @@ ################################################################################ # Source taken from the Linux kernel tree -PERF_SOURCE = PERF_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) +PERF_SOURCE = linux-$(PERF_VERSION).tar.xz +ifeq ($(findstring x2.6.,x$(PERF_VERSION)),x2.6.) +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6 +else +PERF_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x +endif -PERF_DEPENDENCIES = linux host-flex host-bison +PERF_DEPENDENCIES = host-flex host-bison PERF_MAKE_FLAGS = \ $(LINUX_MAKE_FLAGS) \ @@ -53,29 +58,29 @@ else endif define PERF_BUILD_CMDS - $(Q)if test ! -f $(LINUX_DIR)/tools/perf/Makefile ; then \ + $(Q)if test ! -f $(@D)/tools/perf/Makefile ; then \ echo "Your kernel version is too old and does not have the perf tool." ; \ echo "At least kernel 2.6.31 must be used." ; \ exit 1 ; \ fi $(Q)if test "$(BR2_PACKAGE_ELFUTILS)" = "" ; then \ - if ! grep -q NO_LIBELF $(LINUX_DIR)/tools/perf/Makefile* ; then \ - if ! test -r $(LINUX_DIR)/tools/perf/config/Makefile ; then \ + if ! grep -q NO_LIBELF $(@D)/tools/perf/Makefile* ; then \ + if ! test -r $(@D)/tools/perf/config/Makefile ; then \ echo "The perf tool in your kernel cannot be built without libelf." ; \ echo "Either upgrade your kernel to >= 3.7, or enable the elfutils package." ; \ exit 1 ; \ fi \ fi \ fi - $(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools/perf \ - $(PERF_MAKE_FLAGS) O=$(@D) + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools/perf \ + $(PERF_MAKE_FLAGS) endef # After installation, we remove the Perl and Python scripts from the # target. define PERF_INSTALL_TARGET_CMDS - $(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools/perf \ - $(PERF_MAKE_FLAGS) O=$(@D) install + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools/perf \ + $(PERF_MAKE_FLAGS) install $(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/scripts/ endef
This is necessary for introducing patches. Signed-off-by: Steven Noonan <steven@uplinklabs.net> --- v2: - Running 'make source' made me realize that the PERF_SOURCE variable needed to be added, pointing to the same location as e.g. linux-headers. package/perf/perf.mk | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)