[PATCH] dtc: Use better check for libyaml

[PATCH] dtc: Use better check for libyaml

[Marek Behún]

The current check for libyaml based on pkg-config may succeed even if
yaml.h header is missing. Try to determine if the header is also present
by compiling a simple program.

Fixes: 067c650c456e ("dtc: Use pkg-config to locate libyaml")
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Pavel Modilaynen <pavel.modilaynen@axis.com>
Cc: Rob Herring <robh+dt@kernel.org>
---
 scripts/dtc/Makefile | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index ef85f8b7d4a7..75045787f897 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -12,15 +12,24 @@ dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
 
-ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
+_yaml_libs = $(shell pkg-config --libs yaml-0.1 2>/dev/null)
+_cmd_has_yaml =								\
+   { echo "\#include <yaml.h>" ;					\
+     echo "int main(){" ;						\
+     echo "yaml_get_version(NULL,NULL,NULL);" ;				\
+     echo "}" ; } |							\
+   $(HOSTCC) -xc - -o /dev/null $(_yaml_libs) 2>/dev/null && echo yes
+_has_yaml = $(shell $(_cmd_has_yaml))
+
+ifeq ($(_has_yaml),yes)
+dtc-objs	+= yamltree.o
+HOSTLDLIBS_dtc	:= $(_yaml_libs)
+else
 ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
 $(error dtc needs libyaml for DT schema validation support. \
 	Install the necessary libyaml development package.)
 endif
 HOST_EXTRACFLAGS += -DNO_YAML
-else
-dtc-objs	+= yamltree.o
-HOSTLDLIBS_dtc	:= $(shell pkg-config yaml-0.1 --libs)
 endif
 
 # Generated files need one more search path to include headers in source tree
-- 
2.24.1

Re: [PATCH] dtc: Use better check for libyaml

[Masahiro Yamada]

On Fri, May 1, 2020 at 10:53 AM Marek Behún <marek.behun@nic.cz> wrote:
>
> The current check for libyaml based on pkg-config may succeed even if
> yaml.h header is missing. Try to determine if the header is also present
> by compiling a simple program.
>
> Fixes: 067c650c456e ("dtc: Use pkg-config to locate libyaml")
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Pavel Modilaynen <pavel.modilaynen@axis.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> ---


Is it possible to fix the .pc file instead?

This is ugly, and I do not know what is the
point of pkg-config if it cannot detect the pkg correctly.





>  scripts/dtc/Makefile | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
> index ef85f8b7d4a7..75045787f897 100644
> --- a/scripts/dtc/Makefile
> +++ b/scripts/dtc/Makefile
> @@ -12,15 +12,24 @@ dtc-objs    += dtc-lexer.lex.o dtc-parser.tab.o
>  # Source files need to get at the userspace version of libfdt_env.h to compile
>  HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
>
> -ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
> +_yaml_libs = $(shell pkg-config --libs yaml-0.1 2>/dev/null)
> +_cmd_has_yaml =                                                                \
> +   { echo "\#include <yaml.h>" ;                                       \
> +     echo "int main(){" ;                                              \
> +     echo "yaml_get_version(NULL,NULL,NULL);" ;                                \
> +     echo "}" ; } |                                                    \
> +   $(HOSTCC) -xc - -o /dev/null $(_yaml_libs) 2>/dev/null && echo yes
> +_has_yaml = $(shell $(_cmd_has_yaml))
> +
> +ifeq ($(_has_yaml),yes)
> +dtc-objs       += yamltree.o
> +HOSTLDLIBS_dtc := $(_yaml_libs)
> +else
>  ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
>  $(error dtc needs libyaml for DT schema validation support. \
>         Install the necessary libyaml development package.)
>  endif
>  HOST_EXTRACFLAGS += -DNO_YAML
> -else
> -dtc-objs       += yamltree.o
> -HOSTLDLIBS_dtc := $(shell pkg-config yaml-0.1 --libs)
>  endif
>
>  # Generated files need one more search path to include headers in source tree
> --
> 2.24.1
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] dtc: Use better check for libyaml

[Marek Behun]

On Fri, 1 May 2020 14:27:57 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> Is it possible to fix the .pc file instead?
> 
> This is ugly, and I do not know what is the
> point of pkg-config if it cannot detect the pkg correctly.

I know this is ugly, though no more than some code in
scripts/Makefile.build.

What do you mean fixing the pc file?
When the header is not present because libyaml-dev is not installed,
but the library is present, then pkg-config just reports that the
package exists, when asking with --exists, right?

Marek

Re: [PATCH] dtc: Use better check for libyaml

[Masahiro Yamada]

On Sat, May 2, 2020 at 12:08 AM Marek Behun <marek.behun@nic.cz> wrote:
>
> On Fri, 1 May 2020 14:27:57 +0900
> Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> > Is it possible to fix the .pc file instead?
> >
> > This is ugly, and I do not know what is the
> > point of pkg-config if it cannot detect the pkg correctly.
>
> I know this is ugly, though no more than some code in
> scripts/Makefile.build.


You missed to see the cost of parsing the Makefile.


scripts/dtc/Makefile is parsed every time
you run 'make'.

Even if you have nothing to recompile,
it builds the tiny program in background.

This happens for 'make mrproper' too.


>
> What do you mean fixing the pc file?


Maybe, file a bug report to your distribution
if the pc file is strangely installed?


> When the header is not present because libyaml-dev is not installed,
> but the library is present, then pkg-config just reports that the
> package exists, when asking with --exists, right?


So, on your system, yaml-0.1.pc exists even
if libyaml-dev is not installed.
In which situation does this happen?


The concept of pkg-config is to help
to build software that depends on other packages.

Obviously, pkg-config should take care of the headers
because pkg-config supports --cflags option.


It is weird to install the .pc file
when it is not possible to build the software
that depends on it.

In other words, the .pc file should be installed
by the dev package together with the headers,
libraries, and any other components needed to
compile the software that depends on it.


At least, this is true for Ubuntu.


The non-dev package installs only runtime libraries.

The dev package installs the .pc file and
all the components needed to build the program.



$ apt-file list libyaml-0-2
libyaml-0-2: /usr/lib/x86_64-linux-gnu/libyaml-0.so.2
libyaml-0-2: /usr/lib/x86_64-linux-gnu/libyaml-0.so.2.0.5
libyaml-0-2: /usr/share/doc/libyaml-0-2/changelog.Debian.gz
libyaml-0-2: /usr/share/doc/libyaml-0-2/copyright


$ apt-file list libyaml-dev
libyaml-dev: /usr/include/yaml.h
libyaml-dev: /usr/lib/x86_64-linux-gnu/libyaml.a
libyaml-dev: /usr/lib/x86_64-linux-gnu/libyaml.so
libyaml-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/yaml-0.1.pc
libyaml-dev: /usr/share/doc/libyaml-dev/changelog.Debian.gz
libyaml-dev: /usr/share/doc/libyaml-dev/copyright



'make menuconfig' checks the ncurses dev package.

The same for libncurses5 vs libncurses5-dev.


$ apt-file list libncurses5
libncurses5: /lib/x86_64-linux-gnu/libncurses.so.5
libncurses5: /lib/x86_64-linux-gnu/libncurses.so.5.9
libncurses5: /usr/lib/x86_64-linux-gnu/libform.so.5
libncurses5: /usr/lib/x86_64-linux-gnu/libform.so.5.9
libncurses5: /usr/lib/x86_64-linux-gnu/libmenu.so.5
libncurses5: /usr/lib/x86_64-linux-gnu/libmenu.so.5.9
libncurses5: /usr/lib/x86_64-linux-gnu/libpanel.so.5
libncurses5: /usr/lib/x86_64-linux-gnu/libpanel.so.5.9


$ apt-file list libncurses5-dev
libncurses5-dev: /usr/bin/ncurses5-config
libncurses5-dev: /usr/include/curses.h
libncurses5-dev: /usr/include/cursesapp.h
libncurses5-dev: /usr/include/cursesf.h
libncurses5-dev: /usr/include/cursesm.h
libncurses5-dev: /usr/include/cursesp.h
libncurses5-dev: /usr/include/cursesw.h
libncurses5-dev: /usr/include/cursslk.h
libncurses5-dev: /usr/include/eti.h
libncurses5-dev: /usr/include/etip.h
libncurses5-dev: /usr/include/form.h
libncurses5-dev: /usr/include/menu.h
libncurses5-dev: /usr/include/nc_tparm.h
libncurses5-dev: /usr/include/ncurses.h
libncurses5-dev: /usr/include/ncurses_dll.h
libncurses5-dev: /usr/include/panel.h
libncurses5-dev: /usr/include/term.h
libncurses5-dev: /usr/include/term_entry.h
libncurses5-dev: /usr/include/termcap.h
libncurses5-dev: /usr/include/tic.h
libncurses5-dev: /usr/include/unctrl.h
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libcurses.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libcurses.so
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libform.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libform.so
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libmenu.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libmenu.so
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libncurses++.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libncurses.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libncurses.so
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libpanel.a
libncurses5-dev: /usr/lib/x86_64-linux-gnu/libpanel.so
libncurses5-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/form.pc
libncurses5-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/menu.pc
libncurses5-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/ncurses++.pc
libncurses5-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/ncurses.pc
libncurses5-dev: /usr/lib/x86_64-linux-gnu/pkgconfig/panel.pc
libncurses5-dev: /usr/share/doc/libncurses5-dev
libncurses5-dev: /usr/share/man/man1/ncurses5-config.1.gz



>
> Marek
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] dtc: Use better check for libyaml

[Marek Behun]

On Sun, 3 May 2020 02:56:42 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> You missed to see the cost of parsing the Makefile.
> 
> 
> scripts/dtc/Makefile is parsed every time
> you run 'make'.
> 
> Even if you have nothing to recompile,
> it builds the tiny program in background.
> 
> This happens for 'make mrproper' too.

I missed this point by a large margin indeed.

> Maybe, file a bug report to your distribution
> if the pc file is strangely installed?
>
> ...

Very well, thank you for elaborating on that. Sorry this took your time.

Now that I know this, wouldn't it make more sense to decide
whether to build yamltree.c or not in config stage, eg. in a Kconfig
file? Because now it seems that pkg-config is being run everytime we run
make. I understand that it is much cheaper than gcc, but either way...

Marek

Re: [PATCH] dtc: Use better check for libyaml

[Masahiro Yamada]

On Sun, May 3, 2020 at 3:16 AM Marek Behun <marek.behun@nic.cz> wrote:
>
> On Sun, 3 May 2020 02:56:42 +0900
> Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> > You missed to see the cost of parsing the Makefile.
> >
> >
> > scripts/dtc/Makefile is parsed every time
> > you run 'make'.
> >
> > Even if you have nothing to recompile,
> > it builds the tiny program in background.
> >
> > This happens for 'make mrproper' too.
>
> I missed this point by a large margin indeed.
>
> > Maybe, file a bug report to your distribution
> > if the pc file is strangely installed?
> >
> > ...
>
> Very well, thank you for elaborating on that. Sorry this took your time.

No problem.

> Now that I know this, wouldn't it make more sense to decide
> whether to build yamltree.c or not in config stage, eg. in a Kconfig
> file? Because now it seems that pkg-config is being run everytime we run
> make. I understand that it is much cheaper than gcc, but either way...


'make dt_binding_check' does not even require the .config


I do not like to run pkg-config while parsing Makefile.
Another solution is to build two dtc variants,
with/without yaml.

> Marek



-- 
Best Regards
Masahiro Yamada

[PATCH v2] dtc: Use better check for libyaml

[Marek Behún]

The current check for libyaml based on pkg-config may succeed even if
yaml.h header is missing. Try to determine if header and library are
present by compiling a simple program.

This creates macros hostcc-header and hostcc-symbol in
scripts/Kbuild.include, which check whether a header file and a library
symbol are present on host system, by compiling a simple program.

Fixes: 067c650c456e ("dtc: Use pkg-config to locate libyaml")
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Pavel Modilaynen <pavel.modilaynen@axis.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: linux-kbuild@vger.kernel.org
---
 scripts/Kbuild.include | 14 ++++++++++++++
 scripts/dtc/Makefile   | 10 ++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 6cabf20ce66a..33af76aa86c1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -154,6 +154,20 @@ ld-version = $(shell $(LD) --version | $(srctree)/scripts/ld-version.sh)
 # Usage:  $(call ld-ifversion, -ge, 22252, y)
 ld-ifversion = $(shell [ $(ld-version) $(1) $(2) ] && echo $(3) || echo $(4))
 
+# hostcc-header
+# Usage: $(call hostcc-header,header.h,-I/usr/include/libname)
+# third argument is optional
+hostcc-header = $(call try-run,					\
+	{ echo "\#include <$(1)>" ; echo "int main(){}"; } |	\
+	$(HOSTCC) $(2) -x c - -o /dev/null,y,n)
+
+# hostcc-symbol
+# Usage: $(call hostcc-symbol,symbol_name,-llibname)
+# third argument is optional
+hostcc-symbol = $(call try-run,				\
+	echo "void $(1)();int main(){$(1)();}" |	\
+	$(HOSTCC) -x c - -o /dev/null $(2),y,n)
+
 ######
 
 ###
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index ef85f8b7d4a7..d04c03b0899d 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -12,15 +12,17 @@ dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS := -I $(srctree)/$(src)/libfdt
 
-ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)
+_yaml_libs = $(shell pkg-config --libs yaml-0.1 2>/dev/null)
+ifeq ($(call hostcc-header,yaml.h)$(call hostcc-symbol,yaml_get_version,\
+				    $(_yaml_libs)),yy)
+dtc-objs	+= yamltree.o
+HOSTLDLIBS_dtc	:= $(_yaml_libs)
+else
 ifneq ($(CHECK_DT_BINDING)$(CHECK_DTBS),)
 $(error dtc needs libyaml for DT schema validation support. \
 	Install the necessary libyaml development package.)
 endif
 HOST_EXTRACFLAGS += -DNO_YAML
-else
-dtc-objs	+= yamltree.o
-HOSTLDLIBS_dtc	:= $(shell pkg-config yaml-0.1 --libs)
 endif
 
 # Generated files need one more search path to include headers in source tree
-- 
2.24.1