net-irc/znc: Update/rewrite ebuild for 9999 #3236

DarthGandalf · 2016-12-24T20:52:19Z

Copy from znc-1.6.4.ebuild at PR net-irc/znc: Set defaults for several USE-flags to enabled. #3232 as the base
Drop custom patches, redo the same functionality using a new internal
feature in upstream ZNC, and using start-stop-daemon (Patch set from Gentoo znc/znc#1245).
Switch to CMake
Add a new feature from upstream with nls USE-flag
Add 9999 sauce on top, from the old znc-9999.ebuild and from znc-1.4-r1.ebuild
Test (with FEATURES=test) as 9999, and as one of nightly tarballs

@Whissi

gentoo-repo-qa-bot · 2016-12-24T20:56:03Z

Pull Request assignment

Areas affected: ebuilds
Packages affected: net-irc/znc

net-irc/znc: @sbraz, @gentoo/proxy-maint

sbraz · 2016-12-24T21:16:01Z

Could you provide a diff between the old and the new ebuilds/init scripts?

DarthGandalf · 2016-12-24T21:26:29Z

--- znc-1.6.4.ebuild	2016-12-24 20:34:18.431691487 +0000
+++ znc-9999.ebuild	2016-12-24 20:35:39.064208856 +0000
@@ -5,28 +5,43 @@
 EAPI=6
 
 PYTHON_COMPAT=( python{3_4,3_5} )
-inherit eutils python-single-r1 readme.gentoo-r1 systemd user
+PLOCALES="ru"
+CMAKE_MAKEFILE_GENERATOR=ninja
 
-MY_PV=${PV/_/-}
-GTEST_VER="1.7.0"
+inherit cmake-utils l10n python-single-r1 readme.gentoo-r1 systemd user
+
+GTEST_VER="1.8.0"
 GTEST_URL="https://github.com/google/googletest/archive/release-${GTEST_VER}.tar.gz -> googletest-release-${GTEST_VER}.tar.gz"
 DESCRIPTION="An advanced IRC Bouncer"
 
-SRC_URI="
-	http://znc.in/releases/archive/${PN}-${MY_PV}.tar.gz
-	test? ( ${GTEST_URL} )
-"
-KEYWORDS="~amd64 ~arm ~x86"
+if [[ ${PV} == *9999* ]]; then
+	inherit git-r3
+	EGIT_REPO_URI=${EGIT_REPO_URI:-"git://github.com/znc/znc.git"}
+	SRC_URI=""
+	KEYWORDS=""
+	SWIG_DEPEND="
+		perl? ( >=dev-lang/swig-3.0.0 )
+		python? ( >=dev-lang/swig-3.0.0 )
+	"
+else
+	SRC_URI="
+		http://znc.in/releases/archive/${P}.tar.gz
+		test? ( ${GTEST_URL} )
+	"
+	KEYWORDS="~amd64 ~arm ~x86"
+	SWIG_DEPEND=""
+fi
 
 HOMEPAGE="http://znc.in"
 LICENSE="Apache-2.0"
 SLOT="0"
-IUSE="daemon debug ipv6 icu libressl perl python ssl sasl tcl test zlib"
+IUSE="daemon +ipv6 +icu libressl nls perl python +ssl sasl tcl test zlib"
 
-REQUIRED_USE="python? ( ${PYTHON_REQUIRED_USE} )"
+REQUIRED_USE="python? ( ${PYTHON_REQUIRED_USE} icu )"
 
 RDEPEND="
 	icu? ( dev-libs/icu:= )
+	nls? ( dev-libs/boost[nls] )
 	perl? ( >=dev-lang/perl-5.10:= )
 	python? ( ${PYTHON_DEPS} )
 	sasl? ( >=dev-libs/cyrus-sasl-2 )
@@ -39,16 +54,11 @@ RDEPEND="
 "
 DEPEND="
 	${RDEPEND}
+	${SWIG_DEPEND}
+	nls? ( sys-devel/gettext )
 	virtual/pkgconfig
 "
 
-S=${WORKDIR}/${PN}-${MY_PV}
-
-PATCHES=(
-	"${FILESDIR}"/${PN}-1.6.1-systemwideconfig.patch
-	"${FILESDIR}"/${PN}-1.6.1-create-pidfile-per-default.patch
-)
-
 pkg_setup() {
 	if use python; then
 		python-single-r1_pkg_setup
@@ -63,26 +73,55 @@ pkg_setup() {
 	fi
 }
 
+src_prepare() {
+	l10n_find_plocales_changes "${S}/src/po" "${PN}." '.po'
+	remove_locale() {
+		# Some languages can miss some modules
+		rm src/po/${PN}.${1}.po modules/po/*.${1}.po || true
+	}
+	l10n_for_each_disabled_locale_do remove_locale
+	cmake-utils_src_prepare
+}
+
 src_configure() {
-	econf \
-		--with-systemdsystemunitdir="$(systemd_get_systemunitdir)" \
-		$(use_enable debug) \
-		$(use_enable icu charset) \
-		$(use_enable ipv6) \
-		$(use_enable perl) \
-		$(use_enable python) \
-		$(use_enable sasl cyrus) \
-		$(use_enable ssl openssl) \
-		$(use_enable tcl) \
-		$(use_enable zlib) \
-		$(use_with test gtest "${WORKDIR}/googletest-release-${GTEST_VER}")
+	local want_swig
+	if [[ ${PV} == *9999* ]]; then
+		want_swig=yes
+	else
+		want_swig=no
+	fi
+	local mycmakeargs=(
+		-DWANT_SYSTEMD="$(usex daemon)"
+		-DSYSTEMD_DIR="$(systemd_get_systemunitdir)"
+		-DWANT_ICU="$(usex icu)"
+		-DWANT_IPV6="$(usex ipv6)"
+		-DWANT_I18N="$(usex nls)"
+		-DWANT_PERL="$(usex perl)"
+		-DWANT_PYTHON="$(usex python)"
+		-DWANT_CYRUS="$(usex sasl)"
+		-DWANT_OPENSSL="$(usex ssl)"
+		-DWANT_TCL="$(usex tcl)"
+		-DWANT_ZLIB="$(usex zlib)"
+		-DWANT_SWIG="${want_swig}"
+	)
+	if use test; then
+		export GTEST_ROOT="${WORKDIR}/googletest-release-${GTEST_VER}/googletest"
+		export GMOCK_ROOT="${WORKDIR}/googletest-release-${GTEST_VER}/googlemock"
+	fi
+	cmake-utils_src_configure
+}
+
+src_test() {
+	pushd "${BUILD_DIR}" > /dev/null || die
+	${CMAKE_MAKEFILE_GENERATOR} unittest || die "Unit test failed"
+	popd > /dev/null || die
 }
 
 src_install() {
-	default
+	cmake-utils_src_install
 	dodoc NOTICE
 	if use daemon; then
-		newinitd "${FILESDIR}"/znc.initd-r1 znc
+		newinitd "${FILESDIR}"/znc.initd-r2 znc
 		newconfd "${FILESDIR}"/znc.confd-r1 znc
 	fi
 	DOC_CONTENTS=$(<"${FILESDIR}/README.gentoo") || die
@@ -117,7 +156,8 @@ pkg_config() {
 			mkdir -p "${EROOT%/}/var/lib/znc" || die
 			chown -R ${PN}:${PN} "${EROOT%/}/var/lib/znc" ||
 				die "Setting permissions failed"
-			"${EROOT%/}"/usr/bin/znc --system-wide-config-as ${PN} -c -r -d "${EROOT%/}/var/lib/znc" ||
+			start-stop-daemon --start --user ${PN}:${PN} --env ZNC_NO_LAUNCH_AFTER_MAKECONF=1 \
+				"${EROOT%/}"/usr/bin/znc -- --makeconf --datadir "${EROOT%/}/var/lib/znc" ||
 				die "Config failed"
 			echo
 			einfo "To start znc, run '/etc/init.d/znc start'"

--- files/znc.initd-r1	2016-12-18 09:33:45.246807812 +0000
+++ files/znc.initd-r2	2016-12-24 20:36:18.638954539 +0000
@@ -6,7 +6,8 @@
 extra_commands="config"
 extra_started_commands="reload save"
 command="/usr/bin/znc"
-command_args="--datadir \"${ZNC_DATADIR}\""
+command_args="--datadir \"${ZNC_DATADIR}\" --foreground"
+command_background="yes"
 pidfile="${ZNC_PIDFILE:-/run/znc/znc.pid}"
 user=${ZNC_USER:-znc}
 group=${ZNC_GROUP:-znc}

* Copy from znc-1.6.4.ebuild at PR gentoo#3232 as the base * Drop custom patches, redo the same functionality using a new internal feature in upstream ZNC, and using start-stop-daemon (znc/znc#1245). * Switch to CMake * Add a new feature from upstream with `nls` USE-flag * Add 9999 sauce on top, from the old znc-9999.ebuild and from znc-1.4-r1.ebuild * Test (with FEATURES=test) as 9999, and as one of nightly tarballs

Whissi

Overall it looks good: It is still building in different configuration and emerge --config and runscript still works.

Whissi · 2016-12-27T00:09:45Z

net-irc/znc/files/znc.initd-r2

@@ -0,0 +1,41 @@
+#!/sbin/openrc-run
+# Copyright 1999-2015 Gentoo Foundation


Or 2017? :)

Whissi · 2016-12-27T00:14:03Z

net-irc/znc/znc-9999.ebuild

+	inherit git-r3
+	EGIT_REPO_URI=${EGIT_REPO_URI:-"git://github.com/znc/znc.git"}
+	SRC_URI=""
+	KEYWORDS=""


Please remove this line: Some scripts from our ATs add their keyword to the first occurrence which would be a problem if this ebuild will be used for a real version in future. So make sure that KEYWORDS exists only once.

Whissi · 2016-12-27T00:15:49Z

net-irc/znc/znc-9999.ebuild

-		$(use_enable tcl tcl)
+	local want_swig
+	if [[ ${PV} == *9999* ]]; then
+		want_swig=yes


Could you please explain why SWIG is only needed for the live ebuild?

Tarballs include files generated by SWIG, so they don't need it.
However, thinking about it more, to allow more flexibility with /etc/portage/patches/net-irc/znc, probably SWIG should be used for releases too, not only for live.

Whissi · 2016-12-27T00:19:36Z

net-irc/znc/znc-9999.ebuild

+		want_swig=no
+	fi
+	local mycmakeargs=(
+		-DWANT_SYSTEMD="$(usex daemon)"


I am not sure about this one. What does this option (WANT_SYSTEMD) control?

Whether to copy the file to $(systemd_get_systemunitdir), or not.
I use OpenRC, and don't know how systemd works, but I think it requires a single znc user. So I don't see any reason to install the service file without ability to run the daemon system-wide (with daemon flag)

Whissi · 2016-12-27T00:21:18Z

net-irc/znc/znc-9999.ebuild

 		newconfd "${FILESDIR}"/znc.confd-r1 znc
 	fi
+	DOC_CONTENTS=$(<"${FILESDIR}/README.gentoo") || die


This one is at least outdated now that we are no longer shipping our --system-wide-config-as parameter.

Removed outdated sections.

Whissi · 2016-12-27T00:22:37Z

net-irc/znc/znc-9999.ebuild

-	else
-		if use daemon; then
-			ewarn "${CONFDIR} already exists, aborting to avoid damaging"
+	if use daemon; then


Depending on your answer before we maybe don't need the daemon USE flag anymore.

Removed it.

Whissi · 2016-12-27T00:24:03Z

net-irc/znc/znc-9999.ebuild

-KEYWORDS=""
+if [[ ${PV} == *9999* ]]; then
+	inherit git-r3
+	EGIT_REPO_URI=${EGIT_REPO_URI:-"git://github.com/znc/znc.git"}


Please always include the HTTPS URI as fallback for firewalled clients.

Whissi · 2016-12-27T00:32:59Z

Additional questions:

Our now removed patch ensured that we added PidFile option. This is not needed anymore because now we run in foreground and let start-stop-daemon do the magic.

I am unsure how it works for systemd folks. Like said I don't understand the systemd option yet which depends on the daemon USE flag.

Gentoo-unrelated: I noticed that znc's configuration generator doesn't recommend to enable SSL per default:

[ ?? ] Listen using SSL (yes/no) [no]:

Judging from your comment in #3156 (comment) this is probably something you want to change in znc upstream.

DarthGandalf · 2016-12-27T01:15:15Z

I am unsure how it works for systemd folks.

I can't answer that. The service file was added upstream by Fedora maintainers. I don't know how they use it. Some distros (Debian AFAIR) use their own service file for ZNC, but I again don't know much about how it works.

Well, I don't care much about systemd, and would prefer to not have that file upstream at all, unless it can possibly be used by everyone.

doesn't recommend to enable SSL per default

That's true... but it's harder for the listening side. The default cert is self-signed, which would only show the warnings to client and browser.

DarthGandalf · 2016-12-27T08:47:25Z

net-irc/znc/znc-9999.ebuild

-inherit autotools eutils python-single-r1 user
+PYTHON_COMPAT=( python{3_4,3_5} )
+PLOCALES="ru"
+CMAKE_MAKEFILE_GENERATOR=ninja


Does emerge guarantee no incremental rebuild?
If it doesn't, ninja won't work reliably because only makefiles generator supports IMPLICIT_DEPENDS, znc/znc@106a36c

Well, not always if the user opts out (user can disable cleaning, ignore/bypass ebuild phase locks) but by default this isn't a problem and we don't care.

DarthGandalf · 2016-12-28T13:55:40Z

I see. Is there any actual benefit for ninja over make in the default case? 2016-12-28 13:39 GMT+00:00 Thomas Deutschmann <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In net-irc/znc/znc-9999.ebuild <#3236>: > -PYTHON_COMPAT=( python3_4 ) -inherit autotools eutils python-single-r1 user +PYTHON_COMPAT=( python{3_4,3_5} ) +PLOCALES="ru" +CMAKE_MAKEFILE_GENERATOR=ninja Well, not always if the user opts out (user can disable cleaning, ignore/bypass ebuild phase locks) but by default this isn't a problem and we don't care. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3236>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAT15DpCAbsucEkig_kQs2FRSWftYSw-ks5rMmavgaJpZM4LVSyG> .

Whissi · 2016-12-28T14:12:22Z

See https://archives.gentoo.org/gentoo-dev/message/4c7bcb6e01d32052fb8c7b92070e3ead

So we should probably do

: ${CMAKE_MAKEFILE_GENERATOR:=Ninja}

DarthGandalf · 2016-12-28T20:17:10Z

Done.

SoapGentoo · 2016-12-29T08:51:38Z

net-irc/znc/znc-9999.ebuild

-inherit autotools eutils python-single-r1 user
+PYTHON_COMPAT=( python{3_4,3_5} )
+PLOCALES="ru"
+: ${CMAKE_MAKEFILE_GENERATOR:=ninja}


why make this choice for the user?

I copied it from other ebuilds.

gentoo/dev-util/creduce/creduce-2.6.0.ebuild

Line 7 in ca375fd

: ${CMAKE_MAKEFILE_GENERATOR=ninja}

gentoo/sci-chemistry/votca-csgapps/votca-csgapps-1.4.ebuild

Line 7 in 4a89b21

CMAKE_MAKEFILE_GENERATOR="ninja"

SoapGentoo · 2016-12-29T08:52:19Z

net-irc/znc/znc-9999.ebuild

-	sys-libs/zlib
-	perl? ( >=dev-lang/perl-5.10 )
+	icu? ( dev-libs/icu:= )
+	nls? ( dev-libs/boost[nls] )


boost always needs :=

I see many ebuilds without it :\
Done.

SoapGentoo · 2016-12-29T08:53:28Z

net-irc/znc/znc-9999.ebuild

+	l10n_find_plocales_changes "${S}/src/po" "${PN}." '.po'
+	remove_locale() {
+		# Some language/module pairs can be missing
+		rm src/po/${PN}.${1}.po modules/po/*.${1}.po || true


if they are potentially missing, use rm -f instead and still add a die at the end

SoapGentoo · 2016-12-29T08:53:46Z

net-irc/znc/znc-9999.ebuild

+	# Not "| true" to report an error if names the pregenerated files change upstream.
+	if [[ ${PV} != *9999* ]]; then
+		rm modules/modperl/generated.tar.gz
+		rm modules/modpython/generated.tar.gz


die missing

SoapGentoo · 2016-12-29T08:55:40Z

net-irc/znc/znc-9999.ebuild

+src_test() {
+	pushd "${BUILD_DIR}" > /dev/null || die
+	${CMAKE_MAKEFILE_GENERATOR} unittest || die "Unit test failed"
+	popd > /dev/null || die


not sure, try whether cmake-utils_src_test unittest works instead

No tests found. Skipping.

I didn't use CTest upstream, because I didn't find out how to add the dependency from make test to the actual test target which needs to be compiled.

Related bugs:
http://public.kitware.com/Bug/view.php?id=8438
http://public.kitware.com/Bug/view.php?id=8774

SoapGentoo · 2016-12-29T08:56:45Z

net-irc/znc/znc-9999.ebuild

-		elog
+	readme.gentoo_print_elog
+	if [[ -d "${EROOT%/}"/etc/znc ]]; then
+		ewarn "/etc/znc exists on your system."


Also add ${EROOT%/} to all printed messages

SoapGentoo · 2016-12-29T08:57:27Z

net-irc/znc/znc-9999.ebuild

 			die "Setting permissions failed"
-		"${EROOT}"/usr/bin/znc --system-wide-config-as znc -c -r -d "${EROOT}${CONFDIR}" ||
+		start-stop-daemon --start --user ${PN}:${PN} --env ZNC_NO_LAUNCH_AFTER_MAKECONF=1 \
+			"${EROOT%/}"/usr/bin/znc -- --makeconf --datadir "${EROOT%/}/var/lib/znc" ||
 			die "Config failed"
 		echo


don't mix output streams, just use einfo here too

DarthGandalf · 2017-01-04T08:39:47Z

Your dev-cpp/gtest change doesn't work.
For 9999 version, it uses the version from git submodule instead, for tarball version it fails with

-- GoogleTest not found, testing will be disabled. You can set environment variables GTEST_ROOT and GMOCK_ROOT

because it can't find files gtest-all.cc and gmock-all.cc.

See the discussion at https://bugs.gentoo.org/show_bug.cgi?id=602218 and https://bugs.gentoo.org/show_bug.cgi?id=474454

Whissi · 2017-01-04T12:28:28Z

Mh. OK. I only checked -9999 and it was still building and test output didn't change.

But don't we need to make sure in that case, that

	if use test; then
		export GTEST_ROOT="${WORKDIR}/googletest-release-${GTEST_VER}/googletest"
		export GMOCK_ROOT="${WORKDIR}/googletest-release-${GTEST_VER}/googlemock"
fi

only gets executed for non live ebuilds?

DarthGandalf · 2017-01-04T12:43:04Z

That was kind-of handled by test? ( ${GTEST_URL} ) being in SRC_URI only in non-9999 branch.
Then the environment variable points to an empty directory, and https://github.com/znc/znc/blob/master/test/CMakeLists.txt falls back to using the submodule. Contents of that submodule are not presented in the tarballs.

But explicit if use test && ${PV} == *9999* (or probably it should be inside [[ ]]) would also work.

Whissi · 2017-01-04T15:57:22Z

Should be done now via commit 68c6dd1. Thanks for the follow up! And thank you for your contribution as well!

PS: I changed gtest dist filename so we maybe able to share it with other packages like media-libs/chromaprint.

gentoo-repo-qa-bot added the assigned PR successfully assigned to the package maintainer(s). label Dec 24, 2016

DarthGandalf force-pushed the znc9999 branch from 984507d to cdfe0ee Compare December 26, 2016 23:45

Whissi suggested changes Dec 27, 2016

View reviewed changes

net-irc/znc: add requested changes in PR 3236

7180053

net-irc/znc: Use SWIG always, drop daemon USE-flag.

e6cbd5c

DarthGandalf commented Dec 27, 2016

View reviewed changes

net-irc/znc: Default to ninja but make it overridable

f63761a

SoapGentoo suggested changes Dec 29, 2016

View reviewed changes

net-irc/znc: Address more comments in PR 3236

c926735

DarthGandalf force-pushed the znc9999 branch from fc29db8 to c926735 Compare December 29, 2016 20:41

gentoo-bot closed this in 0835209 Jan 4, 2017

DarthGandalf deleted the znc9999 branch January 4, 2017 08:36

DarthGandalf mentioned this pull request Jan 8, 2017

Patch set from Gentoo znc/znc#1245

Closed

		@@ -0,0 +1,41 @@
		#!/sbin/openrc-run
		# Copyright 1999-2015 Gentoo Foundation

net-irc/znc: Update/rewrite ebuild for 9999 #3236

net-irc/znc: Update/rewrite ebuild for 9999 #3236

Conversation

DarthGandalf commented Dec 24, 2016

gentoo-repo-qa-bot commented Dec 24, 2016

sbraz commented Dec 24, 2016

DarthGandalf commented Dec 24, 2016

Whissi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Whissi Dec 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Whissi commented Dec 27, 2016

DarthGandalf commented Dec 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarthGandalf commented Dec 28, 2016 via email

Whissi commented Dec 28, 2016

DarthGandalf commented Dec 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarthGandalf commented Jan 4, 2017

Whissi commented Jan 4, 2017

DarthGandalf commented Jan 4, 2017

Whissi commented Jan 4, 2017 • edited

Whissi Dec 27, 2016 •

edited

Whissi commented Jan 4, 2017 •

edited