Bug 201514 - net-mgmt/collectd5 - upstream released version 5.5.0
Summary: net-mgmt/collectd5 - upstream released version 5.5.0
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Jason Unovitch
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-13 02:30 UTC by elij
Modified: 2015-08-11 01:50 UTC (History)
3 users (show)

See Also:
ports: maintainer-feedback+


Attachments
collectd5 patch for upgrade to version 5.5.0 (22.55 KB, patch)
2015-07-14 11:44 UTC, Krzysztof
ports: maintainer-approval+
Details | Diff
collectd5.cummulative.patch (23.16 KB, patch)
2015-07-16 20:41 UTC, Krzysztof
ports: maintainer-approval+
Details | Diff
collectd5-5.5.0.patch (23.75 KB, patch)
2015-07-19 03:13 UTC, Jason Unovitch
no flags Details | Diff
Poudriere testport log from 10.1-RELEASE jail (110.16 KB, text/x-log)
2015-07-19 03:15 UTC, Jason Unovitch
no flags Details
collectd5-5.5.0.patch (26.93 KB, patch)
2015-08-02 01:26 UTC, Jason Unovitch
no flags Details | Diff
Poudriere testport log from HEAD jail (181.72 KB, text/x-log)
2015-08-02 01:30 UTC, Jason Unovitch
no flags Details
collectd5-5.5.0.patch (28.79 KB, patch)
2015-08-02 13:13 UTC, Jason Unovitch
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description elij 2015-07-13 02:30:10 UTC
Not sure if the port maintainer is aware or not, but upstream collectd 5.5.0 is out.

ref: https://collectd.org/wiki/index.php/Version_5.5
Comment 1 Krzysztof 2015-07-13 07:21:36 UTC
Yes, I knwo that there is new version. I'm working on to make new version. Please be patient :-)))
Comment 2 elij 2015-07-13 21:44:33 UTC
Thanks for the update!
I searched for an open bug first, but didn't find one. Hopefully having this one will help anyone looking in the future, and possibly prevent duplicate notifications. :)
Comment 3 Krzysztof 2015-07-14 11:44:09 UTC
Created attachment 158744 [details]
collectd5 patch for upgrade to version 5.5.0

I've made a patch for port which upgrades collectd to version 5.5.0. Patch was tested by poudriere. You can see the log: https://rtm.bsdserwis.com/poudriere/data/a1amd64-testing/2015-07-14_13h34m13s/logs/collectd5-5.5.0.log.
Comment 4 Krzysztof 2015-07-14 11:45:07 UTC
You can download patch directly from private site: http://prv.wtp3.org/collectd5.cummulative.patch
Comment 5 elij 2015-07-16 08:08:44 UTC
I get a failure when trying to build with the supplied cumulative patch.
poudriere output: https://gist.github.com/cactus/c4a3e77341ce7db93ce0
Comment 6 elij 2015-07-16 08:15:03 UTC
If relevant, the output of patching is here: https://gist.github.com/cactus/c8a86cd8da9f506ca5ca
Comment 7 Krzysztof 2015-07-16 09:37:12 UTC
It looks strange for me. As you can find patch for curl_xml.c (it is removed from files/ subdirectory). I will check it again. Maybe I've made a mistake somewhere...
Comment 8 elij 2015-07-16 17:34:34 UTC
I do end up with an empty patch-src-curl_xml.c file, but the file did not get removed. Odd indeed! I will try manually removing that now empty file and see if it works.
Comment 9 elij 2015-07-16 18:15:33 UTC
I added -E to my patch command, and it looks like it is doing the right thing now. I am now getting a different error when building the port.

poudriere output: https://gist.github.com/cactus/ce5db6302dd9b56dcfb3

My guess is that there is some issue with my configured options and a dependency or something.
Comment 10 Krzysztof 2015-07-16 20:02:14 UTC
Well, I've tested this path with (almost) all plugins selected. In your output there is dependency error on log_logstash. According to collectd documentation (https://collectd.org/wiki/index.php/Table_of_Plugins) this plugin was added to this version. As you can see there is no dedicated documentation to this plugin except collectd.conf.

So I have to check what is dependency to build this plugin.

Also: which options did you select?
Comment 11 elij 2015-07-16 20:06:22 UTC
Here is my poudriere options file content that I use for collectd5. I made no additional changes for the 5.5 version.

----8<----
# This file is auto-generated by 'make config'.
# Options for collectd5-5.4.2_2
_OPTIONS_READ=collectd5-5.4.2_2
_FILE_COMPLETE_OPTIONS_LIST=CGI DEBUG GCRYPT VIRT CURL DBI JSON MEMCACHEC MODBUS MONGODB MYSQL  NUTUPS PERL PGSQL PING PYTHON RABBITMQ REDIS  ROUTEROS SIGROK SNMP STATGRAB TOKYOTYRANT VARNISH  XML XMMS RRDTOOL NOTIFYEMAIL NOTIFYDESKTOP RIEMANN
OPTIONS_FILE_UNSET+=CGI
OPTIONS_FILE_UNSET+=DEBUG
OPTIONS_FILE_UNSET+=GCRYPT
OPTIONS_FILE_UNSET+=VIRT
OPTIONS_FILE_UNSET+=CURL
OPTIONS_FILE_UNSET+=DBI
OPTIONS_FILE_UNSET+=JSON
OPTIONS_FILE_UNSET+=MEMCACHEC
OPTIONS_FILE_UNSET+=MODBUS
OPTIONS_FILE_UNSET+=MONGODB
OPTIONS_FILE_UNSET+=MYSQL
OPTIONS_FILE_UNSET+=NUTUPS
OPTIONS_FILE_UNSET+=PERL
OPTIONS_FILE_UNSET+=PGSQL
OPTIONS_FILE_UNSET+=PING
OPTIONS_FILE_UNSET+=PYTHON
OPTIONS_FILE_UNSET+=RABBITMQ
OPTIONS_FILE_UNSET+=REDIS
OPTIONS_FILE_UNSET+=ROUTEROS
OPTIONS_FILE_UNSET+=SIGROK
OPTIONS_FILE_SET+=SNMP
OPTIONS_FILE_SET+=STATGRAB
OPTIONS_FILE_UNSET+=TOKYOTYRANT
OPTIONS_FILE_UNSET+=VARNISH
OPTIONS_FILE_UNSET+=XML
OPTIONS_FILE_UNSET+=XMMS
OPTIONS_FILE_UNSET+=RRDTOOL
OPTIONS_FILE_UNSET+=NOTIFYEMAIL
OPTIONS_FILE_UNSET+=NOTIFYDESKTOP
OPTIONS_FILE_UNSET+=RIEMANN
----8<----
Comment 12 elij 2015-07-16 20:09:31 UTC
looking at the configure.ac file upstream (https://github.com/collectd/collectd/blob/master/configure.ac) it appears that libyaml is required for logstash (likely json support?).

if test "x$with_libyajl" = "xyes"
then
	plugin_log_logstash="yes"
fi


I will try updating my options and disabling logstash as I won't be using it anyway.
Comment 13 elij 2015-07-16 20:10:00 UTC
Meant libyajl instead of libyaml in my previous comment.
Comment 14 Krzysztof 2015-07-16 20:20:21 UTC
(In reply to elij from comment #12)
OK, my mistake. So I have to modify Makefile dependencies. And because I have all plugins selected libyaml was included :-)))

Thanks a lot for your help
Comment 15 elij 2015-07-16 20:24:01 UTC
https://github.com/collectd/collectd/blob/master/README#L836

Indeed. Building without logstash appears to have worked.

Not sure if this is exactly the right patch to make it optional...

-----8<-----

diff --git a/net-mgmt/collectd5/Makefile b/net-mgmt/collectd5/Makefile
index dfcec28..2c81c15 100644
--- a/net-mgmt/collectd5/Makefile
+++ b/net-mgmt/collectd5/Makefile
@@ -42,6 +42,7 @@ MYSQL_DESC=		Enable mysql-based plugins
 NOTIFYEMAIL_DESC=	Enable notifications via email
 NOTIFYDESKTOP_DESC=	Enable desktop notifications
 NUTUPS_DESC=		Enable nut (ups) plugin
+LOGSTASH_DESC=		Enable logstash plugin
 OLRSD_DESC=		Enable olsrd plugin
 ONEWIRE_DESC=		Eanble onewire plugin (via owfs)
 OPENLDAP_DESC=		Enable OpenLDAP plugin
@@ -119,7 +120,6 @@ CONFIGURE_ARGS+=	\
 		--enable-filecount \
 		--enable-load \
 		--enable-logfile \
-		--enable-log_logstash \
 		--enable-match_empty_counter \
 		--enable-match_hashed \
 		--enable-match_regex \
@@ -314,6 +314,15 @@ CONFIGURE_ARGS+=--without-libupsclient --disable-nut
 PLIST_SUB+=	NUTUPS="@comment "
 .endif
 
+.if ${PORT_OPTIONS:MLOGSTASH}
+LIB_DEPENDS+=	libyajl.so:${PORTSDIR}/devel/yajl
+CONFIGURE_ARGS+=--enable-log_logstash
+PLIST_SUB+=	LOGSTASH=""
+.else
+CONFIGURE_ARGS+=--disable-log_logstash
+PLIST_SUB+=	LOGSTASH="@comment "
+.endif
+
 .if ${PORT_OPTIONS:MOLSRD}
 CONFIGURE_ARGS+=--enable-olsrd
 PLIST_SUB+=	OLSRD=""
diff --git a/net-mgmt/collectd5/pkg-plist b/net-mgmt/collectd5/pkg-plist
index e37bac3..7e6f936 100644
--- a/net-mgmt/collectd5/pkg-plist
+++ b/net-mgmt/collectd5/pkg-plist
@@ -36,7 +36,7 @@ lib/collectd/filecount.so
 %%IPMI%%lib/collectd/ipmi.so
 @dir /var/lib/collectd
 lib/collectd/load.so
-lib/collectd/log_logstash.so
+%%LOGSTASH%%lib/collectd/log_logstash.so
 lib/collectd/logfile.so
 lib/collectd/match_empty_counter.so
 lib/collectd/match_hashed.so

-----8<-----
Comment 16 Krzysztof 2015-07-16 20:41:04 UTC
Created attachment 158871 [details]
collectd5.cummulative.patch

I've made changes (add option for log_logstash plugin). Also I've found typo for OLSRD_DESC. So you can test this new patch. Also you can download this patch from the same URL as previously: http://prv.wtp3.org/collectd5.cummulative.patch
Comment 17 Krzysztof 2015-07-16 20:44:15 UTC
(In reply to elij from comment #15)
It seems that we worked simultaneously :-)))
Comment 18 elij 2015-07-16 20:55:51 UTC
Confirmed that building with your new cumulative patch worked great too.
:)
Comment 19 Jason Unovitch freebsd_committer 2015-07-19 03:13:19 UTC
Created attachment 158964 [details]
collectd5-5.5.0.patch

Nice work on this!  I see there were a handful of QA issues shown with portlint so I've addressed them with the attached revision of your cumulative patch.

WARN: Makefile: [513]: BROKEN messages should not be quoted.
WARN: Makefile: [166]: IGNORE messages should begin with a lowercase letter and end without a period.
WARN: Makefile: no ftp/http mirror in MASTER_SITES for users behind a proxy.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-src__daemon__collectd.h: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-src__daemon__Makefile.am: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-src__dns.c: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-src__Makefile.am: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-src__modbus.c: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.
WARN: /usr/ports/net-mgmt/collectd5/files/patch-version-gen.sh: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' to ensure proper patch format.


The patch I've attached addresses these.  The patches were all standardized with the 'make makepatch' header to silence the noise from those warnings.  The BROKEN message was an easy fix.  The no ftp/http mirror for proxy users was solved with using the https link first followed by the http link.  The only remaining message is unfixable since CGI is a proper acronym and we can't change that.

WARN: Makefile: [167]: IGNORE messages should begin with a lowercase letter and 
end without a period.


Lastly, being an SVN patch, this should cleanly apply and handle the file removals and additions.  I had to run the commands below so this wraps it up with the SVN patch:
svnlite rm --force patch-src-curl_xml.c patch-src__collectd.c patch-src__collectd.h patch-src__disk.c patch-src__users.c patch-src_modbus.c
svnlite add patch-src__daemon__Makefile.am patch-src__daemon__collectd.h patch-src__modbus.c
Comment 20 Jason Unovitch freebsd_committer 2015-07-19 03:15:43 UTC
Created attachment 158965 [details]
Poudriere testport log from 10.1-RELEASE jail

I'm using the 5.5.0 on two hosts and haven't seen any runtime issues.  I use custom options for my hosts.

Poudriere log attached for the default options.  I've also built on the following range in Poudriere:
8.4-RELEASE-p31      amd64
8.4-RELEASE-p31      i386
9.3-RELEASE-p17      amd64
9.3-RELEASE-p17      i386
10.1-RELEASE-p14     amd64
10.1-RELEASE-p14     i386
11.0-CURRENT r284725 amd64
11.0-CURRENT r284725 i386
Comment 21 Krzysztof 2015-07-20 12:52:46 UTC
Comment on attachment 158964 [details]
collectd5-5.5.0.patch

Thank you very much for help with this patch. I see that it is also built without problems. So, this should be added to ports tree - I hope so :-)))
Comment 22 Krzysztof 2015-07-20 12:55:25 UTC
Comment on attachment 158871 [details]
collectd5.cummulative.patch

Bacause jason Unovitch corrected some issues with my patch, so it should be committed (instead of my one)
Comment 23 Jason Unovitch freebsd_committer 2015-07-30 00:50:43 UTC
Hi.

I'm still waiting on my FreeBSD account to get activated.  I opened https://reviews.FreeBSD.org/D3245 for my mentors to review and sign off on.  I intend to commit this as soon as I get access.
Comment 24 Jason Unovitch freebsd_committer 2015-07-30 02:11:51 UTC
(In reply to Krzysztof from comment #22)

Krzysztof,
Can you clarify regarding the removal of /var/lib/collectd in the pkg-plist?  From what I can see it's not being used however a sanity check that this is intended would be appreciated. 

@@ -115,5 +128,4 @@
 man/man5/collectd.conf.5.gz
 man/man5/types.db.5.gz
 @dir /var/db/collectd
-@dir /var/lib/collectd
 @dir /var/lib
Comment 25 Krzysztof 2015-07-31 19:35:56 UTC
(In reply to Jason Unovitch from comment #24)
Well, I tried to remove /var/lib/ from pkg-plist. But it is installed (this directory is in some config variables). After my vacations I will try to find out this directory and remove it completelty from pkg-plist.

Greetings from Italy :-)))
Comment 26 Jason Unovitch freebsd_committer 2015-08-02 01:26:20 UTC
Created attachment 159449 [details]
collectd5-5.5.0.patch

(In reply to Krzysztof from comment #25)

Krzysztof,
What do you think of this revision?

Changes:
- Make Varnish 4 support compile
- Make SIGROK support work
- Remove the vestiages of /var/lib/collectd
- Alphabetic sort of new options in OPTIONS_DEFINE and OPTIONS_GROUP_INPUT
- Remove duplicate /var/lib and varnish.so entries in pkg-plist

Details on 1-3 above:

1. For Varnish it came down to updating the cflags in patch/configure.ac

++              with_libvarnish_cflags="-I$withval/include/varnish"
 +              with_libvarnish_libs="-L$withval/lib/varnish -lvarnishapi"

2. For SIGROK we need to get glib-2.0 in our CFLAGS with this in the Makefile

+CFLAGS+=      `pkg-config --cflags glib-2.0`

3. For /var/lib/collectd, the Makefile.am made this. There were also duplicate @dir entries in the pkg-plist for it so that is why didn't break QA

+ install-exec-hook:
+      $(mkinstalldirs) $(DESTDIR)$(localstatedir)/run
+-     $(mkinstalldirs) $(DESTDIR)$(localstatedir)/lib/$(PACKAGE_NAME)
+      $(mkinstalldirs) $(DESTDIR)$(localstatedir)/log
Comment 27 Jason Unovitch freebsd_committer 2015-08-02 01:30:31 UTC
Created attachment 159450 [details]
Poudriere testport log from HEAD jail

This Poudriere testport is with all options except SNMP.  SNMP will only build in a 'bulk' build and fails testport because of an issue with net-mgmt/net-snmp that is outside your control.  I've also did a testport will the default options.

8.4-RELEASE-p31      amd64
8.4-RELEASE-p31      i386
9.3-RELEASE-p17      amd64
9.3-RELEASE-p17      i386
10.1-RELEASE-p14     amd64
10.1-RELEASE-p14     i386
10.2-BETA2           amd64
10.2-BETA2           i386
11.0-CURRENT r284725 amd64
11.0-CURRENT r284725 i386

The issue with net-snmp causes this QA issue.  That port is linking to the base system libarchive.

====> Running Q/A tests (stage-qa)
Error: Bad linking on /usr/lib/libarchive.so.6: please add USES=libarchive
Error: Bad linking on /usr/lib/libarchive.so.6: please add USES=libarchive
*** Error code 1
Comment 28 Jason Unovitch freebsd_committer 2015-08-02 01:35:55 UTC
(In reply to Jason Unovitch from comment #26)

Never mind.  It seems that /var/lib/collectd is quite persistent.  The attached patch doesn't work at run time.

# collectd -T
Error: Unable to change to directory `/var/lib/collectd'.
Comment 29 Jason Unovitch freebsd_committer 2015-08-02 13:13:21 UTC
Created attachment 159457 [details]
collectd5-5.5.0.patch

Revised to handle /var/lib/collectd5 better.

I'm not sure how I feel about this.  There are several places where the /var/lib is hard coded in and this feels more fragile than fixing it upstream.
Comment 30 Krzysztof 2015-08-11 01:40:44 UTC
Comment on attachment 159457 [details]
collectd5-5.5.0.patch

Jason,

You've made a good work. I should think about it earlier :-)))

So I approve this patch. And now this should work correctly.

Greetings,
--
Krzysztof
Comment 31 Krzysztof 2015-08-11 01:42:42 UTC
Comment on attachment 159457 [details]
collectd5-5.5.0.patch

I think I forgot to change flags that I approve this patch...
Comment 32 commit-hook freebsd_committer 2015-08-11 01:49:38 UTC
A commit references this bug:

Author: junovitch
Date: Tue Aug 11 01:48:59 UTC 2015
New revision: 393930
URL: https://svnweb.freebsd.org/changeset/ports/393930

Log:
  net-mgmt/collectd5: update 5.4.2 -> 5.5.0

  PR:		201514
  Submitted by:	ports@bsdserwis.com (maintainer)
  Approved by:	feld (mentor)
  Differential Revision:	https://reviews.freebsd.org/D3245

Changes:
  head/net-mgmt/collectd5/Makefile
  head/net-mgmt/collectd5/distinfo
  head/net-mgmt/collectd5/files/patch-Makefile.am
  head/net-mgmt/collectd5/files/patch-configure.ac
  head/net-mgmt/collectd5/files/patch-src-curl_xml.c
  head/net-mgmt/collectd5/files/patch-src__Makefile.am
  head/net-mgmt/collectd5/files/patch-src__Makefile.in
  head/net-mgmt/collectd5/files/patch-src__collectd.c
  head/net-mgmt/collectd5/files/patch-src__collectd.h
  head/net-mgmt/collectd5/files/patch-src__daemon__Makefile.am
  head/net-mgmt/collectd5/files/patch-src__daemon__collectd.h
  head/net-mgmt/collectd5/files/patch-src__disk.c
  head/net-mgmt/collectd5/files/patch-src__dns.c
  head/net-mgmt/collectd5/files/patch-src__modbus.c
  head/net-mgmt/collectd5/files/patch-src__users.c
  head/net-mgmt/collectd5/files/patch-src_modbus.c
  head/net-mgmt/collectd5/files/patch-version-gen.sh
  head/net-mgmt/collectd5/pkg-descr
  head/net-mgmt/collectd5/pkg-plist
Comment 33 Jason Unovitch freebsd_committer 2015-08-11 01:50:55 UTC
Thank you both for your work! Committed.