[RFC] build: use libpcap only from pkg-config

Message ID 20201008170536.124111-1-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [RFC] build: use libpcap only from pkg-config |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson Oct. 8, 2020, 5:05 p.m. UTC
  All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
pkg-config file for libpcap, and using other methods of finding the library
can cause issues when cross-compiling, so we can limit build support for
pcap versions without a .pc file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/meson.build | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
  

Comments

Bruce Richardson Oct. 8, 2020, 5:08 p.m. UTC | #1
On Thu, Oct 08, 2020 at 06:05:36PM +0100, Bruce Richardson wrote:
> All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
> pkg-config file for libpcap, and using other methods of finding the library
> can cause issues when cross-compiling, so we can limit build support for
> pcap versions without a .pc file.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  config/meson.build | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 69f2aeb60..edc6c195a 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -163,13 +163,7 @@ if libbsd.found()
>  endif
>  
>  # check for pcap
> -pcap_dep = dependency('pcap', required: false)
> -if pcap_dep.found()
> -	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
> -	# an internal pcap-config finder, which is not compatible with
> -	# cross-compilation, so try to fallback to find_library
> -	pcap_dep = cc.find_library('pcap', required: false)
> -endif
> +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
>  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
>  	dpdk_conf.set('RTE_PORT_PCAP', 1)
>  	dpdk_extra_ldflags += '-lpcap'

Just sending this as an RFC for consideration, since I hit problems with
the pcap code when testing 32-bit (x32) builds, and remembered having hit
it previously too.

Does anyone see an issue with limiting our pcap detection to pkg-config
only in this case?

/Bruce
  
Luca Boccassi Oct. 9, 2020, 9:09 a.m. UTC | #2
On Thu, 2020-10-08 at 18:08 +0100, Bruce Richardson wrote:
> On Thu, Oct 08, 2020 at 06:05:36PM +0100, Bruce Richardson wrote:
> > All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
> > pkg-config file for libpcap, and using other methods of finding the library
> > can cause issues when cross-compiling, so we can limit build support for
> > pcap versions without a .pc file.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  config/meson.build | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/config/meson.build b/config/meson.build
> > index 69f2aeb60..edc6c195a 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -163,13 +163,7 @@ if libbsd.found()
> >  endif
> >  
> >  # check for pcap
> > -pcap_dep = dependency('pcap', required: false)
> > -if pcap_dep.found()
> > -	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
> > -	# an internal pcap-config finder, which is not compatible with
> > -	# cross-compilation, so try to fallback to find_library
> > -	pcap_dep = cc.find_library('pcap', required: false)
> > -endif
> > +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> >  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
> >  	dpdk_conf.set('RTE_PORT_PCAP', 1)
> >  	dpdk_extra_ldflags += '-lpcap'
> 
> Just sending this as an RFC for consideration, since I hit problems with
> the pcap code when testing 32-bit (x32) builds, and remembered having hit
> it previously too.
> 
> Does anyone see an issue with limiting our pcap detection to pkg-config
> only in this case?
> 
> /Bruce

Sadly it's not yet available in Debian stable:

https://packages.debian.org/buster/amd64/libpcap0.8-dev/filelist

Also for Ubuntu 18.04, it's only available with the bionic-updates
repository, and only for the past month - it shipped without it.

I'm absolutely keen on using only pkg-config (so much that I did the
packaging changes myself to ship it in libpcap-dev [1][2]) but perhaps
in the interest of compatibility it's better to wait next year, so that
the new Debian stable supports it, and the Ubuntu 18.04 LTS fix had the
chance to rollout everywhere?
  
Bruce Richardson Oct. 9, 2020, 10:32 a.m. UTC | #3
On Fri, Oct 09, 2020 at 10:09:11AM +0100, Luca Boccassi wrote:
> On Thu, 2020-10-08 at 18:08 +0100, Bruce Richardson wrote:
> > On Thu, Oct 08, 2020 at 06:05:36PM +0100, Bruce Richardson wrote:
> > > All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
> > > pkg-config file for libpcap, and using other methods of finding the library
> > > can cause issues when cross-compiling, so we can limit build support for
> > > pcap versions without a .pc file.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  config/meson.build | 8 +-------
> > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > 
> > > diff --git a/config/meson.build b/config/meson.build
> > > index 69f2aeb60..edc6c195a 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -163,13 +163,7 @@ if libbsd.found()
> > >  endif
> > >  
> > >  # check for pcap
> > > -pcap_dep = dependency('pcap', required: false)
> > > -if pcap_dep.found()
> > > -	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
> > > -	# an internal pcap-config finder, which is not compatible with
> > > -	# cross-compilation, so try to fallback to find_library
> > > -	pcap_dep = cc.find_library('pcap', required: false)
> > > -endif
> > > +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> > >  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
> > >  	dpdk_conf.set('RTE_PORT_PCAP', 1)
> > >  	dpdk_extra_ldflags += '-lpcap'
> > 
> > Just sending this as an RFC for consideration, since I hit problems with
> > the pcap code when testing 32-bit (x32) builds, and remembered having hit
> > it previously too.
> > 
> > Does anyone see an issue with limiting our pcap detection to pkg-config
> > only in this case?
> > 
> > /Bruce
> 
> Sadly it's not yet available in Debian stable:
> 
> https://packages.debian.org/buster/amd64/libpcap0.8-dev/filelist
> 
> Also for Ubuntu 18.04, it's only available with the bionic-updates
> repository, and only for the past month - it shipped without it.
> 
> I'm absolutely keen on using only pkg-config (so much that I did the
> packaging changes myself to ship it in libpcap-dev [1][2]) but perhaps
> in the interest of compatibility it's better to wait next year, so that
> the new Debian stable supports it, and the Ubuntu 18.04 LTS fix had the
> chance to rollout everywhere?
> 

Thanks, that's exactly the sort of feedback I was looking for!

It seems to be the pcap-config support in meson which causes cross-building
issues, so I wonder would it still work ok if we changed this to only use
pkg-config for the initial search and then use find-library if that failed?

 # check for pcap
-pcap_dep = dependency('pcap', required: false)
-if pcap_dep.found()
-       # pcap got a pkg-config file only in 1.9.0 and before that meson uses
-       # an internal pcap-config finder, which is not compatible with
-       # cross-compilation, so try to fallback to find_library
+pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
+if not pcap_dep.found()
+       # pcap got a pkg-config file only in 1.9.0
        pcap_dep = cc.find_library('pcap', required: false)
 endif
  
Luca Boccassi Oct. 9, 2020, 12:51 p.m. UTC | #4
On Fri, 2020-10-09 at 11:32 +0100, Bruce Richardson wrote:
> On Fri, Oct 09, 2020 at 10:09:11AM +0100, Luca Boccassi wrote:
> > On Thu, 2020-10-08 at 18:08 +0100, Bruce Richardson wrote:
> > > On Thu, Oct 08, 2020 at 06:05:36PM +0100, Bruce Richardson wrote:
> > > > All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
> > > > pkg-config file for libpcap, and using other methods of finding the library
> > > > can cause issues when cross-compiling, so we can limit build support for
> > > > pcap versions without a .pc file.
> > > > 
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > >  config/meson.build | 8 +-------
> > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > 
> > > > diff --git a/config/meson.build b/config/meson.build
> > > > index 69f2aeb60..edc6c195a 100644
> > > > --- a/config/meson.build
> > > > +++ b/config/meson.build
> > > > @@ -163,13 +163,7 @@ if libbsd.found()
> > > >  endif
> > > >  
> > > >  # check for pcap
> > > > -pcap_dep = dependency('pcap', required: false)
> > > > -if pcap_dep.found()
> > > > -	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
> > > > -	# an internal pcap-config finder, which is not compatible with
> > > > -	# cross-compilation, so try to fallback to find_library
> > > > -	pcap_dep = cc.find_library('pcap', required: false)
> > > > -endif
> > > > +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> > > >  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
> > > >  	dpdk_conf.set('RTE_PORT_PCAP', 1)
> > > >  	dpdk_extra_ldflags += '-lpcap'
> > > 
> > > Just sending this as an RFC for consideration, since I hit problems with
> > > the pcap code when testing 32-bit (x32) builds, and remembered having hit
> > > it previously too.
> > > 
> > > Does anyone see an issue with limiting our pcap detection to pkg-config
> > > only in this case?
> > > 
> > > /Bruce
> > 
> > Sadly it's not yet available in Debian stable:
> > 
> > https://packages.debian.org/buster/amd64/libpcap0.8-dev/filelist
> > 
> > Also for Ubuntu 18.04, it's only available with the bionic-updates
> > repository, and only for the past month - it shipped without it.
> > 
> > I'm absolutely keen on using only pkg-config (so much that I did the
> > packaging changes myself to ship it in libpcap-dev [1][2]) but perhaps
> > in the interest of compatibility it's better to wait next year, so that
> > the new Debian stable supports it, and the Ubuntu 18.04 LTS fix had the
> > chance to rollout everywhere?
> > 
> 
> Thanks, that's exactly the sort of feedback I was looking for!
> 
> It seems to be the pcap-config support in meson which causes cross-building
> issues, so I wonder would it still work ok if we changed this to only use
> pkg-config for the initial search and then use find-library if that failed?
> 
>  # check for pcap
> -pcap_dep = dependency('pcap', required: false)
> -if pcap_dep.found()
> -       # pcap got a pkg-config file only in 1.9.0 and before that meson uses
> -       # an internal pcap-config finder, which is not compatible with
> -       # cross-compilation, so try to fallback to find_library
> +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> +if not pcap_dep.found()
> +       # pcap got a pkg-config file only in 1.9.0
>         pcap_dep = cc.find_library('pcap', required: false)
>  endif

That would be fine - is the 'method' option supported in the minimum
version we support right now?
  
Bruce Richardson Oct. 9, 2020, 1:39 p.m. UTC | #5
On Fri, Oct 09, 2020 at 01:51:35PM +0100, Luca Boccassi wrote:
> On Fri, 2020-10-09 at 11:32 +0100, Bruce Richardson wrote:
> > On Fri, Oct 09, 2020 at 10:09:11AM +0100, Luca Boccassi wrote:
> > > On Thu, 2020-10-08 at 18:08 +0100, Bruce Richardson wrote:
> > > > On Thu, Oct 08, 2020 at 06:05:36PM +0100, Bruce Richardson wrote:
> > > > > All recent linux distro's - including RHEL 8 and Ubuntu 18.04 - provide a
> > > > > pkg-config file for libpcap, and using other methods of finding the library
> > > > > can cause issues when cross-compiling, so we can limit build support for
> > > > > pcap versions without a .pc file.
> > > > > 
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > ---
> > > > >  config/meson.build | 8 +-------
> > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/config/meson.build b/config/meson.build
> > > > > index 69f2aeb60..edc6c195a 100644
> > > > > --- a/config/meson.build
> > > > > +++ b/config/meson.build
> > > > > @@ -163,13 +163,7 @@ if libbsd.found()
> > > > >  endif
> > > > >  
> > > > >  # check for pcap
> > > > > -pcap_dep = dependency('pcap', required: false)
> > > > > -if pcap_dep.found()
> > > > > -	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
> > > > > -	# an internal pcap-config finder, which is not compatible with
> > > > > -	# cross-compilation, so try to fallback to find_library
> > > > > -	pcap_dep = cc.find_library('pcap', required: false)
> > > > > -endif
> > > > > +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> > > > >  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
> > > > >  	dpdk_conf.set('RTE_PORT_PCAP', 1)
> > > > >  	dpdk_extra_ldflags += '-lpcap'
> > > > 
> > > > Just sending this as an RFC for consideration, since I hit problems with
> > > > the pcap code when testing 32-bit (x32) builds, and remembered having hit
> > > > it previously too.
> > > > 
> > > > Does anyone see an issue with limiting our pcap detection to pkg-config
> > > > only in this case?
> > > > 
> > > > /Bruce
> > > 
> > > Sadly it's not yet available in Debian stable:
> > > 
> > > https://packages.debian.org/buster/amd64/libpcap0.8-dev/filelist
> > > 
> > > Also for Ubuntu 18.04, it's only available with the bionic-updates
> > > repository, and only for the past month - it shipped without it.
> > > 
> > > I'm absolutely keen on using only pkg-config (so much that I did the
> > > packaging changes myself to ship it in libpcap-dev [1][2]) but perhaps
> > > in the interest of compatibility it's better to wait next year, so that
> > > the new Debian stable supports it, and the Ubuntu 18.04 LTS fix had the
> > > chance to rollout everywhere?
> > > 
> > 
> > Thanks, that's exactly the sort of feedback I was looking for!
> > 
> > It seems to be the pcap-config support in meson which causes cross-building
> > issues, so I wonder would it still work ok if we changed this to only use
> > pkg-config for the initial search and then use find-library if that failed?
> > 
> >  # check for pcap
> > -pcap_dep = dependency('pcap', required: false)
> > -if pcap_dep.found()
> > -       # pcap got a pkg-config file only in 1.9.0 and before that meson uses
> > -       # an internal pcap-config finder, which is not compatible with
> > -       # cross-compilation, so try to fallback to find_library
> > +pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
> > +if not pcap_dep.found()
> > +       # pcap got a pkg-config file only in 1.9.0
> >         pcap_dep = cc.find_library('pcap', required: false)
> >  endif
> 
> That would be fine - is the 'method' option supported in the minimum
> version we support right now?
> 
I tried building with meson 0.47 and didn't get any errors so it should be
ok, though the reference guide doesn't explicitly say what version it's
supported in. I think even if unsupported in 0.47, the keyword arg would
just be ignored.

I'll do up a non-RFC v2 so.
  

Patch

diff --git a/config/meson.build b/config/meson.build
index 69f2aeb60..edc6c195a 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -163,13 +163,7 @@  if libbsd.found()
 endif
 
 # check for pcap
-pcap_dep = dependency('pcap', required: false)
-if pcap_dep.found()
-	# pcap got a pkg-config file only in 1.9.0 and before that meson uses
-	# an internal pcap-config finder, which is not compatible with
-	# cross-compilation, so try to fallback to find_library
-	pcap_dep = cc.find_library('pcap', required: false)
-endif
+pcap_dep = dependency('libpcap', required: false, method: 'pkg-config')
 if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
 	dpdk_conf.set('RTE_PORT_PCAP', 1)
 	dpdk_extra_ldflags += '-lpcap'