collectd: drop rrdtool selects

PATCH New Gustavo Zacarias 3 participants 6 messages Oct 8, 2015 – Oct 9, 2015

lore

Share & tools

lore

Gustavo Zacarias

author

2015-10-08 19:00 UTC #0 lore raw

PATCH 1/2 collectd: drop rrdtool selects

Drop indirect rrdtool selects, they're already selected by rrdtool itself so there's no need.

Signed-off-by Gustavo Zacarias <[email protected]>

---
 package/collectd/Config.in | 4 ----
 1 file changed, 4 deletions(-)

package/collectd/Config.in +0 -4

index 2054978..5cdb24e 100644
@@ -504,11 +504,7 @@ config BR2_PACKAGE_COLLECTD_NETWORK
 config BR2_PACKAGE_COLLECTD_RRDTOOL
 	bool "rrdtool"
 	depends on BR2_USE_WCHAR # rrdtool
-	select BR2_PACKAGE_FREETYPE
-	select BR2_PACKAGE_LIBART
-	select BR2_PACKAGE_LIBPNG
 	select BR2_PACKAGE_RRDTOOL
-	select BR2_PACKAGE_ZLIB
 	help
 	  Writes values to RRD-files.

Signature · 1 line

2.4.9

Gustavo Zacarias

author

2015-10-08 19:01 UTC #1 lore raw

PATCH 2/2 rrdtool: bump to version 1.5.4

Drop patches, they're no longer relevant.

Signed-off-by Gustavo Zacarias <[email protected]>

---
 package/rrdtool/0001-automake-compat.patch         | 18 -----------
 ...0002-configure-dont-hardcode-include-dirs.patch | 26 ----------------
 package/rrdtool/Config.in                          | 36 +++++++++++++++++-----
 package/rrdtool/rrdtool.hash                       |  2 +-
 package/rrdtool/rrdtool.mk                         | 36 ++++++++++++----------
 5 files changed, 49 insertions(+), 69 deletions(-)
 delete mode 100644 package/rrdtool/0001-automake-compat.patch
 delete mode 100644 package/rrdtool/0002-configure-dont-hardcode-include-dirs.patch

package/rrdtool/0001-automake-compat.patch deleted +0 ~~-18~~

deleted file mode 100644
index 92b753d..0000000
@@ -1,18 +0,0 @@
-Make it compatible with newer autoconf/automake.
-Patch from OpenWRT.
-
-Signed-off-by: Gustavo Zacarias <[email protected]>
-
---- a/bindings/tcl/Makefile.am
-+++ b/bindings/tcl/Makefile.am
-@@ -26,8 +26,8 @@ tclpkgdir = @TCL_PACKAGE_DIR@
- tclpkg_DATA = pkgIndex.tcl
- tclpkg_SCRIPTS = ifOctets.tcl
- else
--pkglib_DATA = pkgIndex.tcl
--pkglib_SCRIPTS = ifOctets.tcl
-+pkgdata_DATA = pkgIndex.tcl
-+pkgdata_SCRIPTS = ifOctets.tcl
- endif
- 
- # Automake doen't like `tclrrd$(VERSION)$(TCL_SHLIB_SUFFIX)' as

package/rrdtool/0002-configure-dont-hardcode-include-dirs.patch deleted +0 ~~-26~~

deleted file mode 100644
index 43b57a1..0000000
@@ -1,26 +0,0 @@
-[PATCH] configure.ac: don't hardcode include files search paths
-
-Breaks cross compilation if host has libart/freetype.
-
-Signed-off-by: Peter Korsgaard <[email protected]>
----
- configure.ac |    4 ++--
- 1 file changed, 2 insertions(+), 2 deletions(-)
-
-Index: rrdtool-1.2.30/configure.ac
-===================================================================
---- rrdtool-1.2.30.orig/configure.ac
-+++ rrdtool-1.2.30/configure.ac
-@@ -524,10 +524,10 @@
- 
- CORE_LIBS="$LIBS"
- 
--EX_CHECK_ALL(art_lgpl_2, art_vpath_add_point,       libart_lgpl/libart.h,   libart-2.0,  2.3.17, ftp://ftp.gnome.org/pub/GNOME/sources/libart_lgpl/2.3/, /usr/include/libart-2.0)
-+EX_CHECK_ALL(art_lgpl_2, art_vpath_add_point,       libart_lgpl/libart.h,   libart-2.0,  2.3.17, ftp://ftp.gnome.org/pub/GNOME/sources/libart_lgpl/2.3/, "")
- EX_CHECK_ALL(z,          zlibVersion,               zlib.h,                 zlib,        1.2.3,  http://www.gzip.org/zlib/, "")
- EX_CHECK_ALL(png,        png_access_version_number, png.h,                  libpng,      1.2.10,  http://prdownloads.sourceforge.net/libpng/, "")
--EX_CHECK_ALL(freetype,   FT_Init_FreeType,          ft2build.h,		    freetype2,   2.1.10,  http://prdownloads.sourceforge.net/freetype/, /usr/include/freetype2)
-+EX_CHECK_ALL(freetype,   FT_Init_FreeType,          ft2build.h,		    freetype2,   2.1.10,  http://prdownloads.sourceforge.net/freetype/, "")
- 
- if test "$EX_CHECK_ALL_ERR" = "YES"; then
-   AC_MSG_ERROR([Please fix the library issues listed above and try again.])

package/rrdtool/Config.in +29 -7

index 33fa677..b07c446 100644
@@ -1,15 +1,37 @@
 config BR2_PACKAGE_RRDTOOL
 	bool "rrdtool"
-	depends on BR2_USE_WCHAR
-	select BR2_PACKAGE_FREETYPE
-	select BR2_PACKAGE_LIBART
-	select BR2_PACKAGE_LIBPNG
-	select BR2_PACKAGE_ZLIB
+	depends on BR2_USE_WCHAR # libglib2
+	depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
+	depends on BR2_USE_MMU # libglib2
+	select BR2_PACKAGE_LIBGLIB2
 	help
 	  RRDtool is the OpenSource industry standard, high performance
 	  data logging and graphing system for time series data.
 
 	  http://oss.oetiker.ch/rrdtool/
 
-comment "rrdtool needs a toolchain w/ wchar"
-	depends on !BR2_USE_WCHAR
+if BR2_PACKAGE_RRDTOOL
+
+config BR2_PACKAGE_RRDTOOL_RRDGRAPH
+	bool "rrd_graph"
+	default y
+	depends on BR2_ARCH_HAS_ATOMICS # cairo
+	depends on BR2_INSTALL_LIBSTDCPP # freetype support from pango
+	select BR2_PACKAGE_CAIRO
+	select BR2_PACKAGE_CAIRO_PDF
+	select BR2_PACKAGE_CAIRO_PNG
+	select BR2_PACKAGE_CAIRO_PS
+	select BR2_PACKAGE_CAIRO_SVG
+	select BR2_PACKAGE_PANGO
+	help
+	  This enables the graphing capabilities ('rrdgraph').
+	  Without this it will only act as a database backend.
+
+comment "rrd_graph support needs a toolchain w/ C++"
+	depends on !BR2_INSTALL_LIBSTDCPP
+
+endif
+
+comment "rrdtool needs a toolchain w/ wchar, threads"
+	depends on BR2_USE_MMU
+	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS

package/rrdtool/rrdtool.hash +1 -1

index 20af75a..55a7f5b 100644
@@ -1,2 +1,2 @@
 # Locally calculated
-sha256	3190efea410a6dd035799717948b2df09910f608d72d23ee81adad4cd0184ae9	rrdtool-1.2.30.tar.gz
+sha256	3feea3da87c02128a27083f1c7b2cb797ef673e946564c0ce008c1c25a5c3f99	rrdtool-1.5.4.tar.gz

package/rrdtool/rrdtool.mk +19 ~~-17~~

index e422694..f90b137 100644
@@ -4,31 +4,33 @@
 #
 ################################################################################
 
-RRDTOOL_VERSION = 1.2.30
+RRDTOOL_VERSION = 1.5.4
 RRDTOOL_SITE = http://oss.oetiker.ch/rrdtool/pub
 RRDTOOL_LICENSE = GPLv2+ with FLOSS license exceptions as explained in COPYRIGHT
-RRDTOOL_LICENSE_FILES = COPYING COPYRIGHT
-
-RRDTOOL_DEPENDENCIES = host-pkgconf freetype libart libpng zlib
-RRDTOOL_AUTORECONF = YES
+RRDTOOL_LICENSE_FILES = COPYRIGHT LICENSE
+RRDTOOL_DEPENDENCIES = host-pkgconf libglib2
 RRDTOOL_INSTALL_STAGING = YES
-RRDTOOL_CONF_ENV = \
-	rd_cv_ieee_works=yes \
-	rd_cv_null_realloc=nope \
-	ac_cv_func_mmap_fixed_mapped=yes
 RRDTOOL_CONF_OPTS = \
+	--disable-examples \
+	--disable-libdbi \
+	--disable-librados \
+	--disable-libwrap \
+	--disable-lua \
 	--disable-perl \
 	--disable-python \
 	--disable-ruby \
-	--disable-tcl \
-	--program-transform-name='' \
-	$(if $(BR2_TOOLCHAIN_HAS_THREADS),,--disable-pthread)
-RRDTOOL_MAKE = $(MAKE1)
+	--disable-tcl
 
-define RRDTOOL_REMOVE_EXAMPLES
-	rm -rf $(TARGET_DIR)/usr/share/rrdtool/examples
-endef
+ifeq ($(BR2_PACKAGE_RRDTOOL_RRDGRAPH),y)
+RRDTOOL_DEPENDENCIES += cairo pango
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_graph
+endif
 
-RRDTOOL_POST_INSTALL_TARGET_HOOKS += RRDTOOL_REMOVE_EXAMPLES
+ifeq ($(BR2_PACKAGE_LIBXML2),y)
+RRDTOOL_DEPENDENCIES += libxml2
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_restore
+endif
 
 $(eval $(autotools-package))

Signature · 1 line

2.4.9

Vicente Olivert Riera

2015-10-09 12:51 UTC #2 lore raw

PATCH 2/2 rrdtool: bump to version 1.5.4

Dear Gustavo Zacarias,

Quoted 3 lines · Gustavo Zacarias

Drop patches, they're no longer relevant.
Signed-off-by Gustavo Zacarias <[email protected]>

[snip]

Quoted 6 lines

package/rrdtool/Config.in +0 -0

index 33fa677..b07c446 100644
@@ -1,15 +1,37 @@
 config BR2_PACKAGE_RRDTOOL

package/rrdtool/Config.in line 6

Quoted 10 lines

package/rrdtool/Config.in +4 -5

 	bool "rrdtool"
-	depends on BR2_USE_WCHAR
-	select BR2_PACKAGE_FREETYPE
-	select BR2_PACKAGE_LIBART
-	select BR2_PACKAGE_LIBPNG
-	select BR2_PACKAGE_ZLIB
+	depends on BR2_USE_WCHAR # libglib2
+	depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
+	depends on BR2_USE_MMU # libglib2
+	select BR2_PACKAGE_LIBGLIB2

You are doing more things here than just bumping a version and removing some patches (which is what your commit log says). I'm not saying this is wrong. What I mean is that it would be good writing some explanation in the commit log about why are you removing those selects and add libglib2.

Quoted 23 lines

package/rrdtool/Config.in +15 -2

 	help
 	  RRDtool is the OpenSource industry standard, high performance
 	  data logging and graphing system for time series data.
 
 	  http://oss.oetiker.ch/rrdtool/
 
-comment "rrdtool needs a toolchain w/ wchar"
-	depends on !BR2_USE_WCHAR
+if BR2_PACKAGE_RRDTOOL
+
+config BR2_PACKAGE_RRDTOOL_RRDGRAPH
+	bool "rrd_graph"
+	default y
+	depends on BR2_ARCH_HAS_ATOMICS # cairo
+	depends on BR2_INSTALL_LIBSTDCPP # freetype support from pango
+	select BR2_PACKAGE_CAIRO
+	select BR2_PACKAGE_CAIRO_PDF
+	select BR2_PACKAGE_CAIRO_PNG
+	select BR2_PACKAGE_CAIRO_PS
+	select BR2_PACKAGE_CAIRO_SVG
+	select BR2_PACKAGE_PANGO
+	help
+	  This enables the graphing capabilities ('rrdgraph').

package/rrdtool/Config.in line 37

Quoted 10 lines

package/rrdtool/Config.in +10 -0

+	  Without this it will only act as a database backend.
+
+comment "rrd_graph support needs a toolchain w/ C++"
+	depends on !BR2_INSTALL_LIBSTDCPP
+
+endif
+
+comment "rrdtool needs a toolchain w/ wchar, threads"
+	depends on BR2_USE_MMU
+	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS

And here you are adding more functionality to this package, which I think it would be good as well to explain it in the commit log. In fact, this is something I think that should be done in a separate patch. "rrdtools: add rrdgraph support", for instance.

Quoted 27 lines

package/rrdtool/rrdtool.hash +1 -1

index 20af75a..55a7f5b 100644
@@ -1,2 +1,2 @@
 # Locally calculated
-sha256	3190efea410a6dd035799717948b2df09910f608d72d23ee81adad4cd0184ae9	rrdtool-1.2.30.tar.gz
+sha256	3feea3da87c02128a27083f1c7b2cb797ef673e946564c0ce008c1c25a5c3f99	rrdtool-1.5.4.tar.gz

package/rrdtool/rrdtool.mk +3 -5

index e422694..f90b137 100644
@@ -4,31 +4,33 @@
 #
 ################################################################################
 
-RRDTOOL_VERSION = 1.2.30
+RRDTOOL_VERSION = 1.5.4
 RRDTOOL_SITE = http://oss.oetiker.ch/rrdtool/pub
 RRDTOOL_LICENSE = GPLv2+ with FLOSS license exceptions as explained in COPYRIGHT
-RRDTOOL_LICENSE_FILES = COPYING COPYRIGHT
-
-RRDTOOL_DEPENDENCIES = host-pkgconf freetype libart libpng zlib
-RRDTOOL_AUTORECONF = YES
+RRDTOOL_LICENSE_FILES = COPYRIGHT LICENSE
+RRDTOOL_DEPENDENCIES = host-pkgconf libglib2
 RRDTOOL_INSTALL_STAGING = YES

package/rrdtool/rrdtool.mk line 18

Quoted 10 lines

package/rrdtool/rrdtool.mk +5 -4

-RRDTOOL_CONF_ENV = \
-	rd_cv_ieee_works=yes \
-	rd_cv_null_realloc=nope \
-	ac_cv_func_mmap_fixed_mapped=yes
 RRDTOOL_CONF_OPTS = \
+	--disable-examples \
+	--disable-libdbi \
+	--disable-librados \
+	--disable-libwrap \
+	--disable-lua \

A comment in the commit log saying that you add some disables (and why, if there is a reason other than the default BR policy) would be great.

package/rrdtool/rrdtool.mk line 22

Quoted 8 lines

package/rrdtool/rrdtool.mk +1 -4

 	--disable-perl \
 	--disable-python \
 	--disable-ruby \
-	--disable-tcl \
-	--program-transform-name='' \
-	$(if $(BR2_TOOLCHAIN_HAS_THREADS),,--disable-pthread)
-RRDTOOL_MAKE = $(MAKE1)
+	--disable-tcl

Again, no explanation in the commit log about why are you doing this.

package/rrdtool/rrdtool.mk line 30

Quoted 3 lines

package/rrdtool/rrdtool.mk +0 -3

-define RRDTOOL_REMOVE_EXAMPLES
-	rm -rf $(TARGET_DIR)/usr/share/rrdtool/examples
-endef

Again, no explanation in the commit log about why are you doing this.

package/rrdtool/rrdtool.mk line 28

Quoted 5 lines

package/rrdtool/rrdtool.mk +5 -0

+ifeq ($(BR2_PACKAGE_RRDTOOL_RRDGRAPH),y)
+RRDTOOL_DEPENDENCIES += cairo pango
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_graph
+endif

According to one of my comments above, this should be in a separate patch. But, hey, that's only my opinion :-)

package/rrdtool/rrdtool.mk line 32

Quoted 1 line

package/rrdtool/rrdtool.mk +0 -1
-RRDTOOL_POST_INSTALL_TARGET_HOOKS += RRDTOOL_REMOVE_EXAMPLES

Indeed since you remove the define. The problem is, why? ^^

package/rrdtool/rrdtool.mk line 34

Quoted 5 lines

package/rrdtool/rrdtool.mk +5 -0

+ifeq ($(BR2_PACKAGE_LIBXML2),y)
+RRDTOOL_DEPENDENCIES += libxml2
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_restore
+endif

So you disable lots of things by default, but for libxml2 you add this? Why? No explanation about that in the commit log.

Regards,

Vincent.

Quoted 2 lines

package/rrdtool/rrdtool.mk +0 -0
 $(eval $(autotools-package))
 

Gustavo Zacarias

author

2015-10-09 13:00 UTC #3 lore raw

PATCH 2/2 rrdtool: bump to version 1.5.4

Quoted 4 lines · Vicente Olivert Riera

You are doing more things here than just bumping a version and removing some patches (which is what your commit log says). I'm not saying this is wrong. What I mean is that it would be good writing some explanation in the commit log about why are you removing those selects and add libglib2.

Hi Vicente.
Because upstream changed the package that much.
I don't think it merits commenting into the rrdtool development mindset, that's upstream business.
Otherwise the comment would be "it now requires libglib2" (which is redundant).

Quoted 20 lines

package/rrdtool/Config.in +20 -0

+config BR2_PACKAGE_RRDTOOL_RRDGRAPH
+	bool "rrd_graph"
+	default y
+	depends on BR2_ARCH_HAS_ATOMICS # cairo
+	depends on BR2_INSTALL_LIBSTDCPP # freetype support from pango
+	select BR2_PACKAGE_CAIRO
+	select BR2_PACKAGE_CAIRO_PDF
+	select BR2_PACKAGE_CAIRO_PNG
+	select BR2_PACKAGE_CAIRO_PS
+	select BR2_PACKAGE_CAIRO_SVG
+	select BR2_PACKAGE_PANGO
+	help
+	  This enables the graphing capabilities ('rrdgraph').
+	  Without this it will only act as a database backend.
+
+comment "rrd_graph support needs a toolchain w/ C++"
+	depends on !BR2_INSTALL_LIBSTDCPP
+
+endif
+

Quoted 8 lines

package/rrdtool/Config.in line 37
Quoted 3 lines
package/rrdtool/Config.in +3 -0
+comment "rrdtool needs a toolchain w/ wchar, threads"
+	depends on BR2_USE_MMU
+	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
And here you are adding more functionality to this package, which I think it would be good as well to explain it in the commit log. In fact, this is something I think that should be done in a separate patch. "rrdtools: add rrdgraph support", for instance.

A bit of background is in order.
Historically rrdtool has been a stats database backend ("rrd files") and graphing tools all together.
For newer versions that's not the case, you can keep the db part and ditch the graphics.
The only tool that currently uses rrd is collectd, which only uses it for collected data storage, not graphics.
So if people just want to store the data in a standard format and graph somewhere/somehow else then this option, with it's ton of fat dependencies, can be turned off.
It's default on for consistency, to give what the previous series gave in terms of tools.

Quoted 2 lines

A comment in the commit log saying that you add some disables (and why, if there is a reason other than the default BR policy) would be great.

Default BR policy to avoid leaks.

Quoted 10 lines

package/rrdtool/rrdtool.mk line 22
Quoted 8 lines
  	--disable-perl \
  	--disable-python \
  	--disable-ruby \
package/rrdtool/rrdtool.mk +1 -4
-	--disable-tcl \
-	--program-transform-name='' \
-	$(if $(BR2_TOOLCHAIN_HAS_THREADS),,--disable-pthread)
-RRDTOOL_MAKE = $(MAKE1)
+	--disable-tcl
Again, no explanation in the commit log about why are you doing this.

Bindings are completely untested, like before, so i've added the new bindings.
Again, pthread disabling is obvious - it doesn't work, we need libglib2 as a mandatory dep, so it's impossible.

Quoted 1 line

Again, no explanation in the commit log about why are you doing this.

--disable-examples added above takes care of this, standard behaviour > hooks.

Quoted 8 lines

package/rrdtool/rrdtool.mk line 28
Quoted 5 lines
package/rrdtool/rrdtool.mk +5 -0
+ifeq ($(BR2_PACKAGE_RRDTOOL_RRDGRAPH),y)
+RRDTOOL_DEPENDENCIES += cairo pango
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_graph
+endif
According to one of my comments above, this should be in a separate patch. But, hey, that's only my opinion :-)

I don't think so, otherwise you'd be stripping previously-available functionality from rrdtool.

Quoted 8 lines

package/rrdtool/rrdtool.mk line 34
Quoted 5 lines
package/rrdtool/rrdtool.mk +5 -0
+ifeq ($(BR2_PACKAGE_LIBXML2),y)
+RRDTOOL_DEPENDENCIES += libxml2
+else
+RRDTOOL_CONF_OPTS += --disable-rrd_restore
+endif
So you disable lots of things by default, but for libxml2 you add this? Why? No explanation about that in the commit log.

rrd_restore is a new tool to restore rrd data from xml files, previously not available.
I decided to use an autodep for this since it's a niche tool/option that didn't merit adding another bool.

Regards.

Vicente Olivert Riera

2015-10-09 12:31 UTC #4 lore raw

PATCH 1/2 collectd: drop rrdtool selects

In reply to Gustavo Zacarias

Dear Gustavo Zacarias,

Quoted 4 lines · Gustavo Zacarias

Drop indirect rrdtool selects, they're already selected by rrdtool itself so there's no need.
Signed-off-by Gustavo Zacarias <[email protected]>

Reviewed-by Vicente Olivert Riera <[email protected]>

Indeed, rrdtool package already selects freetype, libart, libpng and zlib.

Regards,

Vincent.

Thomas Petazzoni

Applied

2015-10-09 13:25 UTC #5 lore raw

PATCH 1/2 collectd: drop rrdtool selects

In reply to Gustavo Zacarias

Dear Gustavo Zacarias,

Quoted 7 lines · Gustavo Zacarias

Drop indirect rrdtool selects, they're already selected by rrdtool itself so there's no need.
Signed-off-by Gustavo Zacarias <[email protected]>
---
 package/collectd/Config.in | 4 ----
 1 file changed, 4 deletions(-)

Applied, thanks.

Thomas

Signature · 3 lines

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