* [PATCH 1/1] build: update link args and includes for libarchive
@ 2023-10-20 17:01 Srikanth Yalavarthi
2023-10-23 9:26 ` Bruce Richardson
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-20 17:01 UTC (permalink / raw)
To: Bruce Richardson, Aaron Conole, Igor Russkikh, David Marchand
Cc: dev, syalavarthi, sshankarnara, jerinj, stable
In order to avoid linking with all libraries listed as
Libs.private in libarchive.pc, libarchive is not added
to ext_deps during meson setup.
Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.
This patch updates meson build files to add libarchive's
includedir and libdir to compiler flags and would not require
setting c_args and c_link_args externally.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
config/meson.build | 1 +
lib/eal/meson.build | 3 +++
2 files changed, 4 insertions(+)
diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..1bacea74ab 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -239,6 +239,7 @@ if libarchive.found()
# Push libarchive link dependency at the project level to support
# statically linking dpdk apps. Details at:
# https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
+ add_project_link_arguments('-L' + libarchive.get_variable(pkgconfig: 'libdir'), language: 'c')
add_project_link_arguments('-larchive', language: 'c')
dpdk_extra_ldflags += '-larchive'
endif
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..741a5cd088 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ includes += include_directories(libarchive.get_variable(pkgconfig: 'includedir'))
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
@ 2023-10-23 9:26 ` Bruce Richardson
2023-10-23 11:40 ` [EXT] " Srikanth Yalavarthi
2023-10-29 8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 9:26 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, David Marchand, dev, sshankarnara,
jerinj, stable
On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> In order to avoid linking with all libraries listed as
> Libs.private in libarchive.pc, libarchive is not added
> to ext_deps during meson setup.
>
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
>
> This patch updates meson build files to add libarchive's
> includedir and libdir to compiler flags and would not require
> setting c_args and c_link_args externally.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---
Checking back through the mail archives I'm still a little unclear as to
what breaks when we try using libarchive as any other package with a
pkg-config file? I would have thought the best solution was just to add
libarchive as an external dependency, found using pkg-config, to EAL. When
we add it as a dependency, rather than using c/ldflags, we should get all
this path fixup for free?
Can you clarify what breaks when we add libarchive as a libeal dependency
only?
/Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 9:26 ` Bruce Richardson
@ 2023-10-23 11:40 ` Srikanth Yalavarthi
2023-10-23 11:53 ` Bruce Richardson
0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 11:40 UTC (permalink / raw)
To: Bruce Richardson
Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable, Srikanth Yalavarthi
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 14:56
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
>
> External Email
>
> ----------------------------------------------------------------------
> On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> > In order to avoid linking with all libraries listed as Libs.private in
> > libarchive.pc, libarchive is not added to ext_deps during meson setup.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the build
> > failures, user is required to define the 'c_args' and 'c_link_args'
> > with '-I<includedir>' and '-L<libdir>'.
> >
> > This patch updates meson build files to add libarchive's includedir
> > and libdir to compiler flags and would not require setting c_args and
> > c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > ---
>
> Checking back through the mail archives I'm still a little unclear as to what
> breaks when we try using libarchive as any other package with a pkg-config
> file? I would have thought the best solution was just to add libarchive as an
> external dependency, found using pkg-config, to EAL. When we add it as a
> dependency, rather than using c/ldflags, we should get all this path fixup for
> free?
> Can you clarify what breaks when we add libarchive as a libeal dependency
> only?
Below is my observation.
In current implementation, we are looking for libarchive's availability through pkg-config.
When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive' to ldflags.
Since, we are not adding libarchive to ext_deps (to avoid linking with deps.private), the
variables provided by pkg-config like 'includedir' and 'libdir' are ignored by meson and
are not used as part of c_args (-I<includedir>) and c_link_args (-L<libdir>).
In this case, build would fail during cross-compilation and native compilation (when
libarchive is not installed through package managers like yum/apt). And, we need to
manually set c_args and c_link_args as per the build environment to fix the failures.
This patch fixes the issue observed and avoids the requirement to set c_args and c_link_args
manually . Below are the steps to reproduce the issue (cross-compile for aarch64). Tested on
ubuntu:20.04 docker. Same can be reproduced for native build, if libarchive is not installed
through apt-get.
rm -rf /dpdk-buildtest && mkdir -p /dpdk-buildtest
# libarchive
cd /dpdk-buildtest
wget -N https://www.libarchive.org/downloads/libarchive-3.6.1.tar.gz
tar -xzf libarchive-3.6.1.tar.gz
cd libarchive-3.6.1
./configure --prefix /dpdk-buildtest/install-aarch64 --host=aarch64-linux-gnu --without-xml2 --without-lzma --without-zlib
make
make install
# dpdk
cd /dpdk-buildtest
git clone https://dpdk.org/git/dpdk
cd dpdk
git checkout main
export PKG_CONFIG_PATH=/dpdk-buildtest/install-aarch64/lib/pkgconfig
meson setup --prefix /dpdk-buildtest/install-aarch64 --cross-file config/arm/arm64_armv8_linux_gcc build
ninja -C build
### meson log ### libarchive is found by pkg-config
Has header "execinfo.h" : YES
Found pkg-config: /usr/bin/aarch64-linux-gnu-pkg-config (0.29.1)
Run-time dependency libarchive found: YES 3.6.1
Run-time dependency libbsd found: NO (tried pkgconfig)
Run-time dependency jansson found: NO (tried pkgconfig)
### compilation log ### -I<inlcudedir> is not used
[39/2835] Compiling C object 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o'.
FAILED: lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o
aarch64-linux-gnu-gcc -Ilib/76b5a35@@rte_eal@sta -Ilib -I../lib -I. -I../ -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/arm/include -I../lib/eal/arm/include -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/telemetry/../metrics -I../lib/telemetry/../metrics -Ilib/telemetry -I../lib/telemetry -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=c11 -O3 -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=armv8-a+crc -moutline-atomics -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation '-DABI_VERSION="24.0"' -DRTE_LOG_DEFAULT_LOGTYPE=lib.eal -MD -MQ 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o' -MF 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o.d' -o 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o' -c ../lib/eal/unix/eal_firmware.c
../lib/eal/unix/eal_firmware.c:6:10: fatal error: archive.h: No such file or directory
6 | #include <archive.h>
| ^~~~~~~~~~~
compilation terminated.
>
> /Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 11:40 ` [EXT] " Srikanth Yalavarthi
@ 2023-10-23 11:53 ` Bruce Richardson
2023-10-23 12:46 ` Srikanth Yalavarthi
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 11:53 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable
On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 14:56
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > libarchive
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> > > In order to avoid linking with all libraries listed as Libs.private in
> > > libarchive.pc, libarchive is not added to ext_deps during meson setup.
> > >
> > > Since libarchive is not added to ext_deps, cross-compilation or native
> > > compilation with libarchive installed in non-standard location fails
> > > with errors related to "cannot find -larchive"
> > > or "archive.h: No such file or directory". In order to fix the build
> > > failures, user is required to define the 'c_args' and 'c_link_args'
> > > with '-I<includedir>' and '-L<libdir>'.
> > >
> > > This patch updates meson build files to add libarchive's includedir
> > > and libdir to compiler flags and would not require setting c_args and
> > > c_link_args externally.
> > >
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > ---
> >
> > Checking back through the mail archives I'm still a little unclear as to what
> > breaks when we try using libarchive as any other package with a pkg-config
> > file? I would have thought the best solution was just to add libarchive as an
> > external dependency, found using pkg-config, to EAL. When we add it as a
> > dependency, rather than using c/ldflags, we should get all this path fixup for
> > free?
> > Can you clarify what breaks when we add libarchive as a libeal dependency
> > only?
>
> Below is my observation.
>
> In current implementation, we are looking for libarchive's availability through pkg-config.
> When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive' to ldflags.
>
> Since, we are not adding libarchive to ext_deps (to avoid linking with deps.private), the
This is the bit I'm a bit stuck on. What is the issues with adding
libarchive to ext_deps? For other libs, when doing static builds we have to
link with deps.private and it's the correct behaviour AFAIK. Not doing so
would surely lead to problematic builds, no?
/Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 11:53 ` Bruce Richardson
@ 2023-10-23 12:46 ` Srikanth Yalavarthi
2023-10-23 13:00 ` Bruce Richardson
0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 12:46 UTC (permalink / raw)
To: Bruce Richardson, dmitry.kozliuk
Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable, Srikanth Yalavarthi
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 17:24
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
>
> On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: 23 October 2023 14:56
> > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>;
> > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.com>; stable@dpdk.org
> > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > for libarchive
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > wrote:
> > > > In order to avoid linking with all libraries listed as
> > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> meson setup.
> > > >
> > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > native compilation with libarchive installed in non-standard
> > > > location fails with errors related to "cannot find -larchive"
> > > > or "archive.h: No such file or directory". In order to fix the
> > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > with '-I<includedir>' and '-L<libdir>'.
> > > >
> > > > This patch updates meson build files to add libarchive's
> > > > includedir and libdir to compiler flags and would not require
> > > > setting c_args and c_link_args externally.
> > > >
> > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > ---
> > >
> > > Checking back through the mail archives I'm still a little unclear
> > > as to what breaks when we try using libarchive as any other package
> > > with a pkg-config file? I would have thought the best solution was
> > > just to add libarchive as an external dependency, found using
> > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > using c/ldflags, we should get all this path fixup for free?
> > > Can you clarify what breaks when we add libarchive as a libeal
> > > dependency only?
> >
> > Below is my observation.
> >
> > In current implementation, we are looking for libarchive's availability
> through pkg-config.
> > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> to ldflags.
> >
> > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > deps.private), the
>
> This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> ext_deps? For other libs, when doing static builds we have to link with
> deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> lead to problematic builds, no?
I agree on adding to ext_deps as it's the correct behaviour.
However, there was a discussion in mail archives regarding this.
https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
Adding Dmitry Kozlyuk for comments.
>
> /Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 12:46 ` Srikanth Yalavarthi
@ 2023-10-23 13:00 ` Bruce Richardson
2023-10-23 13:06 ` Bruce Richardson
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 13:00 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable
On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 17:24
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > libarchive
> >
> > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: 23 October 2023 14:56
> > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > <irusskikh@marvell.com>; David Marchand
> > <david.marchand@redhat.com>;
> > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > > for libarchive
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > > wrote:
> > > > > In order to avoid linking with all libraries listed as
> > > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> > meson setup.
> > > > >
> > > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > > native compilation with libarchive installed in non-standard
> > > > > location fails with errors related to "cannot find -larchive"
> > > > > or "archive.h: No such file or directory". In order to fix the
> > > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > >
> > > > > This patch updates meson build files to add libarchive's
> > > > > includedir and libdir to compiler flags and would not require
> > > > > setting c_args and c_link_args externally.
> > > > >
> > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > ---
> > > >
> > > > Checking back through the mail archives I'm still a little unclear
> > > > as to what breaks when we try using libarchive as any other package
> > > > with a pkg-config file? I would have thought the best solution was
> > > > just to add libarchive as an external dependency, found using
> > > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > > using c/ldflags, we should get all this path fixup for free?
> > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > dependency only?
> > >
> > > Below is my observation.
> > >
> > > In current implementation, we are looking for libarchive's availability
> > through pkg-config.
> > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> > to ldflags.
> > >
> > > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > > deps.private), the
> >
> > This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> > ext_deps? For other libs, when doing static builds we have to link with
> > deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> > lead to problematic builds, no?
>
> I agree on adding to ext_deps as it's the correct behaviour.
>
> However, there was a discussion in mail archives regarding this.
> https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
>
> Adding Dmitry Kozlyuk for comments.
>
Testing it out myself, the sample applications don't build statically due
to missing dependencies. The libarchive-dev package on Ubuntu, doesn't seem
to install all dependent packages for static builds.
I had to manually install liblz4-dev and libacl1-dev packages, and then
test-meson-builds ran successfully.
Personally, it looks to me like a packaging issue, in that I would expect
the -dev package to install all required dependent dev packages. I also
think using the pkgconfig as a regular dependency is the way we should look
to go, and if necessary, document the list of extra dependencies needed for
libarchive in our docs.
/Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 13:00 ` Bruce Richardson
@ 2023-10-23 13:06 ` Bruce Richardson
2023-10-23 14:01 ` Srikanth Yalavarthi
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 13:06 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable
On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: 23 October 2023 17:24
> > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.com>; stable@dpdk.org
> > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > > libarchive
> > >
> > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > -----Original Message-----
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: 23 October 2023 14:56
> > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > <irusskikh@marvell.com>; David Marchand
> > > <david.marchand@redhat.com>;
> > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > > > for libarchive
> > > > >
> > > > > External Email
> > > > >
> > > > > --------------------------------------------------------------------
> > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > > > wrote:
> > > > > > In order to avoid linking with all libraries listed as
> > > > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> > > meson setup.
> > > > > >
> > > > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > > > native compilation with libarchive installed in non-standard
> > > > > > location fails with errors related to "cannot find -larchive"
> > > > > > or "archive.h: No such file or directory". In order to fix the
> > > > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > >
> > > > > > This patch updates meson build files to add libarchive's
> > > > > > includedir and libdir to compiler flags and would not require
> > > > > > setting c_args and c_link_args externally.
> > > > > >
> > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > Cc: stable@dpdk.org
> > > > > >
> > > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > ---
> > > > >
> > > > > Checking back through the mail archives I'm still a little unclear
> > > > > as to what breaks when we try using libarchive as any other package
> > > > > with a pkg-config file? I would have thought the best solution was
> > > > > just to add libarchive as an external dependency, found using
> > > > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > > > using c/ldflags, we should get all this path fixup for free?
> > > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > > dependency only?
> > > >
> > > > Below is my observation.
> > > >
> > > > In current implementation, we are looking for libarchive's availability
> > > through pkg-config.
> > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> > > to ldflags.
> > > >
> > > > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > > > deps.private), the
> > >
> > > This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> > > ext_deps? For other libs, when doing static builds we have to link with
> > > deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> > > lead to problematic builds, no?
> >
> > I agree on adding to ext_deps as it's the correct behaviour.
> >
> > However, there was a discussion in mail archives regarding this.
> > https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> >
> > Adding Dmitry Kozlyuk for comments.
> >
> Testing it out myself, the sample applications don't build statically due
> to missing dependencies. The libarchive-dev package on Ubuntu, doesn't seem
> to install all dependent packages for static builds.
> I had to manually install liblz4-dev and libacl1-dev packages, and then
> test-meson-builds ran successfully.
>
> Personally, it looks to me like a packaging issue, in that I would expect
> the -dev package to install all required dependent dev packages. I also
> think using the pkgconfig as a regular dependency is the way we should look
> to go, and if necessary, document the list of extra dependencies needed for
> libarchive in our docs.
>
For reference, here is the diff I tested with on Ubuntu 23.04:
diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
libarchive = dependency('libarchive', required: false, method: 'pkg-config')
if libarchive.found()
dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
- # Push libarchive link dependency at the project level to support
- # statically linking dpdk apps. Details at:
- # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
- add_project_link_arguments('-larchive', language: 'c')
- dpdk_extra_ldflags += '-larchive'
endif
# check for libbsd
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ ext_deps += libarchive
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 13:06 ` Bruce Richardson
@ 2023-10-23 14:01 ` Srikanth Yalavarthi
2023-10-26 12:53 ` Srikanth Yalavarthi
0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 14:01 UTC (permalink / raw)
To: Bruce Richardson
Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable, Srikanth Yalavarthi
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 18:36
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> Russkikh <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
>
> On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> > On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: 23 October 2023 17:24
> > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > <irusskikh@marvell.com>; David Marchand
> > > > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar
> Shankar
> > > > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > includes for libarchive
> > > >
> > > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > Sent: 23 October 2023 14:56
> > > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > > <irusskikh@marvell.com>; David Marchand
> > > > <david.marchand@redhat.com>;
> > > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > > includes for libarchive
> > > > > >
> > > > > > External Email
> > > > > >
> > > > > > --------------------------------------------------------------
> > > > > > ------
> > > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth
> > > > > > Yalavarthi
> > > > > > wrote:
> > > > > > > In order to avoid linking with all libraries listed as
> > > > > > > Libs.private in libarchive.pc, libarchive is not added to
> > > > > > > ext_deps during
> > > > meson setup.
> > > > > > >
> > > > > > > Since libarchive is not added to ext_deps, cross-compilation
> > > > > > > or native compilation with libarchive installed in
> > > > > > > non-standard location fails with errors related to "cannot find -
> larchive"
> > > > > > > or "archive.h: No such file or directory". In order to fix
> > > > > > > the build failures, user is required to define the 'c_args' and
> 'c_link_args'
> > > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > > >
> > > > > > > This patch updates meson build files to add libarchive's
> > > > > > > includedir and libdir to compiler flags and would not
> > > > > > > require setting c_args and c_link_args externally.
> > > > > > >
> > > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > > Cc: stable@dpdk.org
> > > > > > >
> > > > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > > ---
> > > > > >
> > > > > > Checking back through the mail archives I'm still a little
> > > > > > unclear as to what breaks when we try using libarchive as any
> > > > > > other package with a pkg-config file? I would have thought the
> > > > > > best solution was just to add libarchive as an external
> > > > > > dependency, found using pkg-config, to EAL. When we add it as
> > > > > > a dependency, rather than using c/ldflags, we should get all this
> path fixup for free?
> > > > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > > > dependency only?
> > > > >
> > > > > Below is my observation.
> > > > >
> > > > > In current implementation, we are looking for libarchive's
> > > > > availability
> > > > through pkg-config.
> > > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-
> larchive'
> > > > to ldflags.
> > > > >
> > > > > Since, we are not adding libarchive to ext_deps (to avoid
> > > > > linking with deps.private), the
> > > >
> > > > This is the bit I'm a bit stuck on. What is the issues with adding
> > > > libarchive to ext_deps? For other libs, when doing static builds
> > > > we have to link with deps.private and it's the correct behaviour
> > > > AFAIK. Not doing so would surely lead to problematic builds, no?
> > >
> > > I agree on adding to ext_deps as it's the correct behaviour.
> > >
> > > However, there was a discussion in mail archives regarding this.
> > > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_
> > > dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPa
> > > z7xtfQ&r=SNPqUkGl0n_Ms1iJa_6wD6LBwX8efL_NOyXvAX-
> iCMI&m=HAX8pzasBDOgY
> > > 4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SPMpIPssi23l&s=wKtxZ-
> fddu7b0b
> > > ADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > >
> > > Adding Dmitry Kozlyuk for comments.
> > >
> > Testing it out myself, the sample applications don't build statically
> > due to missing dependencies. The libarchive-dev package on Ubuntu,
> > doesn't seem to install all dependent packages for static builds.
> > I had to manually install liblz4-dev and libacl1-dev packages, and
> > then test-meson-builds ran successfully.
> >
> > Personally, it looks to me like a packaging issue, in that I would
> > expect the -dev package to install all required dependent dev
> > packages. I also think using the pkgconfig as a regular dependency is
> > the way we should look to go, and if necessary, document the list of
> > extra dependencies needed for libarchive in our docs.
> >
> For reference, here is the diff I tested with on Ubuntu 23.04:
>
> diff --git a/config/meson.build b/config/meson.build index
> d56b0f9bce..4ff101767e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> cc.has_header('execinfo.h') or is_windows) libarchive =
> dependency('libarchive', required: false, method: 'pkg-config') if
> libarchive.found()
> dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> - # Push libarchive link dependency at the project level to support
> - # statically linking dpdk apps. Details at:
> - # https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> s1iJa_6wD6LBwX8efL_NOyXvAX-
> iCMI&m=HAX8pzasBDOgY4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SP
> MpIPssi23l&s=wKtxZ-fddu7b0bADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> - add_project_link_arguments('-larchive', language: 'c')
> - dpdk_extra_ldflags += '-larchive'
> endif
>
> # check for libbsd
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> 9942104386..e1d6c4cf17 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -21,6 +21,9 @@ endif
> if dpdk_conf.has('RTE_USE_LIBBSD')
> ext_deps += libbsd
> endif
> +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> + ext_deps += libarchive
> +endif
> if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> endif
I have tried this approach earlier and it work as expected. But, based on the
discussion in mail archives, I have pushed an alternate one.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
2023-10-23 14:01 ` Srikanth Yalavarthi
@ 2023-10-26 12:53 ` Srikanth Yalavarthi
0 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-26 12:53 UTC (permalink / raw)
To: Bruce Richardson
Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
stable, Srikanth Yalavarthi
> -----Original Message-----
> From: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Sent: 23 October 2023 19:32
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> Russkikh <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org; Srikanth Yalavarthi
> <syalavarthi@marvell.com>; Srikanth Yalavarthi <syalavarthi@marvell.com>
> Subject: RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 18:36
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> > Russkikh <irusskikh@marvell.com>; David Marchand
> > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > includes for libarchive
> >
> > On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> > > On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > > > -----Original Message-----
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: 23 October 2023 17:24
> > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > <irusskikh@marvell.com>; David Marchand
> > > > > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar
> > Shankar
> > > > > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob
> > > > > Kollanukkaran <jerinj@marvell.com>; stable@dpdk.org
> > > > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > includes for libarchive
> > > > >
> > > > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > > Sent: 23 October 2023 14:56
> > > > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > > > <irusskikh@marvell.com>; David Marchand
> > > > > <david.marchand@redhat.com>;
> > > > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > > > includes for libarchive
> > > > > > >
> > > > > > > External Email
> > > > > > >
> > > > > > > ------------------------------------------------------------
> > > > > > > --
> > > > > > > ------
> > > > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth
> > > > > > > Yalavarthi
> > > > > > > wrote:
> > > > > > > > In order to avoid linking with all libraries listed as
> > > > > > > > Libs.private in libarchive.pc, libarchive is not added to
> > > > > > > > ext_deps during
> > > > > meson setup.
> > > > > > > >
> > > > > > > > Since libarchive is not added to ext_deps,
> > > > > > > > cross-compilation or native compilation with libarchive
> > > > > > > > installed in non-standard location fails with errors
> > > > > > > > related to "cannot find -
> > larchive"
> > > > > > > > or "archive.h: No such file or directory". In order to fix
> > > > > > > > the build failures, user is required to define the
> > > > > > > > 'c_args' and
> > 'c_link_args'
> > > > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > > > >
> > > > > > > > This patch updates meson build files to add libarchive's
> > > > > > > > includedir and libdir to compiler flags and would not
> > > > > > > > require setting c_args and c_link_args externally.
> > > > > > > >
> > > > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > > > Cc: stable@dpdk.org
> > > > > > > >
> > > > > > > > Signed-off-by: Srikanth Yalavarthi
> > > > > > > > <syalavarthi@marvell.com>
> > > > > > > > ---
> > > > > > >
> > > > > > > Checking back through the mail archives I'm still a little
> > > > > > > unclear as to what breaks when we try using libarchive as
> > > > > > > any other package with a pkg-config file? I would have
> > > > > > > thought the best solution was just to add libarchive as an
> > > > > > > external dependency, found using pkg-config, to EAL. When we
> > > > > > > add it as a dependency, rather than using c/ldflags, we
> > > > > > > should get all this
> > path fixup for free?
> > > > > > > Can you clarify what breaks when we add libarchive as a
> > > > > > > libeal dependency only?
> > > > > >
> > > > > > Below is my observation.
> > > > > >
> > > > > > In current implementation, we are looking for libarchive's
> > > > > > availability
> > > > > through pkg-config.
> > > > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-
> > larchive'
> > > > > to ldflags.
> > > > > >
> > > > > > Since, we are not adding libarchive to ext_deps (to avoid
> > > > > > linking with deps.private), the
> > > > >
> > > > > This is the bit I'm a bit stuck on. What is the issues with
> > > > > adding libarchive to ext_deps? For other libs, when doing static
> > > > > builds we have to link with deps.private and it's the correct
> > > > > behaviour AFAIK. Not doing so would surely lead to problematic
> builds, no?
> > > >
> > > > I agree on adding to ext_deps as it's the correct behaviour.
> > > >
> > > > However, there was a discussion in mail archives regarding this.
> > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__inbox.dpdk.org_
> > > > dev_20210605004024.660267a1-
> > 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPa
> > > > z7xtfQ&r=SNPqUkGl0n_Ms1iJa_6wD6LBwX8efL_NOyXvAX-
> > iCMI&m=HAX8pzasBDOgY
> > > >
> 4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SPMpIPssi23l&s=wKtxZ-
> > fddu7b0b
> > > > ADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > > >
> > > > Adding Dmitry Kozlyuk for comments.
> > > >
> > > Testing it out myself, the sample applications don't build
> > > statically due to missing dependencies. The libarchive-dev package
> > > on Ubuntu, doesn't seem to install all dependent packages for static
> builds.
> > > I had to manually install liblz4-dev and libacl1-dev packages, and
> > > then test-meson-builds ran successfully.
> > >
> > > Personally, it looks to me like a packaging issue, in that I would
> > > expect the -dev package to install all required dependent dev
> > > packages. I also think using the pkgconfig as a regular dependency
> > > is the way we should look to go, and if necessary, document the list
> > > of extra dependencies needed for libarchive in our docs.
> > >
> > For reference, here is the diff I tested with on Ubuntu 23.04:
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > d56b0f9bce..4ff101767e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> > cc.has_header('execinfo.h') or is_windows) libarchive =
> > dependency('libarchive', required: false, method: 'pkg-config') if
> > libarchive.found()
> > dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> > - # Push libarchive link dependency at the project level to support
> > - # statically linking dpdk apps. Details at:
> > - # https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> >
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> > s1iJa_6wD6LBwX8efL_NOyXvAX-
> >
> iCMI&m=HAX8pzasBDOgY4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SP
> > MpIPssi23l&s=wKtxZ-fddu7b0bADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > - add_project_link_arguments('-larchive', language: 'c')
> > - dpdk_extra_ldflags += '-larchive'
> > endif
> >
> > # check for libbsd
> > diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> > 9942104386..e1d6c4cf17 100644
> > --- a/lib/eal/meson.build
> > +++ b/lib/eal/meson.build
> > @@ -21,6 +21,9 @@ endif
> > if dpdk_conf.has('RTE_USE_LIBBSD')
> > ext_deps += libbsd
> > endif
> > +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> > + ext_deps += libarchive
> > +endif
> > if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> > cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> > endif
>
> I have tried this approach earlier and it work as expected. But, based on the
> discussion in mail archives, I have pushed an alternate one.
Will resubmit the patch with adding libarchive to ext_deps.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/1] build: add libarchive to external deps
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
2023-10-23 9:26 ` Bruce Richardson
@ 2023-10-29 8:10 ` Srikanth Yalavarthi
2023-10-29 8:20 ` [PATCH v3 " Srikanth Yalavarthi
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-29 8:10 UTC (permalink / raw)
To: Bruce Richardson, Srikanth Yalavarthi, Aaron Conole,
Igor Russkikh, David Marchand
Cc: dev, sshankarnara, jerinj, stable
In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.
Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.
This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
config/meson.build | 5 -----
drivers/ml/cnxk/meson.build | 1 +
lib/eal/meson.build | 3 +++
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
libarchive = dependency('libarchive', required: false, method: 'pkg-config')
if libarchive.found()
dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
- # Push libarchive link dependency at the project level to support
- # statically linking dpdk apps. Details at:
- # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
- add_project_link_arguments('-larchive', language: 'c')
- dpdk_extra_ldflags += '-larchive'
endif
# check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
'mvtvm_ml_model.c',
)
+ext_deps += libarchive
ext_deps += jansson_dep
ext_deps += dlpack_dep
ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ ext_deps += libarchive
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/1] build: add libarchive to external deps
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
2023-10-23 9:26 ` Bruce Richardson
2023-10-29 8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
@ 2023-10-29 8:20 ` Srikanth Yalavarthi
2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
2023-11-06 4:12 ` [PATCH v5 " Srikanth Yalavarthi
4 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-29 8:20 UTC (permalink / raw)
To: Bruce Richardson, Srikanth Yalavarthi, David Marchand,
Aaron Conole, Igor Russkikh
Cc: dev, sshankarnara, aprabhu, ptakkar, stable
In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.
Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.
This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
Depends-on: series-30002 ("Implementation of revised ml/cnxk driver")
v3:
- Updated depends on series information
v2:
- Updated patch to include libarchive to ext_deps
v1:
- Initial changes
config/meson.build | 5 -----
drivers/ml/cnxk/meson.build | 1 +
lib/eal/meson.build | 3 +++
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
libarchive = dependency('libarchive', required: false, method: 'pkg-config')
if libarchive.found()
dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
- # Push libarchive link dependency at the project level to support
- # statically linking dpdk apps. Details at:
- # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
- add_project_link_arguments('-larchive', language: 'c')
- dpdk_extra_ldflags += '-larchive'
endif
# check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
'mvtvm_ml_model.c',
)
+ext_deps += libarchive
ext_deps += jansson_dep
ext_deps += dlpack_dep
ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ ext_deps += libarchive
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 1/1] build: add libarchive to external deps
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
` (2 preceding siblings ...)
2023-10-29 8:20 ` [PATCH v3 " Srikanth Yalavarthi
@ 2023-11-03 16:38 ` Srikanth Yalavarthi
2023-11-03 16:50 ` Bruce Richardson
2023-11-06 4:12 ` [PATCH v5 " Srikanth Yalavarthi
4 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-03 16:38 UTC (permalink / raw)
To: Bruce Richardson, Srikanth Yalavarthi, David Marchand,
Aaron Conole, Igor Russkikh
Cc: dev, sshankarnara, aprabhu, ptakkar, stable
In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.
Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.
This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v4:
- Rebase over latest main
v3:
- Add to libarchive ext_deps
v2:
- Update ml/cnxk meson config
v1:
- Initial patch
config/meson.build | 5 -----
drivers/ml/cnxk/meson.build | 1 +
lib/eal/meson.build | 3 +++
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index 0968351740..250833d0a4 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
libarchive = dependency('libarchive', required: false, method: 'pkg-config')
if libarchive.found()
dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
- # Push libarchive link dependency at the project level to support
- # statically linking dpdk apps. Details at:
- # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
- add_project_link_arguments('-larchive', language: 'c')
- dpdk_extra_ldflags += '-larchive'
endif
# check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
'mvtvm_ml_model.c',
)
+ext_deps += libarchive
ext_deps += jansson_dep
ext_deps += dlpack_dep
ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ ext_deps += libarchive
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/1] build: add libarchive to external deps
2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
@ 2023-11-03 16:50 ` Bruce Richardson
2023-11-06 4:27 ` [EXT] " Srikanth Yalavarthi
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-03 16:50 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: David Marchand, Aaron Conole, Igor Russkikh, dev, sshankarnara,
aprabhu, ptakkar, stable
On Fri, Nov 03, 2023 at 09:38:53AM -0700, Srikanth Yalavarthi wrote:
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
>
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
>
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
I think this is the cleanest solution to the problem you were having.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
One minor comment below. Patch is ok without taking it on board if you
like.
> ---
> v4:
> - Rebase over latest main
> v3:
> - Add to libarchive ext_deps
> v2:
> - Update ml/cnxk meson config
> v1:
> - Initial patch
>
> config/meson.build | 5 -----
> drivers/ml/cnxk/meson.build | 1 +
> lib/eal/meson.build | 3 +++
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/config/meson.build b/config/meson.build
> index 0968351740..250833d0a4 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
> libarchive = dependency('libarchive', required: false, method: 'pkg-config')
> if libarchive.found()
> dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> - # Push libarchive link dependency at the project level to support
> - # statically linking dpdk apps. Details at:
> - # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> - add_project_link_arguments('-larchive', language: 'c')
> - dpdk_extra_ldflags += '-larchive'
> endif
>
> # check for libbsd
> diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
> index 0680a0faa5..921dc7e89b 100644
> --- a/drivers/ml/cnxk/meson.build
> +++ b/drivers/ml/cnxk/meson.build
> @@ -67,6 +67,7 @@ sources += files(
> 'mvtvm_ml_model.c',
> )
>
> +ext_deps += libarchive
minor nit - I don't think this is necessary. If libarchive is found, then
DPDK eal will be linked against it, and so all other drivers should simply
have that as a transitive dependency.
Same probably applies to jansson.
> ext_deps += jansson_dep
> ext_deps += dlpack_dep
> ext_deps += dmlc_dep
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build
> index 9942104386..e1d6c4cf17 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -21,6 +21,9 @@ endif
> if dpdk_conf.has('RTE_USE_LIBBSD')
> ext_deps += libbsd
> endif
> +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> + ext_deps += libarchive
> +endif
> if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> endif
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 1/1] build: add libarchive to external deps
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
` (3 preceding siblings ...)
2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
@ 2023-11-06 4:12 ` Srikanth Yalavarthi
2023-11-06 8:53 ` Bruce Richardson
2023-11-06 16:03 ` David Marchand
4 siblings, 2 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-06 4:12 UTC (permalink / raw)
To: Bruce Richardson, Aaron Conole, Igor Russkikh, David Marchand
Cc: dev, syalavarthi, sshankarnara, aprabhu, ptakkar, stable
In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.
Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.
This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v5:
- Updated as per review comments
v4:
- Rebase over latest main
v3:
- Add to libarchive ext_deps
v2:
- Update ml/cnxk meson config
v1:
- Initial patch
config/meson.build | 5 -----
lib/eal/meson.build | 3 +++
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/config/meson.build b/config/meson.build
index 0968351740..250833d0a4 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
libarchive = dependency('libarchive', required: false, method: 'pkg-config')
if libarchive.found()
dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
- # Push libarchive link dependency at the project level to support
- # statically linking dpdk apps. Details at:
- # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
- add_project_link_arguments('-larchive', language: 'c')
- dpdk_extra_ldflags += '-larchive'
endif
# check for libbsd
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
if dpdk_conf.has('RTE_USE_LIBBSD')
ext_deps += libbsd
endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+ ext_deps += libarchive
+endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
--
2.42.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [EXT] Re: [PATCH v4 1/1] build: add libarchive to external deps
2023-11-03 16:50 ` Bruce Richardson
@ 2023-11-06 4:27 ` Srikanth Yalavarthi
0 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-06 4:27 UTC (permalink / raw)
To: Bruce Richardson
Cc: David Marchand, Aaron Conole, Igor Russkikh, dev,
Shivah Shankar Shankar Narayan Rao, Anup Prabhu, Prince Takkar,
stable, Srikanth Yalavarthi
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 03 November 2023 22:21
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: David Marchand <david.marchand@redhat.com>; Aaron Conole
> <aconole@redhat.com>; Igor Russkikh <irusskikh@marvell.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Anup Prabhu <aprabhu@marvell.com>;
> Prince Takkar <ptakkar@marvell.com>; stable@dpdk.org
> Subject: [EXT] Re: [PATCH v4 1/1] build: add libarchive to external deps
>
> External Email
>
> ----------------------------------------------------------------------
> On Fri, Nov 03, 2023 at 09:38:53AM -0700, Srikanth Yalavarthi wrote:
> > In order to avoid linking with Libs.private, libarchive is not added
> > to ext_deps during the meson setup stage.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the build
> > failures, user is required to define the 'c_args' and 'c_link_args'
> > with '-I<includedir>' and '-L<libdir>'.
> >
> > This patch adds libarchive to ext_deps and further would not require
> > setting c_args and c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
>
> I think this is the cleanest solution to the problem you were having.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> One minor comment below. Patch is ok without taking it on board if you like.
>
> > ---
> > v4:
> > - Rebase over latest main
> > v3:
> > - Add to libarchive ext_deps
> > v2:
> > - Update ml/cnxk meson config
> > v1:
> > - Initial patch
> >
> > config/meson.build | 5 -----
> > drivers/ml/cnxk/meson.build | 1 +
> > lib/eal/meson.build | 3 +++
> > 3 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > 0968351740..250833d0a4 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> > cc.has_header('execinfo.h') or is_windows) libarchive =
> > dependency('libarchive', required: false, method: 'pkg-config') if
> libarchive.found()
> > dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> > - # Push libarchive link dependency at the project level to support
> > - # statically linking dpdk apps. Details at:
> > - # https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> s1iJa_6wD6LBwX8efL_NOyXvAX-iCMI&m=6Lg7j9wRNcl4gjqh1r9DHzydY-
> ufK1K63u-
> HpinJ3wSa4B3z1AUzWiaAsg3C5Cwp&s=PFchwx1MTzUrXGTLrJiy1bWDf4M7Y
> cSM8t3BNbGxLUg&e=
> > - add_project_link_arguments('-larchive', language: 'c')
> > - dpdk_extra_ldflags += '-larchive'
> > endif
> >
> > # check for libbsd
> > diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
> > index 0680a0faa5..921dc7e89b 100644
> > --- a/drivers/ml/cnxk/meson.build
> > +++ b/drivers/ml/cnxk/meson.build
> > @@ -67,6 +67,7 @@ sources += files(
> > 'mvtvm_ml_model.c',
> > )
> >
> > +ext_deps += libarchive
>
> minor nit - I don't think this is necessary. If libarchive is found, then DPDK eal
> will be linked against it, and so all other drivers should simply have that as a
> transitive dependency.
Agreed, this would not be necessary for libarchive. Removed in v5.
>
> Same probably applies to jansson.
Jansson is added to ext_deps by lib/metrics. However, 'lib/metrics' is optional and can be disabled through -Ddisable_libs'
I think it would be good to add jansson_dep to ext_deps here to handle the case when 'lib/metrics' is disabled.
Also, I am working on change that would remove Jansson as a dependency for ml/cnxk. We will submit a separate patch for this.
>
> > ext_deps += jansson_dep
> > ext_deps += dlpack_dep
> > ext_deps += dmlc_dep
> > diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> > 9942104386..e1d6c4cf17 100644
> > --- a/lib/eal/meson.build
> > +++ b/lib/eal/meson.build
> > @@ -21,6 +21,9 @@ endif
> > if dpdk_conf.has('RTE_USE_LIBBSD')
> > ext_deps += libbsd
> > endif
> > +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> > + ext_deps += libarchive
> > +endif
> > if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> > cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> > endif
> > --
> > 2.42.0
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 4:12 ` [PATCH v5 " Srikanth Yalavarthi
@ 2023-11-06 8:53 ` Bruce Richardson
2023-11-06 15:24 ` Thomas Monjalon
2023-11-06 16:03 ` David Marchand
1 sibling, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-06 8:53 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, David Marchand, dev, sshankarnara,
aprabhu, ptakkar, stable
On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
>
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
>
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 8:53 ` Bruce Richardson
@ 2023-11-06 15:24 ` Thomas Monjalon
2023-11-06 15:32 ` Thomas Monjalon
0 siblings, 1 reply; 21+ messages in thread
From: Thomas Monjalon @ 2023-11-06 15:24 UTC (permalink / raw)
To: Srikanth Yalavarthi, Bruce Richardson
Cc: dev, Aaron Conole, Igor Russkikh, David Marchand, dev,
sshankarnara, aprabhu, ptakkar, stable
06/11/2023 09:53, Bruce Richardson:
> On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> > In order to avoid linking with Libs.private, libarchive
> > is not added to ext_deps during the meson setup stage.
> >
> > Since libarchive is not added to ext_deps, cross-compilation
> > or native compilation with libarchive installed in non-standard
> > location fails with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the
> > build failures, user is required to define the 'c_args' and
> > 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> >
> > This patch adds libarchive to ext_deps and further would not
> > require setting c_args and c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
I'm not sure to understand which new failure will happen.
Was there a problem solved in new libarchive packages?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 15:24 ` Thomas Monjalon
@ 2023-11-06 15:32 ` Thomas Monjalon
0 siblings, 0 replies; 21+ messages in thread
From: Thomas Monjalon @ 2023-11-06 15:32 UTC (permalink / raw)
To: Srikanth Yalavarthi, Bruce Richardson
Cc: dev, Aaron Conole, Igor Russkikh, David Marchand, dev,
sshankarnara, aprabhu, ptakkar, stable
06/11/2023 16:24, Thomas Monjalon:
> 06/11/2023 09:53, Bruce Richardson:
> > On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> > > In order to avoid linking with Libs.private, libarchive
> > > is not added to ext_deps during the meson setup stage.
> > >
> > > Since libarchive is not added to ext_deps, cross-compilation
> > > or native compilation with libarchive installed in non-standard
> > > location fails with errors related to "cannot find -larchive"
> > > or "archive.h: No such file or directory". In order to fix the
> > > build failures, user is required to define the 'c_args' and
> > > 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> > >
> > > This patch adds libarchive to ext_deps and further would not
> > > require setting c_args and c_link_args externally.
> > >
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> I'm not sure to understand which new failure will happen.
> Was there a problem solved in new libarchive packages?
BTW applied as it fixes an obvious problem.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 4:12 ` [PATCH v5 " Srikanth Yalavarthi
2023-11-06 8:53 ` Bruce Richardson
@ 2023-11-06 16:03 ` David Marchand
2023-11-06 16:23 ` Bruce Richardson
1 sibling, 1 reply; 21+ messages in thread
From: David Marchand @ 2023-11-06 16:03 UTC (permalink / raw)
To: Srikanth Yalavarthi, Bruce Richardson
Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar, stable
On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
<syalavarthi@marvell.com> wrote:
>
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
>
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
>
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
This breaks static compilation of applications.
This can be reproduced with test-meson-builds.sh and in GHA (which was
not linking examples statically, I added a patch in my github repo):
https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572
--
David Marchand
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 16:03 ` David Marchand
@ 2023-11-06 16:23 ` Bruce Richardson
2023-11-06 18:17 ` David Marchand
0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-06 16:23 UTC (permalink / raw)
To: David Marchand
Cc: Srikanth Yalavarthi, Aaron Conole, Igor Russkikh, dev,
sshankarnara, aprabhu, ptakkar, stable
On Mon, Nov 06, 2023 at 05:03:10PM +0100, David Marchand wrote:
> On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
> <syalavarthi@marvell.com> wrote:
> >
> > In order to avoid linking with Libs.private, libarchive is not added to
> > ext_deps during the meson setup stage.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive" or "archive.h: No such
> > file or directory". In order to fix the build failures, user is
> > required to define the 'c_args' and 'c_link_args' with '-I<includedir>'
> > and '-L<libdir>'.
> >
> > This patch adds libarchive to ext_deps and further would not require
> > setting c_args and c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware") Cc:
> > stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
>
> This breaks static compilation of applications. This can be reproduced
> with test-meson-builds.sh and in GHA (which was not linking examples
> statically, I added a patch in my github repo):
> https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572
>
The libarchive-dev Ubuntu package does not install all its needed
dependencies for static linking. The errors can be resolved by manually
installing the 3 missing -dev packages.
It's less than ideal, but to my mind, DPDK is behaving correctly with this
fix - it is marking that it requires libarchive as a dependency. The fact
that the libarchive.pc file lists static libraries that aren't installed is
outside of our control. The previous implementation hacked around this by
just passing -larchive in all cases, rather than using pkg-config
information. This then caused other issues that the patch submitter hit.
/Bruce
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/1] build: add libarchive to external deps
2023-11-06 16:23 ` Bruce Richardson
@ 2023-11-06 18:17 ` David Marchand
0 siblings, 0 replies; 21+ messages in thread
From: David Marchand @ 2023-11-06 18:17 UTC (permalink / raw)
To: Bruce Richardson
Cc: Srikanth Yalavarthi, Aaron Conole, Igor Russkikh, dev,
sshankarnara, aprabhu, ptakkar, stable
On Mon, Nov 6, 2023 at 5:24 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Mon, Nov 06, 2023 at 05:03:10PM +0100, David Marchand wrote:
> > On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
> > <syalavarthi@marvell.com> wrote:
> > >
> > > In order to avoid linking with Libs.private, libarchive is not added to
> > > ext_deps during the meson setup stage.
> > >
> > > Since libarchive is not added to ext_deps, cross-compilation or native
> > > compilation with libarchive installed in non-standard location fails
> > > with errors related to "cannot find -larchive" or "archive.h: No such
> > > file or directory". In order to fix the build failures, user is
> > > required to define the 'c_args' and 'c_link_args' with '-I<includedir>'
> > > and '-L<libdir>'.
> > >
> > > This patch adds libarchive to ext_deps and further would not require
> > > setting c_args and c_link_args externally.
> > >
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware") Cc:
> > > stable@dpdk.org
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> >
> > This breaks static compilation of applications. This can be reproduced
> > with test-meson-builds.sh and in GHA (which was not linking examples
> > statically, I added a patch in my github repo):
> > https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572
> >
> The libarchive-dev Ubuntu package does not install all its needed
> dependencies for static linking. The errors can be resolved by manually
> installing the 3 missing -dev packages.
Same with fedora.
>
> It's less than ideal, but to my mind, DPDK is behaving correctly with this
> fix - it is marking that it requires libarchive as a dependency. The fact
> that the libarchive.pc file lists static libraries that aren't installed is
> outside of our control. The previous implementation hacked around this by
> just passing -larchive in all cases, rather than using pkg-config
> information. This then caused other issues that the patch submitter hit.
Ok, I'll see how to workaround this on my side.
--
David Marchand
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-11-06 18:17 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
2023-10-23 9:26 ` Bruce Richardson
2023-10-23 11:40 ` [EXT] " Srikanth Yalavarthi
2023-10-23 11:53 ` Bruce Richardson
2023-10-23 12:46 ` Srikanth Yalavarthi
2023-10-23 13:00 ` Bruce Richardson
2023-10-23 13:06 ` Bruce Richardson
2023-10-23 14:01 ` Srikanth Yalavarthi
2023-10-26 12:53 ` Srikanth Yalavarthi
2023-10-29 8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
2023-10-29 8:20 ` [PATCH v3 " Srikanth Yalavarthi
2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
2023-11-03 16:50 ` Bruce Richardson
2023-11-06 4:27 ` [EXT] " Srikanth Yalavarthi
2023-11-06 4:12 ` [PATCH v5 " Srikanth Yalavarthi
2023-11-06 8:53 ` Bruce Richardson
2023-11-06 15:24 ` Thomas Monjalon
2023-11-06 15:32 ` Thomas Monjalon
2023-11-06 16:03 ` David Marchand
2023-11-06 16:23 ` Bruce Richardson
2023-11-06 18:17 ` David Marchand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).