Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] linux-tools: Add liblockdep
Date: Sun, 11 Nov 2018 22:53:02 +0100	[thread overview]
Message-ID: <20181111225302.61f04f5d@windsurf.home> (raw)
In-Reply-To: <20181111150652.17459-3-alexander.sverdlin@gmail.com>

Hello,

Thanks for this patch, unfortunately, I encountered a number of issues
while testing it, and I have some review comments. See below.

On Sun, 11 Nov 2018 16:06:48 +0100, Alexander Sverdlin wrote:
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
>  package/linux-tools/Config.in                | 13 ++++++++
>  package/linux-tools/linux-tool-liblockdep.mk | 34 ++++++++++++++++++++

This file should be named linux-tool-liblockdep.mk.in, to follow the
pattern of other files in the same folder.

> +config BR2_PACKAGE_LINUX_TOOLS_LIBLOCKDEP

You need a:

	# <execinfo.h>
	depends on BR2_TOOLCHAIN_USES_GLIBC

because the liblockdep source code uses <execinfo.h>, which is only
provided by glibc.

Also, you forgot:

	select BR2_PACKAGE_LINUX_TOOLS

and therefore if only liblockdep is selected in linux-tools, the
linux-tools package is not built at all.

> +	bool "liblockdep"
> +	help
> +	  liblockdep is a tiny wrapper built around kernel/lockdep.c.
> +	  The aim is to provide the same functionality the kernel gets
> +	  from lockdep to userspace.
> +
> +	  The bulk of the code here is the LD_PRELOAD support which
> +	  provides users an easy way to test their code without having
> +	  to integrate liblockdep into said code.
> +
> +	  http://lwn.net/Articles/536363/

And therefore, you need:

comment "liblockdep needs a toolchain w/ glibc"
	depends on !BR2_TOOLCHAIN_USES_GLIBC

> +LIBLOCKDEP_MAKE_FLAGS = \

We are not very consistent between the different .mk.in files, but I
think I'd prefer to see this variable named LIBLOCKDEP_MAKE_OPTS.

> +define LIBLOCKDEP_BUILD_CMDS
> +	$(Q)if test ! -f $(@D)/tools/lib/lockdep/Makefile ; then \

It clearly doesn't work with $(@D) here, you need to use $(LINUX_DIR)
everywhere.

But even with this fixed, I get:

  CC       lockdep.o
In file included from lockdep.c:8:0:
../../../kernel/locking/lockdep.c:49:10: fatal error: linux/jhash.h: No such file or directory
 #include <linux/jhash.h>
          ^~~~~~~~~~~~~~~
compilation terminated.
mv: cannot stat './.lockdep.o.tmp': No such file or directory

When building with Linux 4.9.

Could you have a look at those issues, and send an updated version ?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2018-11-11 21:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-11 15:06 [Buildroot] [PATCH] libtool: Make libltdl a separate package Alexander Sverdlin
2018-11-11 15:06 ` [Buildroot] [PATCH] Add syslinux installer host package Alexander Sverdlin
2018-11-11 20:33   ` Thomas Petazzoni
2018-11-12  0:03     ` Carlos Santos
2018-11-11 15:06 ` [Buildroot] [PATCH] linux-tools: Add liblockdep Alexander Sverdlin
2018-11-11 21:53   ` Thomas Petazzoni [this message]
2018-11-11 15:06 ` [Buildroot] [PATCH] Add mini-snmpd package Alexander Sverdlin
2018-11-13 21:52   ` Thomas Petazzoni
2018-11-22 17:55     ` Alexander Sverdlin
2018-11-11 15:06 ` [Buildroot] [PATCH] acpid: Add systemd service Alexander Sverdlin
2018-11-11 20:46   ` Thomas Petazzoni
2018-11-11 15:06 ` [Buildroot] [PATCH] acpid: Make bundled event files optional Alexander Sverdlin
2018-11-11 20:50   ` Thomas Petazzoni
2018-11-11 23:36     ` Carlos Santos
2018-11-11 15:06 ` [Buildroot] [PATCH] systemd: Explicitly configure split-usr=false and split-bin=true Alexander Sverdlin
2018-11-11 16:34   ` Yann E. MORIN
2018-11-11 20:55   ` Thomas Petazzoni
2018-11-11 23:41     ` Alexander Sverdlin
2018-11-12  8:08     ` Yann E. MORIN
2018-11-13 22:19     ` Alexander Sverdlin
2018-11-11 21:17 ` [Buildroot] [PATCH] libtool: Make libltdl a separate package Thomas Petazzoni
2018-11-11 22:15   ` Arnout Vandecappelle
2018-11-12  8:06     ` Yann E. MORIN
2018-11-22 19:02       ` Alexander Sverdlin
2018-11-23  8:04         ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181111225302.61f04f5d@windsurf.home \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox