diff mbox

package/rrdtool: add support for perl bindings

Message ID 1466665963-5543-1-git-send-email-yann.morin@orange.com
State Changes Requested
Headers show

Commit Message

Yann E. MORIN June 23, 2016, 7:12 a.m. UTC 
From: Cedric Chedaleux <cedric.chedaleux@orange.com>

Signed-off-by: Cedric Chedaleux <cedric.chedaleux@orange.com>
[yann.morin@orange.com: slight cleanup in code layout, change leading
spaces with leading tabs, rebase on top of master]
Signed-off-by: "Yann E. MORIN" <yann.morin@orange.com>
---
 package/rrdtool/rrdtool.mk | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni June 24, 2016, 3:58 p.m. UTC | #1 
Hello,

On Thu, 23 Jun 2016 09:12:43 +0200, Yann E. MORIN wrote:

> +ifeq ($(BR2_PACKAGE_PERL),y)
> +RRDTOOL_DEPENDENCIES += host-perl

It is somewhat weird, one would expect to see a dependency on "perl"
here, not "host-perl".

However, maybe you simply need host-perl at *build* time to build the
bindings, and the target perl is only needed at *runtime*. If that's
the case, then a comment above this dependency would be good to have.

If you confirm my understanding, I can also just add the comment when
applying.

Best regards,

Thomas
Yann E. MORIN June 28, 2016, 8:30 a.m. UTC | #2 
Thomas, All,

On 2016-06-24 17:58 +0200, Thomas Petazzoni spake thusly:
> On Thu, 23 Jun 2016 09:12:43 +0200, Yann E. MORIN wrote:
> > +ifeq ($(BR2_PACKAGE_PERL),y)
> > +RRDTOOL_DEPENDENCIES += host-perl
> It is somewhat weird, one would expect to see a dependency on "perl"
> here, not "host-perl".
> 
> However, maybe you simply need host-perl at *build* time to build the
> bindings, and the target perl is only needed at *runtime*. If that's
> the case, then a comment above this dependency would be good to have.
> 
> If you confirm my understanding, I can also just add the comment when
> applying.

In fact, the host-perl is only used for a few things:

 1- extract some configuration:

    - the perl version: we need host-perl because otherwise the host
      system perl may be a different version;

    - the compiler used to build perl: this is not OK, because we'd get
      the host compiler, not the target one. This is why we override it
      in the RRDTOOL_CONF_ENV.

 2- generate the Makefiles from Makefile.PL templates.


However, I withdraw this patch, as it breaks now with the paranoid
wrapper. I'll fix that in the coming days.

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/package/rrdtool/rrdtool.mk b/package/rrdtool/rrdtool.mk
index 6d6ab0c..589e4a3 100644
--- a/package/rrdtool/rrdtool.mk
+++ b/package/rrdtool/rrdtool.mk
@@ -19,11 +19,22 @@  RRDTOOL_CONF_OPTS = \
 	--disable-librados \
 	--disable-libwrap \
 	--disable-lua \
-	--disable-perl \
 	--disable-python \
 	--disable-ruby \
 	--disable-tcl
 
+ifeq ($(BR2_PACKAGE_PERL),y)
+RRDTOOL_DEPENDENCIES += host-perl
+RRDTOOL_CONF_OPTS += --enable-perl
+RRDTOOL_CONF_ENV += \
+	ac_cv_path_PERL=$(HOST_DIR)/usr/bin/perl \
+	PERLCC="$(TARGET_CC_NOCCACHE)" \
+	PERLCCFLAGS="$(TARGET_CFLAGS)" \
+	PERLLDFLAGS="$(TARGET_LDFLAGS)"
+else
+RRDTOOL_CONF_OPTS += --disable-perl
+endif
+
 ifeq ($(BR2_PACKAGE_RRDTOOL_RRDGRAPH),y)
 RRDTOOL_DEPENDENCIES += cairo pango
 else