Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] httping: new package
@ 2013-01-12  0:00 gilles.talis at gmail.com
  2013-01-12 11:07 ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: gilles.talis at gmail.com @ 2013-01-12  0:00 UTC (permalink / raw)
  To: buildroot

From: Gilles Talis <gilles.talis@gmail.com>

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/

Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
---
 package/Config.in          |    1 +
 package/httping/Config.in  |   24 ++++++++++++++++++++++++
 package/httping/httping.mk |   44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 0 deletions(-)
 create mode 100644 package/httping/Config.in
 create mode 100644 package/httping/httping.mk

diff --git a/package/Config.in b/package/Config.in
index bd1db6b..2ce7de8 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -617,6 +617,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..578028f
--- /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"
+	default y
+	select BR2_PACKAGE_OPENSSL
+
+config BR2_PACKAGE_HTTPING_TFO
+	bool "TCP Fast Open (TFO) support"
+	default n
+
+endif
+
diff --git a/package/httping/httping.mk b/package/httping/httping.mk
new file mode 100644
index 0000000..7076440
--- /dev/null
+++ b/package/httping/httping.mk
@@ -0,0 +1,44 @@
+#############################################################
+#
+# httping
+#
+#############################################################
+HTTPING_VERSION = 1.5.6
+HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
+HTTPING_SITE = http://www.vanheusden.com/httping
+HTTPING_LICENSE = GPL
+HTTPING_LICENSE_FILES = license.txt
+
+ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
+HTTPING_DEPENDENCIES = openssl
+endif
+
+
+ifneq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
+     HTTPING_SSL = no
+endif
+
+ifeq ($(BR2_PACKAGE_HTTPING_TFO),y)
+     HTTPING_TFO = yes 
+endif
+
+define HTTPING_BUILD_CMDS
+     $(MAKE) CC="$(TARGET_CC)" \
+     SSL=$(HTTPING_SSL) \
+     DEBUG=no \
+     TFO=$(HTTPING_TFO) \
+     LD="$(TARGET_LD)" \
+     STRIP="$(TARGET_STRIP)" -C $(@D)
+endef
+
+define HTTPING_INSTALL_TARGET_CMDS
+     $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin
+endef
+
+define HTTPING_CLEAN_CMDS
+     $(MAKE) -C $(@D) clean
+endef
+
+$(eval $(generic-package))
+
+# http://www.vanheusden.com/httping/httping-1.5.6.tgz
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-12  0:00 gilles.talis at gmail.com
@ 2013-01-12 11:07 ` Thomas Petazzoni
  2013-01-12 16:37   ` Gilles Talis
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2013-01-12 11:07 UTC (permalink / raw)
  To: buildroot

Dear gilles.talis at gmail.com,

Thanks for your contribution! Your package mostly looks good, but I (of
course!) have a few comments. See below.

On Fri, 11 Jan 2013 16:00:46 -0800, gilles.talis at gmail.com wrote:
> diff --git a/package/httping/Config.in b/package/httping/Config.in
> new file mode 100644
> index 0000000..578028f
> --- /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.

Indentation of this line is not correct: it is composed of spaces only,
while it should be one tab + 2 spaces like the other lines.

> +if BR2_PACKAGE_HTTPING
> +
> +config BR2_PACKAGE_HTTPING_OPENSSL
> +	bool "OpenSSL support"
> +	default y
> +	select BR2_PACKAGE_OPENSSL

Here there is a point that is rather unclear in Buildroot:

 *) Should each package offer a configuration sub-option to enable
    features that depend on other packages (like you did on OpenSSL)

 *) Or should a package automatically enable features if it finds that
    the dependencies are available? This is generally what we do for
    packages having an optional dependency on OpenSSL.

This is to be discussed with other Buildroot developers. Probably we
need to clarify what the rule is, and then document it.

> +config BR2_PACKAGE_HTTPING_TFO
> +	bool "TCP Fast Open (TFO) support"
> +	default n

'default n' is the default, so it's not needed.

> --- /dev/null
> +++ b/package/httping/httping.mk
> @@ -0,0 +1,44 @@
> +#############################################################
> +#
> +# httping
> +#
> +#############################################################
> +HTTPING_VERSION = 1.5.6
> +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
> +HTTPING_SITE = http://www.vanheusden.com/httping
> +HTTPING_LICENSE = GPL

You should be more specific about the version, and whether the "or
later" specifier is here or not. Good choices are: GPLv2, GPLv2+,
GPLv3, GPLv3+.

> +HTTPING_LICENSE_FILES = license.txt
> +
> +ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> +HTTPING_DEPENDENCIES = openssl
> +endif
> +
> +
> +ifneq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> +     HTTPING_SSL = no
> +endif

This should be slightly changed to something like:

ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
	HTTPING_DEPENDENCIES += openssl
	HTTPING_SSL = yes
else
	HTTPING_SSL = no
endif

(assuming SSL=yes is correct to enable SSL support)

> +ifeq ($(BR2_PACKAGE_HTTPING_TFO),y)
> +     HTTPING_TFO = yes 
> +endif
> +
> +define HTTPING_BUILD_CMDS
> +     $(MAKE) CC="$(TARGET_CC)" \
> +     SSL=$(HTTPING_SSL) \
> +     DEBUG=no \
> +     TFO=$(HTTPING_TFO) \
> +     LD="$(TARGET_LD)" \
> +     STRIP="$(TARGET_STRIP)" -C $(@D)
> +endef

Here, I would recommend using $(TARGET_CONFIGURE_OPTS) :

	$(MAKE) $(TARGET_CONFIGURE_OPTS) \
		SSL=$(HTTPING_SSL) \
		DEBUG=no \
		TFO=$(HTTPING_TFO) \
		-C $(@D)

> +define HTTPING_INSTALL_TARGET_CMDS
> +     $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin
> +endef

With -D, you should give a complete path to the  target binary:

	$(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin/httping

> +define HTTPING_CLEAN_CMDS
> +     $(MAKE) -C $(@D) clean
> +endef
> +
> +$(eval $(generic-package))
> +
> +# http://www.vanheusden.com/httping/httping-1.5.6.tgz

This comment is not needed.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-12 11:07 ` Thomas Petazzoni
@ 2013-01-12 16:37   ` Gilles Talis
  2013-01-13  0:37   ` Gilles Talis
  2013-01-19 13:39   ` Arnout Vandecappelle
  2 siblings, 0 replies; 14+ messages in thread
From: Gilles Talis @ 2013-01-12 16:37 UTC (permalink / raw)
  To: buildroot

Dear Thomas,
Thanks a lot for your comments, I am going to fix these issues right away!
It is amazing how much you can learn from code reviews :-).
Best regards,
Gilles
 On Jan 12, 2013 3:07 AM, "Thomas Petazzoni" <
thomas.petazzoni@free-electrons.com> wrote:

> Dear gilles.talis at gmail.com,
>
> Thanks for your contribution! Your package mostly looks good, but I (of
> course!) have a few comments. See below.
>
> On Fri, 11 Jan 2013 16:00:46 -0800, gilles.talis at gmail.com wrote:
> > diff --git a/package/httping/Config.in b/package/httping/Config.in
> > new file mode 100644
> > index 0000000..578028f
> > --- /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.
>
> Indentation of this line is not correct: it is composed of spaces only,
> while it should be one tab + 2 spaces like the other lines.
>
> > +if BR2_PACKAGE_HTTPING
> > +
> > +config BR2_PACKAGE_HTTPING_OPENSSL
> > +     bool "OpenSSL support"
> > +     default y
> > +     select BR2_PACKAGE_OPENSSL
>
> Here there is a point that is rather unclear in Buildroot:
>
>  *) Should each package offer a configuration sub-option to enable
>     features that depend on other packages (like you did on OpenSSL)
>
>  *) Or should a package automatically enable features if it finds that
>     the dependencies are available? This is generally what we do for
>     packages having an optional dependency on OpenSSL.
>
> This is to be discussed with other Buildroot developers. Probably we
> need to clarify what the rule is, and then document it.
>
> > +config BR2_PACKAGE_HTTPING_TFO
> > +     bool "TCP Fast Open (TFO) support"
> > +     default n
>
> 'default n' is the default, so it's not needed.
>
> > --- /dev/null
> > +++ b/package/httping/httping.mk
> > @@ -0,0 +1,44 @@
> > +#############################################################
> > +#
> > +# httping
> > +#
> > +#############################################################
> > +HTTPING_VERSION = 1.5.6
> > +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
> > +HTTPING_SITE = http://www.vanheusden.com/httping
> > +HTTPING_LICENSE = GPL
>
> You should be more specific about the version, and whether the "or
> later" specifier is here or not. Good choices are: GPLv2, GPLv2+,
> GPLv3, GPLv3+.
>
> > +HTTPING_LICENSE_FILES = license.txt
> > +
> > +ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> > +HTTPING_DEPENDENCIES = openssl
> > +endif
> > +
> > +
> > +ifneq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> > +     HTTPING_SSL = no
> > +endif
>
> This should be slightly changed to something like:
>
> ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
>         HTTPING_DEPENDENCIES += openssl
>         HTTPING_SSL = yes
> else
>         HTTPING_SSL = no
> endif
>
> (assuming SSL=yes is correct to enable SSL support)
>
> > +ifeq ($(BR2_PACKAGE_HTTPING_TFO),y)
> > +     HTTPING_TFO = yes
> > +endif
> > +
> > +define HTTPING_BUILD_CMDS
> > +     $(MAKE) CC="$(TARGET_CC)" \
> > +     SSL=$(HTTPING_SSL) \
> > +     DEBUG=no \
> > +     TFO=$(HTTPING_TFO) \
> > +     LD="$(TARGET_LD)" \
> > +     STRIP="$(TARGET_STRIP)" -C $(@D)
> > +endef
>
> Here, I would recommend using $(TARGET_CONFIGURE_OPTS) :
>
>         $(MAKE) $(TARGET_CONFIGURE_OPTS) \
>                 SSL=$(HTTPING_SSL) \
>                 DEBUG=no \
>                 TFO=$(HTTPING_TFO) \
>                 -C $(@D)
>
> > +define HTTPING_INSTALL_TARGET_CMDS
> > +     $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin
> > +endef
>
> With -D, you should give a complete path to the  target binary:
>
>         $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin/httping
>
> > +define HTTPING_CLEAN_CMDS
> > +     $(MAKE) -C $(@D) clean
> > +endef
> > +
> > +$(eval $(generic-package))
> > +
> > +# http://www.vanheusden.com/httping/httping-1.5.6.tgz
>
> This comment is not needed.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130112/7eff6bf2/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
@ 2013-01-13  0:29 Gilles Talis
  2013-01-13 11:05 ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Gilles Talis @ 2013-01-13  0:29 UTC (permalink / raw)
  To: buildroot

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

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))
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-12 11:07 ` Thomas Petazzoni
  2013-01-12 16:37   ` Gilles Talis
@ 2013-01-13  0:37   ` Gilles Talis
  2013-01-19 13:39   ` Arnout Vandecappelle
  2 siblings, 0 replies; 14+ messages in thread
From: Gilles Talis @ 2013-01-13  0:37 UTC (permalink / raw)
  To: buildroot

Dear Thomas,
Thanks again for your comments.

There's one recommendation I could not follow though. Using
$(TARGET_CONFIGURE_OPTS) instead of "CC=..." and "LD..." seems to dismiss
the "SSL=" and "TFO=" options. I will have to spend more time to figure out
why this happens.

I have kept the build command that way for the time being.

best regards

Gilles.

On Jan 12, 2013 3:07 AM, "Thomas Petazzoni" <
thomas.petazzoni@free-electrons.com> wrote:

> Dear gilles.talis at gmail.com,
>
> Thanks for your contribution! Your package mostly looks good, but I (of
> course!) have a few comments. See below.
>
> On Fri, 11 Jan 2013 16:00:46 -0800, gilles.talis at gmail.com wrote:
> > diff --git a/package/httping/Config.in b/package/httping/Config.in
> > new file mode 100644
> > index 0000000..578028f
> > --- /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.
>
> Indentation of this line is not correct: it is composed of spaces only,
> while it should be one tab + 2 spaces like the other lines.
>
> > +if BR2_PACKAGE_HTTPING
> > +
> > +config BR2_PACKAGE_HTTPING_OPENSSL
> > +     bool "OpenSSL support"
> > +     default y
> > +     select BR2_PACKAGE_OPENSSL
>
> Here there is a point that is rather unclear in Buildroot:
>
>  *) Should each package offer a configuration sub-option to enable
>     features that depend on other packages (like you did on OpenSSL)
>
>  *) Or should a package automatically enable features if it finds that
>     the dependencies are available? This is generally what we do for
>     packages having an optional dependency on OpenSSL.
>
> This is to be discussed with other Buildroot developers. Probably we
> need to clarify what the rule is, and then document it.
>
> > +config BR2_PACKAGE_HTTPING_TFO
> > +     bool "TCP Fast Open (TFO) support"
> > +     default n
>
> 'default n' is the default, so it's not needed.
>
> > --- /dev/null
> > +++ b/package/httping/httping.mk
> > @@ -0,0 +1,44 @@
> > +#############################################################
> > +#
> > +# httping
> > +#
> > +#############################################################
> > +HTTPING_VERSION = 1.5.6
> > +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
> > +HTTPING_SITE = http://www.vanheusden.com/httping
> > +HTTPING_LICENSE = GPL
>
> You should be more specific about the version, and whether the "or
> later" specifier is here or not. Good choices are: GPLv2, GPLv2+,
> GPLv3, GPLv3+.
>
> > +HTTPING_LICENSE_FILES = license.txt
> > +
> > +ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> > +HTTPING_DEPENDENCIES = openssl
> > +endif
> > +
> > +
> > +ifneq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
> > +     HTTPING_SSL = no
> > +endif
>
> This should be slightly changed to something like:
>
> ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
>         HTTPING_DEPENDENCIES += openssl
>         HTTPING_SSL = yes
> else
>         HTTPING_SSL = no
> endif
>
> (assuming SSL=yes is correct to enable SSL support)
>
> > +ifeq ($(BR2_PACKAGE_HTTPING_TFO),y)
> > +     HTTPING_TFO = yes
> > +endif
> > +
> > +define HTTPING_BUILD_CMDS
> > +     $(MAKE) CC="$(TARGET_CC)" \
> > +     SSL=$(HTTPING_SSL) \
> > +     DEBUG=no \
> > +     TFO=$(HTTPING_TFO) \
> > +     LD="$(TARGET_LD)" \
> > +     STRIP="$(TARGET_STRIP)" -C $(@D)
> > +endef
>
> Here, I would recommend using $(TARGET_CONFIGURE_OPTS) :
>
>         $(MAKE) $(TARGET_CONFIGURE_OPTS) \
>                 SSL=$(HTTPING_SSL) \
>                 DEBUG=no \
>                 TFO=$(HTTPING_TFO) \
>                 -C $(@D)
>
> > +define HTTPING_INSTALL_TARGET_CMDS
> > +     $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin
> > +endef
>
> With -D, you should give a complete path to the  target binary:
>
>         $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin/httping
>
> > +define HTTPING_CLEAN_CMDS
> > +     $(MAKE) -C $(@D) clean
> > +endef
> > +
> > +$(eval $(generic-package))
> > +
> > +# http://www.vanheusden.com/httping/httping-1.5.6.tgz
>
> This comment is not needed.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130112/2f3d69cc/attachment-0001.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-13  0:29 Gilles Talis
@ 2013-01-13 11:05 ` Thomas Petazzoni
  2013-01-13 20:21   ` Gilles Talis
       [not found]   ` <CAKcgs2xk-=iG-3Ezmc1upEPBjhSGcmgg9WSwn=1E3+OKDT05fg@mail.gmail.com>
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2013-01-13 11:05 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-13 11:05 ` Thomas Petazzoni
@ 2013-01-13 20:21   ` Gilles Talis
  2013-01-13 20:32     ` Thomas Petazzoni
       [not found]   ` <CAKcgs2xk-=iG-3Ezmc1upEPBjhSGcmgg9WSwn=1E3+OKDT05fg@mail.gmail.com>
  1 sibling, 1 reply; 14+ messages in thread
From: Gilles Talis @ 2013-01-13 20:21 UTC (permalink / raw)
  To: buildroot

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130113/86e948ac/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-13 20:21   ` Gilles Talis
@ 2013-01-13 20:32     ` Thomas Petazzoni
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2013-01-13 20:32 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
       [not found]   ` <CAKcgs2xk-=iG-3Ezmc1upEPBjhSGcmgg9WSwn=1E3+OKDT05fg@mail.gmail.com>
@ 2013-01-14  0:47     ` Gilles Talis
  2013-01-14 13:02       ` Peter Korsgaard
  0 siblings, 1 reply; 14+ messages in thread
From: Gilles Talis @ 2013-01-14  0:47 UTC (permalink / raw)
  To: buildroot

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
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130113/6e554f49/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-14  0:47     ` Gilles Talis
@ 2013-01-14 13:02       ` Peter Korsgaard
  2013-01-19 13:51         ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Korsgaard @ 2013-01-14 13:02 UTC (permalink / raw)
  To: buildroot

>>>>> "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 += ..

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
@ 2013-01-14 23:32 gilles.talis at gmail.com
  2013-01-15  8:23 ` Peter Korsgaard
  0 siblings, 1 reply; 14+ messages in thread
From: gilles.talis at gmail.com @ 2013-01-14 23:32 UTC (permalink / raw)
  To: buildroot

From: Gilles Talis <gilles.talis@gmail.com>

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)

Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
---
 package/Config.in                             |    1 +
 package/httping/Config.in                     |   23 +++++++++++
 package/httping/httping-override-cflags.patch |   53 +++++++++++++++++++++++++
 package/httping/httping.mk                    |   37 +++++++++++++++++
 4 files changed, 114 insertions(+), 0 deletions(-)
 create mode 100644 package/httping/Config.in
 create mode 100644 package/httping/httping-override-cflags.patch
 create mode 100644 package/httping/httping.mk

diff --git a/package/Config.in b/package/Config.in
index 42cde07..c59f342 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -631,6 +631,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..aa1550a
--- /dev/null
+++ b/package/httping/Config.in
@@ -0,0 +1,23 @@
+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"
+       select BR2_PACKAGE_OPENSSL
+       help
+         Adds openSSL support to httping
+
+config BR2_PACKAGE_HTTPING_TFO
+       bool "TCP Fast Open (TFO) support"
+
+endif
diff --git a/package/httping/httping-override-cflags.patch b/package/httping/httping-override-cflags.patch
new file mode 100644
index 0000000..a3cf59d
--- /dev/null
+++ b/package/httping/httping-override-cflags.patch
@@ -0,0 +1,53 @@
+From 66f5811dd45fa27a7bfacf946dfefd16d765bb4d Mon Sep 17 00:00:00 2001
+From: Gilles Talis <gilles.talis@gmail.com>
+Date: Mon, 14 Jan 2013 11:48:00 -0800
+Subject: [PATCH] allow CFLAGS/LDFLAGS to be overriden from command line
+
+Ensure required CFLAGS/LDFLAGS are appended to those provided in command line
+
+Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
+---
+ Makefile |   12 ++++++------
+ 1 files changed, 6 insertions(+), 6 deletions(-)
+
+diff --git a/Makefile b/Makefile
+index 4c4f6a8..fdbb4cb 100644
+--- a/Makefile
++++ b/Makefile
+@@ -19,7 +19,7 @@ TARGET=httping
+ DEBUG=yes
+ WFLAGS=-Wall -W
+ OFLAGS=-O3
+-CFLAGS+=$(WFLAGS) $(OFLAGS) -DVERSION=\"$(VERSION)\"
++override CFLAGS+=$(WFLAGS) $(OFLAGS) -DVERSION=\"$(VERSION)\"
+
+ PACKAGE=$(TARGET)-$(VERSION)
+ PREFIX=/usr
+@@ -48,19 +48,19 @@ DOCS=license.txt license.OpenSSL readme.txt
+ # TFO=yes
+
+ ifeq ($(SSL),no)
+-CFLAGS+=-DNO_SSL
++override CFLAGS+=-DNO_SSL
+ else
+ OBJS+=mssl.o
+-LDFLAGS+=-lssl -lcrypto
++override LDFLAGS+=-lssl -lcrypto
+ endif
+
+ ifeq ($(TFO),yes)
+-CFLAGS+=-DTCP_TFO
++override CFLAGS+=-DTCP_TFO
+ endif
+
+ ifeq ($(DEBUG),yes)
+-CFLAGS+=-D_DEBUG -ggdb
+-LDFLAGS+=-g
++override CFLAGS+=-D_DEBUG -ggdb
++override LDFLAGS+=-g
+ endif
+
+ ifeq ($(ARM),yes)
+--
+1.7.4.1
+
diff --git a/package/httping/httping.mk b/package/httping/httping.mk
new file mode 100644
index 0000000..bec65f3
--- /dev/null
+++ b/package/httping/httping.mk
@@ -0,0 +1,37 @@
+#############################################################
+#
+# httping
+#
+#############################################################
+HTTPING_VERSION = 1.5.7
+HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
+HTTPING_SITE = http://www.vanheusden.com/httping
+HTTPING_LICENSE = GPLv2
+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) $(TARGET_CONFIGURE_OPTS) \
+               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))
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-14 23:32 [Buildroot] [PATCH 1/1] httping: new package gilles.talis at gmail.com
@ 2013-01-15  8:23 ` Peter Korsgaard
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Korsgaard @ 2013-01-15  8:23 UTC (permalink / raw)
  To: buildroot

>>>>> "gilles" == gilles talis <gilles.talis@gmail.com> writes:

 gilles> From: Gilles Talis <gilles.talis@gmail.com>
 gilles> Httping is like 'ping' but for http-requests.
 gilles> Give it an url, and it'll show you how long it takes to connect,
 gilles> send a request and retrieve the reply (only the headers)

Thanks, committed with minor changes (see below).

 gilles> +++ b/package/httping/Config.in
 gilles> @@ -0,0 +1,23 @@
 gilles> +config BR2_PACKAGE_HTTPING
 gilles> +       bool "httping"
 gilles> +       help
 gilles> +         Httping is like 'ping' but for http-requests.
 gilles> +         Give it an url, and it'll show you how long it takes to connect,
 gilles> +         send a request and retrieve the reply (only the headers).
 gilles> +         Be aware that the transmission across the network also takes time!
 gilles> +         So it measures the latency of the webserver + network.
 gilles> +
 gilles> +         http://www.vanheusden.com/httping/
 gilles> +
 gilles> +if BR2_PACKAGE_HTTPING
 gilles> +
 gilles> +config BR2_PACKAGE_HTTPING_OPENSSL
 gilles> +       bool "OpenSSL support"
 gilles> +       select BR2_PACKAGE_OPENSSL
 gilles> +       help
 gilles> +         Adds openSSL support to httping

We normally automatically add openssl support if the openssl package is
enabled, so I've dropped this option.

 gilles> +
 gilles> +config BR2_PACKAGE_HTTPING_TFO
 gilles> +       bool "TCP Fast Open (TFO) support"

The TFO option seems to add very little overhead, so I think we could
have just always enabled it, but OK.

 gilles> +++ b/package/httping/httping.mk
 gilles> @@ -0,0 +1,37 @@
 gilles> +#############################################################
 gilles> +#
 gilles> +# httping
 gilles> +#
 gilles> +#############################################################
 gilles> +HTTPING_VERSION = 1.5.7

We normally have an empty line before _VERSION.

 gilles> +HTTPING_SOURCE = httping-$(HTTPING_VERSION).tgz
 gilles> +HTTPING_SITE = http://www.vanheusden.com/httping
 gilles> +HTTPING_LICENSE = GPLv2
 gilles> +HTTPING_LICENSE_FILES = license.txt
 gilles> +
 gilles> +ifeq ($(BR2_PACKAGE_HTTPING_OPENSSL),y)
 gilles> +       HTTPING_DEPENDENCIES = openssl
 gilles> +else
 gilles> +       HTTPING_SSL = no
 gilles> +endif

 gilles> +
 gilles> +ifeq ($(BR2_PACKAGE_HTTPING_TFO),y)
 gilles> +       HTTPING_TFO = yes
 gilles> +endif
 gilles> +
 gilles> +define HTTPING_BUILD_CMDS
 gilles> +       $(MAKE) $(TARGET_CONFIGURE_OPTS) \
 gilles> +               SSL=$(HTTPING_SSL) \
 gilles> +               DEBUG=no \
 gilles> +               TFO=$(HTTPING_TFO) -C $(@D)
 gilles> +endef

We normally indent the lines inside defines with <tabs> to be consistent
with make rules. I've cleaned this up a bit while changing the openssl
handling.

 gilles> +
 gilles> +define HTTPING_INSTALL_TARGET_CMDS
 gilles> +       $(INSTALL) -D -m 0755 $(@D)/httping $(TARGET_DIR)/usr/bin/httping

Upstream has a working 'make install', so I changed it to use that
instead.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-12 11:07 ` Thomas Petazzoni
  2013-01-12 16:37   ` Gilles Talis
  2013-01-13  0:37   ` Gilles Talis
@ 2013-01-19 13:39   ` Arnout Vandecappelle
  2 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2013-01-19 13:39 UTC (permalink / raw)
  To: buildroot

On 12/01/13 12:07, Thomas Petazzoni wrote:
>> >  +config BR2_PACKAGE_HTTPING_OPENSSL
>> >  +	bool "OpenSSL support"
>> >  +	default y
>> >  +	select BR2_PACKAGE_OPENSSL
> Here there is a point that is rather unclear in Buildroot:
>
>   *) Should each package offer a configuration sub-option to enable
>      features that depend on other packages (like you did on OpenSSL)
>
>   *) Or should a package automatically enable features if it finds that
>      the dependencies are available? This is generally what we do for
>      packages having an optional dependency on OpenSSL.
>
> This is to be discussed with other Buildroot developers. Probably we
> need to clarify what the rule is, and then document it.

  +1 on the "needs to be documented".

  But for me the rule is fairly clear: if it doesn't significantly 
increase binary size, then it should be selected automatically if available.

  It would be good to add to the package's help text that "SSL support is 
available if the openssl package (in the Libraries -> Crypto menu) is 
selected."


  Regards,
  Arnout


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Buildroot] [PATCH 1/1] httping: new package
  2013-01-14 13:02       ` Peter Korsgaard
@ 2013-01-19 13:51         ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2013-01-19 13:51 UTC (permalink / raw)
  To: buildroot

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


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-01-19 13:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-14 23:32 [Buildroot] [PATCH 1/1] httping: new package gilles.talis at gmail.com
2013-01-15  8:23 ` Peter Korsgaard
  -- strict thread matches above, loose matches on Subject: below --
2013-01-13  0:29 Gilles Talis
2013-01-13 11:05 ` Thomas Petazzoni
2013-01-13 20:21   ` Gilles Talis
2013-01-13 20:32     ` Thomas Petazzoni
     [not found]   ` <CAKcgs2xk-=iG-3Ezmc1upEPBjhSGcmgg9WSwn=1E3+OKDT05fg@mail.gmail.com>
2013-01-14  0:47     ` Gilles Talis
2013-01-14 13:02       ` Peter Korsgaard
2013-01-19 13:51         ` Arnout Vandecappelle
2013-01-12  0:00 gilles.talis at gmail.com
2013-01-12 11:07 ` Thomas Petazzoni
2013-01-12 16:37   ` Gilles Talis
2013-01-13  0:37   ` Gilles Talis
2013-01-19 13:39   ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox