| Message ID | 1358036972-10406-1-git-send-email-gilles.talis@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Gilles Talis, On Sat, 12 Jan 2013 16:29:32 -0800, Gilles Talis wrote: > Httping is like 'ping' but for http-requests. > > Fixed commit following review Do not put such a comment in the commit. If you want to put a changelog with the differences since the first posting, it should go... > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> > --- ... here. I.e, after the "---" sign. That's because we don't want the changelog to end up forever in the Buildroot commit history. > +if BR2_PACKAGE_HTTPING > + > +config BR2_PACKAGE_HTTPING_OPENSSL > + bool "OpenSSL support" > + depends on BR2_PACKAGE_OPENSSL > + default y > + help > + Adds openSSL support to httping I'd say it should rather be: config BR2_PACKAGE_HTTPING_OPENSSL bool "OpenSSL support" select BR2_PACKAGE_OPENSSL help Adds OpenSSL support to httping When we have sub-options to enable more features, we generally use "select" to make sure that the needed libraries are brought in. > +HTTPING_VERSION = 1.5.6 Any reason not to use 1.5.7. > +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz > +HTTPING_SITE = http://www.vanheusden.com/httping > +HTTPING_LICENSE = GPLv3 > +HTTPING_LICENSE_FILES = license.txt Actually, the license seems to be GPLv2. If you look at this license.txt file, it says: The license of this program can be obtained from: http://www.vanheusden.com/license.txt And if you look at this other license.txt file, it contains the text of GPLv2. > +define HTTPING_BUILD_CMDS > + $(MAKE) CC="$(TARGET_CC)" \ > + LD="$(TARGET_LD)" \ > + STRIP="$(TARGET_STRIP)" \ > + SSL=$(HTTPING_SSL) \ > + DEBUG=no \ > + TFO=$(HTTPING_TFO) -C $(@D) > +endef I saw your e-mail with your issues using TARGET_CONFIGURE_OPTS. But there shouldn't be any issue doing: define HTTPING_BUILD_CMDS $(MAKE) $(TARGET_CONFIGURE_OPTS) \ SSL=$(HTTPING_SSL) \ DEBUG=no \ TFO=$(HTTPING_TFO) -C $(@D) endef Thanks! Thomas
Dear Thomas, Thanks again for the reviews. I'll go and fix the remaining issues. I'll also go and use v1.5.7. I actually just found out today that this version was released. Thanks Gilles On Jan 13, 2013 3:05 AM, "Thomas Petazzoni" < thomas.petazzoni@free-electrons.com> wrote: > Dear Gilles Talis, > > On Sat, 12 Jan 2013 16:29:32 -0800, Gilles Talis wrote: > > Httping is like 'ping' but for http-requests. > > > > Fixed commit following review > > Do not put such a comment in the commit. If you want to put a changelog > with the differences since the first posting, it should go... > > > > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> > > --- > > ... here. I.e, after the "---" sign. > > That's because we don't want the changelog to end up forever in the > Buildroot commit history. > > > +if BR2_PACKAGE_HTTPING > > + > > +config BR2_PACKAGE_HTTPING_OPENSSL > > + bool "OpenSSL support" > > + depends on BR2_PACKAGE_OPENSSL > > + default y > > + help > > + Adds openSSL support to httping > > I'd say it should rather be: > > config BR2_PACKAGE_HTTPING_OPENSSL > bool "OpenSSL support" > select BR2_PACKAGE_OPENSSL > help > Adds OpenSSL support to httping > > When we have sub-options to enable more features, we generally use > "select" to make sure that the needed libraries are brought in. > > > +HTTPING_VERSION = 1.5.6 > > Any reason not to use 1.5.7. > > > +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz > > +HTTPING_SITE = http://www.vanheusden.com/httping > > +HTTPING_LICENSE = GPLv3 > > +HTTPING_LICENSE_FILES = license.txt > > Actually, the license seems to be GPLv2. If you look at this > license.txt file, it says: > > The license of this program can be obtained from: > http://www.vanheusden.com/license.txt > > And if you look at this other license.txt file, it contains the text of > GPLv2. > > > +define HTTPING_BUILD_CMDS > > + $(MAKE) CC="$(TARGET_CC)" \ > > + LD="$(TARGET_LD)" \ > > + STRIP="$(TARGET_STRIP)" \ > > + SSL=$(HTTPING_SSL) \ > > + DEBUG=no \ > > + TFO=$(HTTPING_TFO) -C $(@D) > > +endef > > I saw your e-mail with your issues using TARGET_CONFIGURE_OPTS. But > there shouldn't be any issue doing: > > define HTTPING_BUILD_CMDS > $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > SSL=$(HTTPING_SSL) \ > DEBUG=no \ > TFO=$(HTTPING_TFO) -C $(@D) > endef > > Thanks! > > Thomas > -- > Thomas Petazzoni, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com >
Dear Gilles Talis, On Sun, 13 Jan 2013 12:21:24 -0800, Gilles Talis wrote: > Thanks again for the reviews. I'll go and fix the remaining issues. > I'll also go and use v1.5.7. I actually just found out today that this > version was released. Excellent, thanks for your perseverance with this patch! Best regards, Thomas
I forgot to copy mailing list. Sorry about that and thanks for your help. Gilles. 2013/1/13 Gilles Talis <gilles.talis@gmail.com> > Dear Thomas, > > I thought I knew Makefiles better than that. Well, I was wrong :-). > The reason why TARGET_CONFIGURE_OPTS does not work in this case is because > it overrides the original Makefile's CFLAGS variable (that should be kept > in order for target to compile). Apart from adding "override" in the > package Makefile, I actually do not see how to use TARGET_CONFIGURE_OPTS > and still keep Makefile CFLAGS. Any hint? > This will surely help me for future patches/projects. > > thanks > Gilles. > > 2013/1/13 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > >> Dear Gilles Talis, >> >> On Sat, 12 Jan 2013 16:29:32 -0800, Gilles Talis wrote: >> > Httping is like 'ping' but for http-requests. >> > >> > Fixed commit following review >> >> Do not put such a comment in the commit. If you want to put a changelog >> with the differences since the first posting, it should go... >> >> > >> > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> >> > --- >> >> ... here. I.e, after the "---" sign. >> >> That's because we don't want the changelog to end up forever in the >> Buildroot commit history. >> >> > +if BR2_PACKAGE_HTTPING >> > + >> > +config BR2_PACKAGE_HTTPING_OPENSSL >> > + bool "OpenSSL support" >> > + depends on BR2_PACKAGE_OPENSSL >> > + default y >> > + help >> > + Adds openSSL support to httping >> >> I'd say it should rather be: >> >> config BR2_PACKAGE_HTTPING_OPENSSL >> bool "OpenSSL support" >> select BR2_PACKAGE_OPENSSL >> help >> Adds OpenSSL support to httping >> >> When we have sub-options to enable more features, we generally use >> "select" to make sure that the needed libraries are brought in. >> >> > +HTTPING_VERSION = 1.5.6 >> >> Any reason not to use 1.5.7. >> >> > +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz >> > +HTTPING_SITE = http://www.vanheusden.com/httping >> > +HTTPING_LICENSE = GPLv3 >> > +HTTPING_LICENSE_FILES = license.txt >> >> Actually, the license seems to be GPLv2. If you look at this >> license.txt file, it says: >> >> The license of this program can be obtained from: >> http://www.vanheusden.com/license.txt >> >> And if you look at this other license.txt file, it contains the text of >> GPLv2. >> >> > +define HTTPING_BUILD_CMDS >> > + $(MAKE) CC="$(TARGET_CC)" \ >> > + LD="$(TARGET_LD)" \ >> > + STRIP="$(TARGET_STRIP)" \ >> > + SSL=$(HTTPING_SSL) \ >> > + DEBUG=no \ >> > + TFO=$(HTTPING_TFO) -C $(@D) >> > +endef >> >> I saw your e-mail with your issues using TARGET_CONFIGURE_OPTS. But >> there shouldn't be any issue doing: >> >> define HTTPING_BUILD_CMDS >> $(MAKE) $(TARGET_CONFIGURE_OPTS) \ >> SSL=$(HTTPING_SSL) \ >> DEBUG=no \ >> TFO=$(HTTPING_TFO) -C $(@D) >> endef >> >> Thanks! >> >> Thomas >> -- >> Thomas Petazzoni, Free Electrons >> Kernel, drivers, real-time and embedded Linux >> development, consulting, training and support. >> http://free-electrons.com >> > >
>>>>> "Gilles" == Gilles Talis <gilles.talis@gmail.com> writes:
Gilles> I forgot to copy mailing list.
Gilles> Sorry about that and thanks for your help.
Gilles> Gilles.
Gilles> 2013/1/13 Gilles Talis <gilles.talis@gmail.com>
Gilles> Dear Thomas,
Gilles> I thought I knew Makefiles better than that. Well, I was wrong :-).
Gilles> The reason why TARGET_CONFIGURE_OPTS does not work in this case is because
Gilles> it overrides the original Makefile's CFLAGS variable (that should be kept
Gilles> in order for target to compile). Apart from adding "override" in the
Gilles> package Makefile, I actually do not see how to use TARGET_CONFIGURE_OPTS
Gilles> and still keep Makefile CFLAGS. Any hint?
Gilles> This will surely help me for future patches/projects.
That's indeed how it is normally done.
override CFLAGS += ..
On 14/01/13 14:02, Peter Korsgaard wrote: >>>>>> "Gilles" == Gilles Talis<gilles.talis@gmail.com> writes: > > Gilles> I forgot to copy mailing list. > Gilles> Sorry about that and thanks for your help. > Gilles> Gilles. > > Gilles> 2013/1/13 Gilles Talis<gilles.talis@gmail.com> > > Gilles> Dear Thomas, > > Gilles> I thought I knew Makefiles better than that. Well, I was wrong :-). > Gilles> The reason why TARGET_CONFIGURE_OPTS does not work in this case is because > Gilles> it overrides the original Makefile's CFLAGS variable (that should be kept > Gilles> in order for target to compile). Apart from adding "override" in the > Gilles> package Makefile, I actually do not see how to use TARGET_CONFIGURE_OPTS > Gilles> and still keep Makefile CFLAGS. Any hint? > Gilles> This will surely help me for future patches/projects. > > That's indeed how it is normally done. > > override CFLAGS += .. No. Normally CFLAGS should be assigned to either as CFLAGS += or CFLAGS ?= and the user-settable CFLAGS are passed in the environment. I think most Makefiles follow that convention. Patch follows. Regards, Arnout
diff --git a/package/Config.in b/package/Config.in index fcc2480..7c96095 100644 --- a/package/Config.in +++ b/package/Config.in @@ -618,6 +618,7 @@ source "package/ethtool/Config.in" source "package/heirloom-mailx/Config.in" source "package/hiawatha/Config.in" source "package/hostapd/Config.in" +source "package/httping/Config.in" if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/ifplugd/Config.in" endif diff --git a/package/httping/Config.in b/package/httping/Config.in new file mode 100644 index 0000000..46e21f5 --- /dev/null +++ b/package/httping/Config.in @@ -0,0 +1,24 @@ +config BR2_PACKAGE_HTTPING + bool "httping" + help + Httping is like 'ping' but for http-requests. + Give it an url, and it'll show you how long it takes to connect, + send a request and retrieve the reply (only the headers). + Be aware that the transmission across the network also takes time! + So it measures the latency of the webserver + network. + + http://www.vanheusden.com/httping/ + +if BR2_PACKAGE_HTTPING + +config BR2_PACKAGE_HTTPING_OPENSSL + bool "OpenSSL support" + depends on BR2_PACKAGE_OPENSSL + default y + help + Adds openSSL support to httping + +config BR2_PACKAGE_HTTPING_TFO + bool "TCP Fast Open (TFO) support" + +endif diff --git a/package/httping/httping.mk b/package/httping/httping.mk new file mode 100644 index 0000000..6783b79 --- /dev/null +++ b/package/httping/httping.mk @@ -0,0 +1,39 @@ +############################################################# +# +# httping +# +############################################################# +HTTPING_VERSION = 1.5.6 +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz +HTTPING_SITE = http://www.vanheusden.com/httping +HTTPING_LICENSE = GPLv3 +HTTPING_LICENSE_FILES = license.txt + +ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y) + HTTPING_DEPENDENCIES = openssl +else + HTTPING_SSL = no +endif + +ifeq ($(BR2_PACKAGE_HTTPING_TFO),y) + HTTPING_TFO = yes +endif + +define HTTPING_BUILD_CMDS + $(MAKE) CC="$(TARGET_CC)" \ + LD="$(TARGET_LD)" \ + STRIP="$(TARGET_STRIP)" \ + SSL=$(HTTPING_SSL) \ + DEBUG=no \ + TFO=$(HTTPING_TFO) -C $(@D) +endef + +define HTTPING_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin/httping +endef + +define HTTPING_CLEAN_CMDS + $(MAKE) -C $(@D) clean +endef + +$(eval $(generic-package))
Httping is like 'ping' but for http-requests. Fixed commit following review Signed-off-by: Gilles Talis <gilles.talis@gmail.com> --- package/Config.in | 1 + package/httping/Config.in | 24 ++++++++++++++++++++++++ package/httping/httping.mk | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 0 deletions(-) create mode 100644 package/httping/Config.in create mode 100644 package/httping/httping.mk