[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Bernd Kuhls]

To reproduce the build error I had to install libpcap0.8-dev on my host
system, then wireshark configure picks up

checking for pcap-config... /usr/bin/pcap-config

and fails.

Fixes
http://autobuild.buildroot.net/results/7e3/7e363d18866057df0db3d0d95fa8d9116728f6ce//
and many others

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/wireshark/wireshark.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk
index 534131b..2b06699 100644
--- a/package/wireshark/wireshark.mk
+++ b/package/wireshark/wireshark.mk
@@ -24,6 +24,7 @@ WIRESHARK_CONF_OPTS = \
 	--enable-static=no \
 	--with-libsmi=no \
 	--with-lua=no \
+	--with-pcap=$(STAGING_DIR)/usr \
 	--includedir=$(STAGING_DIR)/usr/include
 
 # wireshark GUI options
-- 
2.6.2

[Buildroot] [PATCH 2/2] package/wireshark: libpcap is an optional dependency

[Bernd Kuhls]

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/wireshark/Config.in    |  1 -
 package/wireshark/wireshark.mk | 14 ++++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/package/wireshark/Config.in b/package/wireshark/Config.in
index 52b0f52..fec583b 100644
--- a/package/wireshark/Config.in
+++ b/package/wireshark/Config.in
@@ -1,6 +1,5 @@
 config BR2_PACKAGE_WIRESHARK
 	bool "wireshark"
-	select BR2_PACKAGE_LIBPCAP
 	select BR2_PACKAGE_LIBGLIB2
 	depends on BR2_USE_MMU # fork(), glib2
 	depends on BR2_USE_WCHAR # glib2
diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk
index 2b06699..69f7b22 100644
--- a/package/wireshark/wireshark.mk
+++ b/package/wireshark/wireshark.mk
@@ -9,9 +9,7 @@ WIRESHARK_SOURCE = wireshark-$(WIRESHARK_VERSION).tar.bz2
 WIRESHARK_SITE = http://www.wireshark.org/download/src/all-versions
 WIRESHARK_LICENSE = wireshark license
 WIRESHARK_LICENSE_FILES = COPYING
-WIRESHARK_DEPENDENCIES = host-pkgconf libpcap libglib2
-WIRESHARK_CONF_ENV = \
-	ac_cv_path_PCAP_CONFIG=$(STAGING_DIR)/usr/bin/pcap-config
+WIRESHARK_DEPENDENCIES = host-pkgconf libglib2
 
 # patch touching configure.ac
 WIRESHARK_AUTORECONF = YES
@@ -24,7 +22,6 @@ WIRESHARK_CONF_OPTS = \
 	--enable-static=no \
 	--with-libsmi=no \
 	--with-lua=no \
-	--with-pcap=$(STAGING_DIR)/usr \
 	--includedir=$(STAGING_DIR)/usr/include
 
 # wireshark GUI options
@@ -92,4 +89,13 @@ else
 WIRESHARK_CONF_OPTS += --without-libnl
 endif
 
+ifeq ($(BR2_PACKAGE_LIBPCAP),y)
+WIRESHARK_CONF_OPTS += --with-pcap=$(STAGING_DIR)/usr
+WIRESHARK_CONF_ENV += \
+	ac_cv_path_PCAP_CONFIG=$(STAGING_DIR)/usr/bin/pcap-config
+WIRESHARK_DEPENDENCIES += libpcap
+else
+WIRESHARK_CONF_OPTS += --without-pcap
+endif
+
 $(eval $(autotools-package))
-- 
2.6.2

[Buildroot] [PATCH 2/2] package/wireshark: libpcap is an optional dependency

[Gustavo Zacarias]

On 13/12/15 06:16, Bernd Kuhls wrote:

> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>   package/wireshark/Config.in    |  1 -
>   package/wireshark/wireshark.mk | 14 ++++++++++----
>   2 files changed, 10 insertions(+), 5 deletions(-)

Hi.
I'm not so sure this is a good idea, capturing packets in wireshark is 
kind of an expected functionality, and wireshark itself is already big 
compared to libpcap (lots of dissectors and so on) so it's not like 
we're saving anything great here.
Regards.

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Thomas Petazzoni]

Dear Bernd Kuhls,

On Sun, 13 Dec 2015 10:16:24 +0100, Bernd Kuhls wrote:
> To reproduce the build error I had to install libpcap0.8-dev on my host
> system, then wireshark configure picks up
> 
> checking for pcap-config... /usr/bin/pcap-config
> 
> and fails.
> 
> Fixes
> http://autobuild.buildroot.net/results/7e3/7e363d18866057df0db3d0d95fa8d9116728f6ce//
> and many others
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/wireshark/wireshark.mk | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/wireshark: libpcap is an optional dependency

[Thomas Petazzoni]

Dear Bernd Kuhls,

On Sun, 13 Dec 2015 10:16:25 +0100, Bernd Kuhls wrote:
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/wireshark/Config.in    |  1 -
>  package/wireshark/wireshark.mk | 14 ++++++++++----
>  2 files changed, 10 insertions(+), 5 deletions(-)

I agree with Gustavo on this one, wireshark with libpcap is not very
useful, so I don't think it's a good idea to make this an optional
dependency.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Arnout Vandecappelle]

On 13-12-15 10:16, Bernd Kuhls wrote:
> To reproduce the build error I had to install libpcap0.8-dev on my host
> system, then wireshark configure picks up
> 
> checking for pcap-config... /usr/bin/pcap-config

 To fundamentally avoid this type of issue (we have a bunch of fixes like this),
I think we should add an additional directory that is put in the _beginning_ of
the path in TARGET_MAKE_ENV. This directory could then be populated through the
_CONFIG_SCRIPTS mechanism.

 What do the others think?

 It will be a bit of work to revert all these fixes again :-)

 Regards,
 Arnout

> 
> and fails.
> 
> Fixes
> http://autobuild.buildroot.net/results/7e3/7e363d18866057df0db3d0d95fa8d9116728f6ce//
> and many others
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/wireshark/wireshark.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk
> index 534131b..2b06699 100644
> --- a/package/wireshark/wireshark.mk
> +++ b/package/wireshark/wireshark.mk
> @@ -24,6 +24,7 @@ WIRESHARK_CONF_OPTS = \
>  	--enable-static=no \
>  	--with-libsmi=no \
>  	--with-lua=no \
> +	--with-pcap=$(STAGING_DIR)/usr \
>  	--includedir=$(STAGING_DIR)/usr/include
>  
>  # wireshark GUI options
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Yann E. MORIN]

Arnout, All,

On 2015-12-13 21:59 +0100, Arnout Vandecappelle spake thusly:
> On 13-12-15 10:16, Bernd Kuhls wrote:
> > To reproduce the build error I had to install libpcap0.8-dev on my host
> > system, then wireshark configure picks up
> > 
> > checking for pcap-config... /usr/bin/pcap-config
> 
>  To fundamentally avoid this type of issue (we have a bunch of fixes like this),
> I think we should add an additional directory that is put in the _beginning_ of
> the path in TARGET_MAKE_ENV. This directory could then be populated through the
> _CONFIG_SCRIPTS mechanism.
> 
>  What do the others think?

I'v estarted having a look at your suggestion. The basic idea is
relatively trivial to do, and I now have a FOO_CONFIG_DIR location
where I put those scripts.

However, I'm not facing a wall: we use the same PATH for both the target
and the host packages, set in BR_PATH. But of course, we do not want the
host variants to find those foo-config scripts, since they are for the
target; we really want host packages to find the host variants, whether
the ones we install as part of our host packages, or the ones from the
system.

So, we'd need to split the PATH for host and target variants.

I wonder if that would be acceptable.

>  It will be a bit of work to revert all these fixes again :-)

Indeed. I've already tried to hunt down all our "fixes" for this, and I
have identified two cases:

  - packages already accept a FOO_CONFIG environement variable; for
    those we just need to unset it;

  - we patch the package to replace a hard-coded cal to foo-config with
    a construct like ${FOO_CONFIG:-foo-config}, for those, we need to
    alse unpatch the package.

My grep-fuu must still be asleep today, since I could only identify wine
in the second category, although I'm pretty sure we have at least a few
other packages that we do patch for that.

The packages in the first category are reltively rare: only about 50 of
them I could identify, so it should be pretty easy to fix.

I'll work on this a bit mor elater today...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Arnout Vandecappelle]

On 20-12-15 13:49, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2015-12-13 21:59 +0100, Arnout Vandecappelle spake thusly:
>> On 13-12-15 10:16, Bernd Kuhls wrote:
>>> To reproduce the build error I had to install libpcap0.8-dev on my host
>>> system, then wireshark configure picks up
>>>
>>> checking for pcap-config... /usr/bin/pcap-config
>>
>>  To fundamentally avoid this type of issue (we have a bunch of fixes like this),
>> I think we should add an additional directory that is put in the _beginning_ of
>> the path in TARGET_MAKE_ENV. This directory could then be populated through the
>> _CONFIG_SCRIPTS mechanism.
>>
>>  What do the others think?
> 
> I'v estarted having a look at your suggestion. The basic idea is
> relatively trivial to do, and I now have a FOO_CONFIG_DIR location
> where I put those scripts.
> 
> However, I'm not facing a wall:

 I guess you _are_ facing a wall :-)

> we use the same PATH for both the target
> and the host packages, set in BR_PATH. But of course, we do not want the
> host variants to find those foo-config scripts, since they are for the
> target; we really want host packages to find the host variants, whether
> the ones we install as part of our host packages, or the ones from the
> system.
> 
> So, we'd need to split the PATH for host and target variants.
> 
> I wonder if that would be acceptable.

 They used to be, but since they were identical, Samuel factored them away in
b989976.

> 
>>  It will be a bit of work to revert all these fixes again :-)
> 
> Indeed. I've already tried to hunt down all our "fixes" for this, and I
> have identified two cases:
> 
>   - packages already accept a FOO_CONFIG environement variable; for
>     those we just need to unset it;
> 
>   - we patch the package to replace a hard-coded cal to foo-config with
>     a construct like ${FOO_CONFIG:-foo-config}, for those, we need to
>     alse unpatch the package.
> 
> My grep-fuu must still be asleep today, since I could only identify wine
> in the second category, although I'm pretty sure we have at least a few
> other packages that we do patch for that.

git grep \\-config -- \*.patch

 Lot's of false positives though.


> The packages in the first category are reltively rare: only about 50 of
> them I could identify, so it should be pretty easy to fix.

 I was more concerned with the fixes like the one that was added by this patch:
diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk
index 534131b..2b06699 100644
--- a/package/wireshark/wireshark.mk
+++ b/package/wireshark/wireshark.mk
@@ -24,6 +24,7 @@ WIRESHARK_CONF_OPTS = \
 	--enable-static=no \
 	--with-libsmi=no \
 	--with-lua=no \
+	--with-pcap=$(STAGING_DIR)/usr \
 	--includedir=$(STAGING_DIR)/usr/include

 Another grep which turns up lots of false positives:
git grep '\--with-.*=\$(STAGING_DIR)'


 Regards,
 Arnout


> 
> I'll work on this a bit mor elater today...
> 
> Regards,
> Yann E. MORIN.
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Yann E. MORIN]

Arnout, All,

On 2015-12-20 16:49 +0100, Arnout Vandecappelle spake thusly:
> On 20-12-15 13:49, Yann E. MORIN wrote:
> > On 2015-12-13 21:59 +0100, Arnout Vandecappelle spake thusly:
[--SNIP--]
> >>  To fundamentally avoid this type of issue (we have a bunch of fixes like this),
> >> I think we should add an additional directory that is put in the _beginning_ of
> >> the path in TARGET_MAKE_ENV. This directory could then be populated through the
> >> _CONFIG_SCRIPTS mechanism.
> >>
> >>  What do the others think?
> > 
> > I'v estarted having a look at your suggestion. The basic idea is
> > relatively trivial to do, and I now have a FOO_CONFIG_DIR location
> > where I put those scripts.
> > However, I'm not facing a wall:
>  I guess you _are_ facing a wall :-)

s/not/now/ :-)

> > So, we'd need to split the PATH for host and target variants.
> > I wonder if that would be acceptable.
>  They used to be, but since they were identical, Samuel factored them away in
> b989976.

Ah, thianks for the sha1, I was looking for it earlier.

[--SNIP--]
> > My grep-fuu must still be asleep today, since I could only identify wine
> > in the second category, although I'm pretty sure we have at least a few
> > other packages that we do patch for that.
> git grep \\-config -- \*.patch
>  Lot's of false positives though.

I can start with that.

> > The packages in the first category are reltively rare: only about 50 of
> > them I could identify, so it should be pretty easy to fix.
> 
>  I was more concerned with the fixes like the one that was added by this patch:
> diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk
> index 534131b..2b06699 100644
> --- a/package/wireshark/wireshark.mk
> +++ b/package/wireshark/wireshark.mk
> @@ -24,6 +24,7 @@ WIRESHARK_CONF_OPTS = \
>  	--enable-static=no \
>  	--with-libsmi=no \
>  	--with-lua=no \
> +	--with-pcap=$(STAGING_DIR)/usr \
>  	--includedir=$(STAGING_DIR)/usr/include
> 
>  Another grep which turns up lots of false positives:
> git grep '\--with-.*=\$(STAGING_DIR)'

Yep, we can hunt them one by one as time passes...

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/wireshark: Fix libpcap detection

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

> On 13-12-15 10:16, Bernd Kuhls wrote:
>> To reproduce the build error I had to install libpcap0.8-dev on my host
>> system, then wireshark configure picks up
>> 
>> checking for pcap-config... /usr/bin/pcap-config

>  To fundamentally avoid this type of issue (we have a bunch of fixes like this),
> I think we should add an additional directory that is put in the _beginning_ of
> the path in TARGET_MAKE_ENV. This directory could then be populated through the
> _CONFIG_SCRIPTS mechanism.

>  What do the others think?

Sounds good to me!

>  It will be a bit of work to revert all these fixes again :-)

But that can be done iteratively. The point is that buildroot will do
the right thing for these scripts out of the box (as long as they are
listed in the packages providing them) and users of those scripts will
automatically get the right ones.

-- 
Bye, Peter Korsgaard