Bug 176038 - sysutils/conky: convert to new options framework
Summary: sysutils/conky: convert to new options framework
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 22:20 UTC by Guido Falsi
Modified: 2016-06-12 10:41 UTC (History)
2 users (show)

See Also:


Attachments
file.diff (7.63 KB, patch)
2013-02-11 22:20 UTC, Guido Falsi
no flags Details | Diff
conky-1.9.0_1.diff (8.82 KB, patch)
2013-02-14 16:59 UTC, ntarmos
no flags Details | Diff
Add implicit dependencies between options and use CONFIGURE_ENABLE (2.24 KB, patch)
2016-06-11 09:53 UTC, Fernando Herrero Carrón
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guido Falsi freebsd_committer 2013-02-11 22:20:00 UTC
I made patches to make the following changes to the sysutils/conky
and sysutils/conky-awesome ports you maintain:

- Convert to new options framework
- Use USE_PKGCONFIG instead of USE_GNOME=pkgconfig
- Remove ABI version numbers from LIB_DEPENDS
- Trim Makefile headers on sysutils/conky-awesome

Reviewd by:	bapt

Can you revise this patch and approve it?

Thanks.

Maintainer is CCed.
Comment 1 Edwin Groothuis freebsd_committer 2013-02-11 22:20:09 UTC
Maintainer of sysutils/conky,

Please note that PR ports/176038 has just been submitted.

If it contains a patch for an upgrade, an enhancement or a bug fix
you agree on, reply to this email stating that you approve the patch
and a committer will take care of it.

The full text of the PR can be found at:
    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/176038

-- 
Edwin Groothuis via the GNATS Auto Assign Tool
edwin@FreeBSD.org
Comment 2 Edwin Groothuis freebsd_committer 2013-02-11 22:20:10 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Comment 3 Guido Falsi freebsd_committer 2013-02-11 22:24:05 UTC
Responsible Changed
From-To: freebsd-ports-bugs->madpilot

I'll take it.
Comment 4 ntarmos 2013-02-14 16:59:35 UTC
Here's a revised patch.

\n\n
Comment 5 Guido Falsi freebsd_committer 2013-02-14 21:00:21 UTC
State Changed
From-To: feedback->closed

Committed. Thanks!
Comment 6 dfilter service freebsd_committer 2013-02-14 21:00:26 UTC
Author: madpilot
Date: Thu Feb 14 21:00:12 2013
New Revision: 312241
URL: http://svnweb.freebsd.org/changeset/ports/312241

Log:
  - Convert to new options framework [1]
  - Use USE_PKGCONFIG instead of USE_GNOME=pkgconfig [1]
  - Remove ABI version numbers from LIB_DEPENDS [1]
  - Trim Makefile headers on sysutils/conky-awesome [1]
  - Update maintainer address [2]
  - Add new options [2]
  - Bump PORTREVISION [2]
  
  PR:		ports/176038 [1]
  Submitted by:	myself
  Reviewed by:	bapt [1]
  Approved by:	Nikos Ntarmos <ntarmos@ceid.upatras.gr> (maintainer) [2]

Modified:
  head/sysutils/conky-awesome/Makefile   (contents, props changed)
  head/sysutils/conky/Makefile   (contents, props changed)

Modified: head/sysutils/conky-awesome/Makefile
==============================================================================
--- head/sysutils/conky-awesome/Makefile	Thu Feb 14 20:51:10 2013	(r312240)
+++ head/sysutils/conky-awesome/Makefile	Thu Feb 14 21:00:12 2013	(r312241)
@@ -1,13 +1,9 @@
-# New ports collection makefile for:	conky
-# Date created:				2005-08-27
-# Whom:					Roman Bogorodskiy <novel@FreeBSD.org>
-#
+# Created by: Roman Bogorodskiy <novel@FreeBSD.org>
 # $FreeBSD$
-#
 
 PKGNAMESUFFIX=	-awesome
 
-MAINTAINER=	ntarmos@cs.uoi.gr
+MAINTAINER=	ntarmos@ceid.upatras.gr
 COMMENT=	An advanced, highly configurable system monitor (configured for x11-wm/awesome)
 
 MASTERDIR=	${.CURDIR}/../conky
@@ -15,6 +11,8 @@ DESCR=		${.CURDIR}/pkg-descr
 
 CONFLICTS=	conky-[0-9]*
 
-USE_XORG=	# This is a comment
+OPTIONS_EXCLUDE=	X11
+OPTIONS_GROUP=
+OPTIONS_DEFAULT=
 
 .include "${MASTERDIR}/Makefile"

Modified: head/sysutils/conky/Makefile
==============================================================================
--- head/sysutils/conky/Makefile	Thu Feb 14 20:51:10 2013	(r312240)
+++ head/sysutils/conky/Makefile	Thu Feb 14 21:00:12 2013	(r312241)
@@ -3,10 +3,11 @@
 
 PORTNAME=	conky
 PORTVERSION=	1.9.0
+PORTREVISION=	1
 CATEGORIES=	sysutils
 MASTER_SITES=	SF
 
-MAINTAINER=	ntarmos@cs.uoi.gr
+MAINTAINER=	ntarmos@ceid.upatras.gr
 COMMENT=	An advanced, highly configurable system monitor for X
 
 CONFLICTS?=	conky-awesome-[0-9]*
@@ -15,14 +16,14 @@ SLAVEDIRS=	sysutils/conky-awesome
 USE_ICONV=	yes
 USE_BZIP2=	yes
 GNU_CONFIGURE=	yes
-USE_XORG?=	x11 xext xdamage
-USE_GNOME?=	pkgconfig
 USE_ICONV=	yes
 USE_GMAKE=	yes
+USE_PKGCONFIG=	build
 CONFIGURE_ARGS+=	--disable-portmon \
 			--disable-hddtemp \
 			--disable-alsa \
-			--disable-bmpx
+			--disable-bmpx \
+			--disable-iostats
 CPPFLAGS+=	-I${LOCALBASE}/include
 LDFLAGS+=	-L${LOCALBASE}/lib
 
@@ -32,130 +33,156 @@ PORTEXAMPLES=	conkyrc.sample
 PORTDOCS=	README AUTHORS ChangeLog TODO NEWS \
 		docs.html variables.html config_settings.html
 
-OPTIONS+=	APCUPSD "Enable APCUPSD support" Off \
-		AUDACIOUS "Enable Audacious support" Off \
-		INOTIFY "Enable inotify support" Off \
-		MPD "Enable MPD support" Off \
-		NCURSES "Enable ncurses support" Off \
-		RSS "Enable RSS support" Off \
-		METAR "Enable METAR Weather support" Off \
-		XOAP "Enable XOAP Weather support" Off \
-		XMMS2 "Enable XMMS2 support" Off
-
-.if !empty(USE_XORG)
-OPTIONS+=	DOUBLE_BUFFER "Enable double buffering" On \
-		IMLIB2 "Enable Imlib2 support" Off \
-		LUA "Enable Lua support" Off \
-		LUA_CAIRO "Enable Lua-Cairo binding (impl. Lua)" Off \
-		LUA_IMLIB2 "Enable Lua-Imlib2 binding (impl. Lua/Imlib2)" Off \
-		XFT "Enable Xft support" Off
-.else
-OPTIONS+=	LUA "Enable Lua support" Off
-.endif
+OPTIONS_DEFINE=	APCUPSD AUDACIOUS INOTIFY LUA METAR MOC MPD NCURSES RSS \
+		X11 XMMS2 XOAP
 
-.include <bsd.port.pre.mk>
-
-.if empty(USE_XORG)
-WITHOUT_DOUBLE_BUFFER=	On
+OPTIONS_GROUP?=	X11
+OPTIONS_GROUP_X11=	ARGB DOUBLE_BUFFER IMLIB2 XFT LUA_CAIRO LUA_IMLIB2
+OPTIONS_DEFAULT?=	X11 ARGB DOUBLE_BUFFER
+
+ARGB_DESC=		Use an ARGB visual to draw on X11
+APCUPSD_DESC=		Monitor APCUPSD
+AUDACIOUS_DESC=		Control Audacious sound player
+DOUBLE_BUFFER_DESC=	Enable X11 double buffering
+INOTIFY_DESC=		Monitor file changes via Inotify
+LUA_CAIRO_DESC=		Lua-Cairo binding
+LUA_IMLIB2_DESC=	Lua-Imlib2 binding
+METAR_DESC=		Display METAR weather reports
+MOC_DESC=		Control MOC (Music On Console)
+MPD_DESC=		Control MPD (Music Player Daemon)
+NCURSES_DESC=		Use ncurses to draw on terminals
+RSS_DESC=		Display RSS feeds
+XMMS2_DESC=		Control XMMS2 media player
+XOAP_DESC=		Display XOAP weather reports
+
+.include <bsd.port.options.mk>
+
+.if ${PORT_OPTIONS:MARGB} || ${PORT_OPTIONS:MDOUBLE_BUFFER} || ${PORT_OPTIONS:MIMLIB2} || ${PORT_OPTIONS:MLUA_CAIRO} || ${PORT_OPTIONS:MLUA_IMLIB2} || ${PORT_OPTIONS:MXFT} || ${PORT_OPTIONS:MX11}
+USE_XORG=	x11 xext xdamage
+CONFIGURE_ARGS+=	--enable-x11 --enable-own-window
+EXAMPLE_CONF_FILE=	${WRKSRC}/data/conky.conf
+.else
 CONFIGURE_ARGS+=	--disable-x11 --disable-own-window
-.undef WITH_IMLIB2
-.undef WITH_LUA_CAIRO
-.undef WITH_LUA_IMLIB2
-.undef WITH_XFT
+EXAMPLE_CONF_FILE=	${WRKSRC}/data/conky_no_x11.conf
 .endif
 
-.if defined(WITH_APCUPSD)
-RUN_DEPENDS+=		${LOCALBASE}/sbin/apcupsd:${PORTSDIR}/sysutils/apcupsd
+.if ${PORT_OPTIONS:MAPCUPSD}
 CONFIGURE_ARGS+=	--enable-apcupsd
 .else
 CONFIGURE_ARGS+=	--disable-apcupsd
 .endif
 
-.if defined(WITH_AUDACIOUS)
-LIB_DEPENDS+=		audclient.2:${PORTSDIR}/multimedia/audacious
+.if ${PORT_OPTIONS:MARGB}
+CONFIGURE_ARGS+=	--enable-argb
+.else
+CONFIGURE_ARGS+=	--disable-argb
+.endif
+
+.if ${PORT_OPTIONS:MAUDACIOUS}
+LIB_DEPENDS+=		audclient:${PORTSDIR}/multimedia/audacious
 CONFIGURE_ARGS+=	--enable-audacious
 .else
 CONFIGURE_ARGS+=	--disable-audacious
 .endif
 
-.if defined(WITH_INOTIFY)
-LIB_DEPENDS+=		inotify.0:${PORTSDIR}/devel/libinotify
-CONFIGURE_ARGS+=	--enable-inotify
+.if ${PORT_OPTIONS:MDOUBLE_BUFFER}
+CONFIGURE_ARGS+=	--enable-double-buffer
 .else
-CONFIGURE_ARGS+=	--disable-inotify
+CONFIGURE_ARGS+=	--disable-double-buffer
 .endif
 
-.if defined(WITH_LUA) || defined(WITH_LUA_CAIRO) || defined(WITH_LUA_IMLIB2)
-USE_LUA=		5.1+
-CONFIGURE_ARGS+=	--enable-lua
+.if ${PORT_OPTIONS:MIMLIB2} || ${PORT_OPTIONS:MLUA_IMLIB2}
+LIB_DEPENDS+=		Imlib2:${PORTSDIR}/graphics/imlib2
+CONFIGURE_ARGS+=	--enable-imlib2
 .else
-CONFIGURE_ARGS+=	--disable-lua
+CONFIGURE_ARGS+=	--disable-imlib2
 .endif
 
-.if defined(WITH_LUA_CAIRO)
-LIB_DEPENDS+=		cairo.2:${PORTSDIR}/graphics/cairo
+.if ${PORT_OPTIONS:MINOTIFY}
+LIB_DEPENDS+=		inotify:${PORTSDIR}/devel/libinotify
+CONFIGURE_ARGS+=	--enable-inotify
+.else
+CONFIGURE_ARGS+=	--disable-inotify
+.endif
+
+.if ${PORT_OPTIONS:MLUA_CAIRO}
+LIB_DEPENDS+=		cairo:${PORTSDIR}/graphics/cairo
 BUILD_DEPENDS+=		tolua++-5.1:${PORTSDIR}/lang/tolua++
 RUN_DEPENDS+=		tolua++-5.1:${PORTSDIR}/lang/tolua++
 CONFIGURE_ARGS+=	--enable-lua-cairo
-.endif
-
-.if defined(WITH_IMLIB2) || defined(WITH_LUA_IMLIB2)
-LIB_DEPENDS+=		Imlib2.5:${PORTSDIR}/graphics/imlib2
-CONFIGURE_ARGS+=	--enable-imlib2
 .else
-CONFIGURE_ARGS+=	--disable-imlib2
+CONFIGURE_ARGS+=	--disable-lua-cairo
 .endif
 
-.if defined(WITH_LUA_IMLIB2)
+.if ${PORT_OPTIONS:MLUA_IMLIB2}
 BUILD_DEPENDS+=		tolua++-5.1:${PORTSDIR}/lang/tolua++
 RUN_DEPENDS+=		tolua++-5.1:${PORTSDIR}/lang/tolua++
 CONFIGURE_ARGS+=	--enable-lua-imlib2
+.else
+CONFIGURE_ARGS+=	--disable-lua-imlib2
 .endif
 
-.if defined(WITH_NCURSES)
-CONFIGURE_ARGS+=	--enable-ncurses
+.if ${PORT_OPTIONS:MLUA} || ${PORT_OPTIONS:MLUA_CAIRO} || ${PORT_OPTIONS:MLUA_IMLIB2}
+USE_LUA=		5.1+
+CONFIGURE_ARGS+=	--enable-lua
 .else
-CONFIGURE_ARGS+=	--disable-ncurses
+CONFIGURE_ARGS+=	--disable-lua
 .endif
 
-.if defined(WITH_XFT)
-LIB_DEPENDS+=		Xft.2:${PORTSDIR}/x11-fonts/libXft
-CONFIGURE_ARGS+=	--enable-xft
+.if ${PORT_OPTIONS:MMETAR}
+LIB_DEPENDS+=		curl:${PORTSDIR}/ftp/curl
+CONFIGURE_ARGS+=	--enable-weather-metar
 .else
-CONFIGURE_ARGS+=	--disable-xft
+CONFIGURE_ARGS+=	--disable-weather-metar
 .endif
 
-.if defined(WITHOUT_MPD)
+.if ${PORT_OPTIONS:MMOC}
+CONFIGURE_ARGS+=	--enable-moc
+.else
+CONFIGURE_ARGS+=	--disable-moc
+.endif
+
+.if ${PORT_OPTIONS:MMPD}
+CONFIGURE_ARGS+=	--enable-mpd
+.else
 CONFIGURE_ARGS+=	--disable-mpd
 .endif
 
-.if defined(WITH_RSS)
-LIB_DEPENDS+=		curl.6:${PORTSDIR}/ftp/curl \
-			xml2.5:${PORTSDIR}/textproc/libxml2
-CONFIGURE_ARGS+=	--enable-rss
-USE_GNOME+=		glib20
+.if ${PORT_OPTIONS:MNCURSES}
+CONFIGURE_ARGS+=	--enable-ncurses
+.else
+CONFIGURE_ARGS+=	--disable-ncurses
 .endif
 
-.if defined(WITH_METAR)
-LIB_DEPENDS+=		curl.6:${PORTSDIR}/ftp/curl
-CONFIGURE_ARGS+=	--enable-weather-metar
+.if ${PORT_OPTIONS:MRSS}
+LIB_DEPENDS+=		curl:${PORTSDIR}/ftp/curl \
+			xml2:${PORTSDIR}/textproc/libxml2
+CONFIGURE_ARGS+=	--enable-rss
+USE_GNOME+=		glib20
+.else
+CONFIGURE_ARGS+=	--disable-rss
 .endif
 
-.if defined(WITH_XOAP)
-LIB_DEPENDS+=		curl.6:${PORTSDIR}/ftp/curl \
-			xml2.5:${PORTSDIR}/textproc/libxml2
-CONFIGURE_ARGS+=	--enable-weather-xoap
+.if ${PORT_OPTIONS:MXFT}
+LIB_DEPENDS+=		Xft:${PORTSDIR}/x11-fonts/libXft
+CONFIGURE_ARGS+=	--enable-xft
+.else
+CONFIGURE_ARGS+=	--disable-xft
 .endif
 
-.if defined(WITH_XMMS2)
-LIB_DEPENDS+=		xmmsclient.6:${PORTSDIR}/audio/xmms2
+.if ${PORT_OPTIONS:MXMMS2}
+LIB_DEPENDS+=		xmmsclient:${PORTSDIR}/audio/xmms2
 CONFIGURE_ARGS+=	--enable-xmms2
 .else
 CONFIGURE_ARGS+=	--disable-xmms2
 .endif
 
-.if defined(WITHOUT_DOUBLE_BUFFER)
-CONFIGURE_ARGS+=	--disable-double-buffer
+.if ${PORT_OPTIONS:MXOAP}
+LIB_DEPENDS+=		curl:${PORTSDIR}/ftp/curl \
+			xml2:${PORTSDIR}/textproc/libxml2
+CONFIGURE_ARGS+=	--enable-weather-xoap
+.else
+CONFIGURE_ARGS+=	--disable-weather-xoap
 .endif
 
 post-patch:
@@ -169,16 +196,12 @@ do-install:
 	${INSTALL_MAN} ${WRKSRC}/doc/conky.1 ${PREFIX}/man/man1
 
 post-install:
-.if !defined(NOPORTEXAMPLES)
+.if ${PORT_OPTIONS:MEXAMPLES}
 	@${MKDIR} ${EXAMPLESDIR}
-.	if !empty(USE_XORG)
-	@${INSTALL_DATA} ${WRKSRC}/data/conky.conf ${EXAMPLESDIR}/conkyrc.sample
-.	else
-	@${INSTALL_DATA} ${WRKSRC}/data/conky_no_x11.conf ${EXAMPLESDIR}/conkyrc.sample
-.	endif
+	@${INSTALL_DATA} ${EXAMPLE_CONF_FILE} ${EXAMPLESDIR}/conkyrc.sample
 .endif
 
-.if !defined(NOPORTDOCS)
+.if ${PORT_OPTIONS:MDOCS}
 	@${MKDIR} ${DOCSDIR}
 .for i in README AUTHORS ChangeLog TODO NEWS
 	${INSTALL_DATA} ${WRKSRC}/${i} ${DOCSDIR}
@@ -188,4 +211,4 @@ post-install:
 .endfor
 .endif
 
-.include <bsd.port.post.mk>
+.include <bsd.port.mk>
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 7 Fernando Herrero Carrón 2016-06-11 09:53:45 UTC
Created attachment 171300 [details]
Add implicit dependencies between options and use CONFIGURE_ENABLE

Sorry for reopening this PR. Seems to fit my purpose.

I am adding explicit dependencies between options and simplifying "enable" options.
Comment 8 Guido Falsi freebsd_committer 2016-06-11 13:58:57 UTC
(In reply to elferdo from comment #7)
> Created attachment 171300 [details]
> Add implicit dependencies between options and use CONFIGURE_ENABLE
> 
> Sorry for reopening this PR. Seems to fit my purpose.
> 
> I am adding explicit dependencies between options and simplifying "enable"
> options.

You should really have opened a new PR. This one was closed and the description does not match what you are proposing.

Also, reopening old PRs has the risk of the information being lost. The status of the PR is still closed.

Can you please open a new one with the same attachment and notify me via email if you want me to take it?

Really reopening old PRs for mildly related changes isn't a good practice.
Comment 9 Fernando Herrero Carrón 2016-06-12 09:12:20 UTC
Thanks for your reply, Guido.

I agree that reopening a closed PR is bad practice, but I don't agree that my changes are "mildly related" to the original title. In fact, my patch does not add any new functionality, it just converts options handling to conform to the porter's handbook recommendations, a.k.a. how to use the new options framework. In my opinion, worse than reopening a closed PR is having duplicated PRs.

That said, if you still feel that my patch deserves a new PR I will submit one.
Comment 10 Guido Falsi freebsd_committer 2016-06-12 10:41:27 UTC
(In reply to elferdo from comment #9)
> Thanks for your reply, Guido.
> 
> I agree that reopening a closed PR is bad practice, but I don't agree that
> my changes are "mildly related" to the original title. In fact, my patch
> does not add any new functionality, it just converts options handling to
> conform to the porter's handbook recommendations, a.k.a. how to use the new
> options framework. In my opinion, worse than reopening a closed PR is having
> duplicated PRs.

My comment was not about your patch. Even if it did just fix a bug introduced with the commit generated by this PR I'd have asked you to submit a new PR. That's the way PRs have been managed in FreeBSD historically.

I also suggested you to do that in the future because I just noticed this submission by chance. Followup to a closed PR has an high chance of being ignored and is discouraged.

It's not a technical matter, just a matter of policy.

> 
> That said, if you still feel that my patch deserves a new PR I will submit
> one.

Please do if it's not a problem for you.

Thanks.