Re: [Buildroot] [PATCH 1/4] package/libtraceevent: new package

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Pierre Floury <pierre.floury@gmail.com>,
	Giulio Benetti <giulio.benetti@benettiengineering.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/4] package/libtraceevent: new package
Date: Sun, 14 May 2023 16:01:19 +0200	[thread overview]
Message-ID: <20230514140119.GF2582048@scaer> (raw)
In-Reply-To: <20230130144655.1193660-2-maxime.chevallier@bootlin.com>

Maxime, All,

On 2023-01-30 15:46 +0100, Maxime Chevallier via buildroot spake thusly:
> Introduce the libtraceevent library, that used to be bundled with the
> trace-cmd tool. This library is now used by several tools and libraries
> such as trace-cmd, libtracefs and rtla.
> 
> This package tries to install libraries in /usr/local/lib by default,
> hence the redefinition of the "prefix" variable, so that the library
> gets installed in "/usr/lib" instead.
> 
> The same applies to the pkgconfig handling of the proved Makefile, which
> installs the .pc files in the /usr/local area by default, hence the
> redefinition of the pkgconfig_dir to either the correct directory within the
> staging area, or to empty value for the target to avoid installing the .pc
> in that case.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  DEVELOPERS                             |  3 +++
>  package/Config.in                      |  1 +
>  package/libtraceevent/Config.in        |  9 +++++++
>  package/libtraceevent/libtraceevent.mk | 33 ++++++++++++++++++++++++++

Missing hash file, see:
    https://buildroot.org/downloads/manual/manual.html#adding-packages-hash

[--SNIP--]
> diff --git a/package/libtraceevent/libtraceevent.mk b/package/libtraceevent/libtraceevent.mk
> new file mode 100644
> index 0000000000..0125023508
> --- /dev/null
> +++ b/package/libtraceevent/libtraceevent.mk
> @@ -0,0 +1,33 @@
> +################################################################################
> +#
> +# libtraceevent
> +#
> +################################################################################
> +
> +LIBTRACEEVENT_VERSION = 1.7.1
> +LIBTRACEEVENT_SITE = https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/snapshot

Are the snapshots generated by git.kernel.org reproducible? I'd prefer
in this case, that we use the git download method.

> +LIBTRACEEVENT_INSTALL_STAGING = YES
> +LIBTRACEEVENT_LICENSE = GPL-2.0, LGPL-2.1
> +LIBTRACEEVENT_LICENSE_FILES = LICENSES/GPL-2.0 LICENSES/LGPL-2.1
> +
> +define LIBTRACEEVENT_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D)

Please, try to keep the ordering of common options the same across the
various _CMDS. For example, in _BUILD_CMDS, you have;
    $(TARGET_CONFIGURE_OPTS) -C $(@D)

while in _INSTALL_*_CMDS, you have:
    -C $(@D) $(TARGET_CONFIGURE_OPTS)

> +endef
> +
> +define LIBTRACEEVENT_INSTALL_STAGING_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
> +	DESTDIR=$(STAGING_DIR) \
> +	pkgconfig_dir="/usr/lib/pkgconfig" \
> +	prefix="/usr" \
> +	install

Addiotnally indent continuation lines, i.e.:

    \t  $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) \
    \t  \t  $(TARGET_CONFIGURE_OPTS) \
    \t  \t  DESTDIR=$(STAGING_DIR) \
    \t  \t  pkgconfig_dir="/usr/lib/pkgconfig" \
    \t  \t  prefix="/usr" \
    \t  \t  install

Why do we need MAKE1 in the install steps, and not in the build step?

Also, don't we need to also pass prefix=/usr at build time? Hmm, the
Makefile gnerates the .pc file at install time, not at build time (this
is not clean, but explains why we don;t need prefix at build time,
maybe?).

Regards,
Yann E. MORIN.

> +endef
> +
> +define LIBTRACEEVENT_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
> +	DESTDIR=$(TARGET_DIR) \
> +	pkgconfig_dir="" \
> +	prefix="/usr" \
> +	install
> +endef
> +
> +$(eval $(generic-package))
> -- 
> 2.39.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot