diff mbox

[v2] package/perf: build outside kernel tree

Message ID 1426762651-7734-1-git-send-email-steven@uplinklabs.net
State Rejected
Headers show

Commit Message

Steven Noonan March 19, 2015, 10:57 a.m. UTC
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(-)

Comments

Baruch Siach March 19, 2015, 11:02 a.m. UTC | #1
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
Steven Noonan March 19, 2015, 11:04 a.m. UTC | #2
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?
Baruch Siach March 19, 2015, 11:21 a.m. UTC | #3
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
Thomas Petazzoni March 19, 2015, 12:15 p.m. UTC | #4
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
Steven Noonan March 19, 2015, 12:46 p.m. UTC | #5
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.
Thomas Petazzoni July 11, 2015, 11:11 p.m. UTC | #6
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 mbox

Patch

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