| Message ID | 1390468034-7430-1-git-send-email-judge.packham@gmail.com |
|---|---|
| State | Accepted |
| Commit | 340ba1bec04a87eae00b49098070d01e77457819 |
| Headers | show |
>>>>> "Chris" == Chris Packham <judge.packham@gmail.com> writes: > If tcpdump is enabled set ac_cv_path_tcpdump_path so that verbose output > is enabled on the target. > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > Hi, > This should get verbose support working if the tcpdump package is selected. > This may also fix some of the build errors (if they set BR2_PACKAGE_TCPDUMP=y) > but the real fix will come from upstream. > package/tcpreplay/tcpreplay.mk | 6 ++++++ > 1 file changed, 6 insertions(+) > diff --git a/package/tcpreplay/tcpreplay.mk b/package/tcpreplay/tcpreplay.mk > index 0939c6c..a2cd16e 100644 > --- a/package/tcpreplay/tcpreplay.mk > +++ b/package/tcpreplay/tcpreplay.mk > @@ -18,4 +18,10 @@ TCPREPLAY_DEPENDENCIES = libpcap > TCPREPLAY_LIBS = -lpcap $(if $(BR2_PACKAGE_LIBUSB),-lusb-1.0) > TCPREPLAY_CONF_ENV += ac_cv_search_pcap_close='$(TCPREPLAY_LIBS)' > +ifeq ($(BR2_PACKAGE_TCPDUMP),y) > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=/usr/sbin/tcpdump The problem here is that the configure script checks if /usr/bin/tcpdump (on the build machine) is executable, and otherwise errors out - So this breaks if you try to build it on a machine without tcpdump. The configure script needs to be changed to not do this test when cross compiling (or only warn). > +else > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=no > +endif > + > $(eval $(autotools-package)) > -- > 1.8.4.rc2
Peter, Chris, On 23/01/14 11:30, Peter Korsgaard wrote: >>>>>> "Chris" == Chris Packham <judge.packham@gmail.com> writes: > > > If tcpdump is enabled set ac_cv_path_tcpdump_path so that verbose output > > is enabled on the target. > > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > > --- > > Hi, > > > This should get verbose support working if the tcpdump package is selected. > > This may also fix some of the build errors (if they set BR2_PACKAGE_TCPDUMP=y) > > but the real fix will come from upstream. > > > package/tcpreplay/tcpreplay.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > diff --git a/package/tcpreplay/tcpreplay.mk b/package/tcpreplay/tcpreplay.mk > > index 0939c6c..a2cd16e 100644 > > --- a/package/tcpreplay/tcpreplay.mk > > +++ b/package/tcpreplay/tcpreplay.mk > > @@ -18,4 +18,10 @@ TCPREPLAY_DEPENDENCIES = libpcap > > TCPREPLAY_LIBS = -lpcap $(if $(BR2_PACKAGE_LIBUSB),-lusb-1.0) > > TCPREPLAY_CONF_ENV += ac_cv_search_pcap_close='$(TCPREPLAY_LIBS)' > > > +ifeq ($(BR2_PACKAGE_TCPDUMP),y) > > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=/usr/sbin/tcpdump > > The problem here is that the configure script checks if /usr/bin/tcpdump > (on the build machine) is executable, and otherwise errors out - So this > breaks if you try to build it on a machine without tcpdump. I think the patch is actually OK. Looking at the configure script when ac_cv_path_tcpdump_path is set it does not check if the executable exists. It only checks when using the --with-tcpdump option. I tested the patch and choose tcpdump and tcpreplay with no /usr/sbin/tcpdump on the host. The code builds and I see in the configure output the lines checking for tcpdump... (cached) /usr/sbin/tcpdump and tcpdump binary path: /usr/sbin/tcpdump So i think this patch will work. Thanks > > The configure script needs to be changed to not do this test when cross > compiling (or only warn). > > > +else > > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=no > > +endif > > + > > $(eval $(autotools-package)) > > -- > > 1.8.4.rc2 > > >
>>>>> "Martin" == Martin Bark <martin@barkynet.com> writes: Hi, >> > +ifeq ($(BR2_PACKAGE_TCPDUMP),y) >> > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=/usr/sbin/tcpdump >> >> The problem here is that the configure script checks if /usr/bin/tcpdump >> (on the build machine) is executable, and otherwise errors out - So this >> breaks if you try to build it on a machine without tcpdump. > I think the patch is actually OK. Looking at the configure script > when ac_cv_path_tcpdump_path is set it does not check if the > executable exists. It only checks when using the --with-tcpdump > option. > I tested the patch and choose tcpdump and tcpreplay with no > /usr/sbin/tcpdump on the host. The code builds and I see in the > configure output the lines > checking for tcpdump... (cached) /usr/sbin/tcpdump > and > tcpdump binary path: /usr/sbin/tcpdump > So i think this patch will work. Ups, you're right - Sorry. Committed patch, thanks.
On Fri, Jan 24, 2014 at 10:31 AM, Peter Korsgaard <jacmet@uclibc.org> wrote: >>>>>> "Martin" == Martin Bark <martin@barkynet.com> writes: > > Hi, > > >> > +ifeq ($(BR2_PACKAGE_TCPDUMP),y) > >> > +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=/usr/sbin/tcpdump > >> > >> The problem here is that the configure script checks if /usr/bin/tcpdump > >> (on the build machine) is executable, and otherwise errors out - So this > >> breaks if you try to build it on a machine without tcpdump. > > > I think the patch is actually OK. Looking at the configure script > > when ac_cv_path_tcpdump_path is set it does not check if the > > executable exists. It only checks when using the --with-tcpdump > > option. > > > I tested the patch and choose tcpdump and tcpreplay with no > > /usr/sbin/tcpdump on the host. The code builds and I see in the > > configure output the lines > > > checking for tcpdump... (cached) /usr/sbin/tcpdump > > > and > > > tcpdump binary path: /usr/sbin/tcpdump > > > So i think this patch will work. > > Ups, you're right - Sorry. > > Committed patch, thanks. > Thanks. I was about to say the same thing. On the build error front are we concerned about the failures? I haven't heard anything from upstream yet. We probably want to move to tcpreplay 4.0.2 before the buildroot 2014.02 release.
>>>>> "Chris" == Chris Packham <judge.packham@gmail.com> writes: Hi, >> > So i think this patch will work. >> >> Ups, you're right - Sorry. >> >> Committed patch, thanks. >> > Thanks. I was about to say the same thing. > On the build error front are we concerned about the failures? I > haven't heard anything from upstream yet. We probably want to move to > tcpreplay 4.0.2 before the buildroot 2014.02 release. Yes, I got tired of the autobuilder issues, so I've added your patch from the pull request. A patch to bump the version would be good as well, thank.
diff --git a/package/tcpreplay/tcpreplay.mk b/package/tcpreplay/tcpreplay.mk index 0939c6c..a2cd16e 100644 --- a/package/tcpreplay/tcpreplay.mk +++ b/package/tcpreplay/tcpreplay.mk @@ -18,4 +18,10 @@ TCPREPLAY_DEPENDENCIES = libpcap TCPREPLAY_LIBS = -lpcap $(if $(BR2_PACKAGE_LIBUSB),-lusb-1.0) TCPREPLAY_CONF_ENV += ac_cv_search_pcap_close='$(TCPREPLAY_LIBS)' +ifeq ($(BR2_PACKAGE_TCPDUMP),y) +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=/usr/sbin/tcpdump +else +TCPREPLAY_CONF_ENV += ac_cv_path_tcpdump_path=no +endif + $(eval $(autotools-package))
If tcpdump is enabled set ac_cv_path_tcpdump_path so that verbose output is enabled on the target. Signed-off-by: Chris Packham <judge.packham@gmail.com> --- Hi, This should get verbose support working if the tcpdump package is selected. This may also fix some of the build errors (if they set BR2_PACKAGE_TCPDUMP=y) but the real fix will come from upstream. package/tcpreplay/tcpreplay.mk | 6 ++++++ 1 file changed, 6 insertions(+)