[gentoo-dev] New eclass - twisted-r1

[gentoo-dev] New eclass - twisted-r1

[yac]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

Hello everypony

In order to bump dev-python/twisted packages to EAPI=5 we also need to
bump the twisted eclass to use distutils-r1.

I have made this bump (to the eclass and dev-python/twisted*) packages
in my yac overlay [1]

You can view the actuall changes at [2]. It's merely the existing
twisted eclass adapted to distutils-r1.

I have tested the packages (merge, module import in python and tests)
on python2_7 at amd64.

I don't know when I'll get around to testing other pythons and x86.
Certainly I'm currently unable to test pypy and eprefix.

Besides reviewing the changes, could somepony test these arches?


[1] https://github.com/yaccz/gentoo-overlay/tree/twisted-r1
[2] https://github.com/yaccz/gentoo-overlay/compare/5421996...020b0c2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 4083 bytes --]

First of all: please wrap lines at 72 or 80 chars (even if the original
eclass didn't do that).

> if [[ "${CATEGORY}/${PN}" == "dev-python/twisted"* ]]; then

I know you're not responsible for this but it seems wrong to have two
different behaviors depending on PN. Is this used somewhere actually?

> 	MY_PV="${MY_PV:-${PV}}"
> 	MY_P="Twisted${MY_PACKAGE}-${MY_PV}"
> 
> 	HOMEPAGE="http://www.twistedmatrix.com/"
> 	#SRC_URI="http://tmrc.mit.edu/mirror/twisted/${MY_PACKAGE}/$(get_version_component_range 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> 	SRC_URI="http://twistedmatrix.com/Releases/${MY_PACKAGE}/$(get_version_component_range 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> 
> 	LICENSE="MIT"
> 	SLOT="0"
> 	IUSE=""
> 
> 	S="${WORKDIR}/${MY_P}"
> 
> 	TWISTED_PLUGINS="${TWISTED_PLUGINS:-twisted.plugins}"
> fi
> 
> # @ECLASS-VARIABLE: TWISTED_PLUGINS
> # @DESCRIPTION:
> # Twisted plugins, whose cache is regenerated in pkg_postinst() and pkg_postrm() phases.
> 
> python_test() {
> 	# TODO: this seems to be used only in dev-python/twisted-* packages as
> 	# dev-python/twisted-13.0.0 have it's own src_test
> 	if [[ "${CATEGORY}/${PN}" != "dev-python/twisted"* ]]; then
> 		die "${FUNCNAME}() can be used only in dev-python/twisted* packages"
> 	fi
> 
> 	local sitedir="${EPREFIX}$(python_get_sitedir)"
> 
> 	# Copy modules of other Twisted packages from site-packages directory to temporary directory.
> 	mkdir -p "${T}/${sitedir}"
> 	cp -R "${ROOT}${sitedir}/twisted" "${T}/${sitedir}" || die "Copying of modules of other Twisted packages failed with $(python_get_implementation) $(python_get_version)"
> 	rm -fr "${T}/${sitedir}/${PN/-//}"

Awful and ugly. Try to find a way to run the tests without this hackery.

> 	# Install modules of current package to temporary directory.
> 	"${PYTHON}" setup.py build -b "build-${EPYTHON}" install --force --no-compile --root="${T}" || die "Installation into temporary directory failed with $(python_get_implementation) $(python_get_version)"

This is likely a candidate for 'distutils_install_for_testing'.
Or 'esetup.py' at least. Inlining build-dir is simply wrong.

> 	pushd "${T}/${sitedir}" > /dev/null || return 1
> 	PATH="${T}${EPREFIX}/usr/bin:${PATH}" PYTHONPATH="${T}/${sitedir}" trial ${PN/-/.} || return 1
> 	popd > /dev/null || return 1
> 
> 	rm -fr "${T}/${sitedir}"
> }
> 
> twisted-r1_src_install() {
> 	distutils-r1_src_install
> 
> 	if [[ -d doc/man ]]; then
> 		doman doc/man/*.[[:digit:]]
> 	fi
> 
> 	if [[ -d doc ]]; then
> 		insinto /usr/share/doc/${PF}
> 		doins -r $(find doc -mindepth 1 -maxdepth 1 -not -name man)
> 	fi
> }
> 
> _twisted-r1_update_plugin_cache() {
> 	local dir exit_status="0" module
> 
> 	for module in ${TWISTED_PLUGINS}; do
> 		if [[ -d "${EROOT}$(python_get_sitedir)/${module//.//}" ]]; then
> 			find "${EROOT}$(python_get_sitedir)/${module//.//}" -name dropin.cache -print0 | xargs -0 rm -f
> 		fi
> 	done
> 
> 	for module in ${TWISTED_PLUGINS}; do
> 		# http://twistedmatrix.com/documents/current/core/howto/plugin.html
> 		"${PYTHON}" -c \
> "import sys
> sys.path.insert(0, '${EROOT}$(python_get_sitedir)')
> 
> try:
> 	import twisted.plugin
> 	import ${module}
> except ImportError:
> 	if '${EBUILD_PHASE}' == 'postinst':
> 		raise
> 	else:
> 	    # Twisted, zope.interface or given plugins might have been uninstalled.
> 		sys.exit(0)
> 
> list(twisted.plugin.getPlugins(twisted.plugin.IPlugin, ${module}))" || exit_status="1"
> 	done
> 
> 	for module in ${TWISTED_PLUGINS}; do
> 		# Delete empty parent directories.
> 		local dir="${EROOT}$(python_get_sitedir)/${module//.//}"
> 		while [[ "${dir}" != "${EROOT%/}" ]]; do
> 			rmdir "${dir}" 2> /dev/null || break
> 			dir="${dir%/*}"
> 		done
> 	done
> 
> 	return "${exit_status}"
> }
> 
> twisted-r1_pkg_postinst() {
> 	_distutils-r1_run_foreach_impl _twisted-r1_update_plugin_cache
> }
> 
> twisted-r1_pkg_postrm() {
> 	_distutils-r1_run_foreach_impl _twisted-r1_update_plugin_cache
> }

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[yac]

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Wed, 10 Jul 2013 22:25:50 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> First of all: please wrap lines at 72 or 80 chars (even if the
> original eclass didn't do that).
> 
> > if [[ "${CATEGORY}/${PN}" == "dev-python/twisted"* ]]; then
> 
> I know you're not responsible for this but it seems wrong to have two
> different behaviors depending on PN. Is this used somewhere actually?
> 
> > 	MY_PV="${MY_PV:-${PV}}"
> > 	MY_P="Twisted${MY_PACKAGE}-${MY_PV}"
> > 
> > 	HOMEPAGE="http://www.twistedmatrix.com/"
> > 	#SRC_URI="http://tmrc.mit.edu/mirror/twisted/${MY_PACKAGE}/$(get_version_component_range
> > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > SRC_URI="http://twistedmatrix.com/Releases/${MY_PACKAGE}/$(get_version_component_range
> > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > 
> > 	LICENSE="MIT"
> > 	SLOT="0"
> > 	IUSE=""
> > 
> > 	S="${WORKDIR}/${MY_P}"
> > 
> > 	TWISTED_PLUGINS="${TWISTED_PLUGINS:-twisted.plugins}"
> > fi

That's what I thought when I saw that but I found that all the
dev-python/twisted-* packages are depending on this, so I went with it.

Now I'm thinking I could instead provide functions
twisted-r1_twisted_src_uri and twisted-r1_twisted_build_dir used as

SRC_URI="$(twisted-r1_twisted_src_uri)"
S="$(twisted-r1_twisted_build_dir)"

in the depender ebuilds.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

Dnia 2013-07-10, o godz. 23:40:11
yac <yac@gentoo.org> napisał(a):

> On Wed, 10 Jul 2013 22:25:50 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > First of all: please wrap lines at 72 or 80 chars (even if the
> > original eclass didn't do that).
> > 
> > > if [[ "${CATEGORY}/${PN}" == "dev-python/twisted"* ]]; then
> > 
> > I know you're not responsible for this but it seems wrong to have two
> > different behaviors depending on PN. Is this used somewhere actually?
> > 
> > > 	MY_PV="${MY_PV:-${PV}}"
> > > 	MY_P="Twisted${MY_PACKAGE}-${MY_PV}"
> > > 
> > > 	HOMEPAGE="http://www.twistedmatrix.com/"
> > > 	#SRC_URI="http://tmrc.mit.edu/mirror/twisted/${MY_PACKAGE}/$(get_version_component_range
> > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > SRC_URI="http://twistedmatrix.com/Releases/${MY_PACKAGE}/$(get_version_component_range
> > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > 
> > > 	LICENSE="MIT"
> > > 	SLOT="0"
> > > 	IUSE=""
> > > 
> > > 	S="${WORKDIR}/${MY_P}"
> > > 
> > > 	TWISTED_PLUGINS="${TWISTED_PLUGINS:-twisted.plugins}"
> > > fi
> 
> That's what I thought when I saw that but I found that all the
> dev-python/twisted-* packages are depending on this, so I went with it.
> 
> Now I'm thinking I could instead provide functions
> twisted-r1_twisted_src_uri and twisted-r1_twisted_build_dir used as
> 
> SRC_URI="$(twisted-r1_twisted_src_uri)"
> S="$(twisted-r1_twisted_build_dir)"
> 
> in the depender ebuilds.

I'm more-of wondering if that conditional is really necessary useful.
Unless I'm missing something, the non-twisted ebuilds will replace
those variables anyway, won't them?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[yac]

[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]

On Thu, 11 Jul 2013 08:55:47 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> Dnia 2013-07-10, o godz. 23:40:11
> yac <yac@gentoo.org> napisał(a):
> 
> > On Wed, 10 Jul 2013 22:25:50 +0200
> > Michał Górny <mgorny@gentoo.org> wrote:
> > 
> > > First of all: please wrap lines at 72 or 80 chars (even if the
> > > original eclass didn't do that).
> > > 
> > > > if [[ "${CATEGORY}/${PN}" == "dev-python/twisted"* ]]; then
> > > 
> > > I know you're not responsible for this but it seems wrong to have
> > > two different behaviors depending on PN. Is this used somewhere
> > > actually?
> > > 
> > > > 	MY_PV="${MY_PV:-${PV}}"
> > > > 	MY_P="Twisted${MY_PACKAGE}-${MY_PV}"
> > > > 
> > > > 	HOMEPAGE="http://www.twistedmatrix.com/"
> > > > 	#SRC_URI="http://tmrc.mit.edu/mirror/twisted/${MY_PACKAGE}/$(get_version_component_range
> > > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > > SRC_URI="http://twistedmatrix.com/Releases/${MY_PACKAGE}/$(get_version_component_range
> > > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > > 
> > > > 	LICENSE="MIT"
> > > > 	SLOT="0"
> > > > 	IUSE=""
> > > > 
> > > > 	S="${WORKDIR}/${MY_P}"
> > > > 
> > > > 	TWISTED_PLUGINS="${TWISTED_PLUGINS:-twisted.plugins}"
> > > > fi
> > 
> > That's what I thought when I saw that but I found that all the
> > dev-python/twisted-* packages are depending on this, so I went with
> > it.
> > 
> > Now I'm thinking I could instead provide functions
> > twisted-r1_twisted_src_uri and twisted-r1_twisted_build_dir used as
> > 
> > SRC_URI="$(twisted-r1_twisted_src_uri)"
> > S="$(twisted-r1_twisted_build_dir)"
> > 
> > in the depender ebuilds.
> 
> I'm more-of wondering if that conditional is really necessary useful.
> Unless I'm missing something, the non-twisted ebuilds will replace
> those variables anyway, won't them?

The dev-python/twisted-* packages do rely on this. They will set just
MY_PACKAGE=

And the S, SRC_URI, LICENSE, SLOT, IUSE is set by the eclass.

Other than dev-python/twisted-* packages does not match, so they need
not to override it.

I think the main purpose is code deduplication regarding setting the
SRC_URI and S.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[yac]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

On Wed, 10 Jul 2013 22:25:50 +0200
Michał Górny <mgorny@gentoo.org> wrote:
> > python_test() {
> > 	# TODO: this seems to be used only in dev-python/twisted-*
> > packages as # dev-python/twisted-13.0.0 have it's own src_test
> > 	if [[ "${CATEGORY}/${PN}" != "dev-python/twisted"* ]]; then
> > 		die "${FUNCNAME}() can be used only in
> > dev-python/twisted* packages" fi
> > 
> > 	local sitedir="${EPREFIX}$(python_get_sitedir)"
> > 
> > 	# Copy modules of other Twisted packages from site-packages
> > directory to temporary directory. mkdir -p "${T}/${sitedir}"
> > 	cp -R "${ROOT}${sitedir}/twisted" "${T}/${sitedir}" || die
> > "Copying of modules of other Twisted packages failed with
> > $(python_get_implementation) $(python_get_version)" rm -fr
> > "${T}/${sitedir}/${PN/-//}"
> 
> Awful and ugly. Try to find a way to run the tests without this
> hackery.

What are you referring to?

For the "${CATEGORY}/${PN}" match. All the non-twisted packages are
overriding src_test but it's probably be gonna cleaner to rename the
python_test to twisted-r1_twisted_test and call it explicitly for the
twisted packages.

As for the Copy modules, I'm afraid that's inevitable.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] New eclass - twisted-r1

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

Dnia 2013-07-11, o godz. 13:34:01
yac <yac@gentoo.org> napisał(a):

> On Thu, 11 Jul 2013 08:55:47 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > Dnia 2013-07-10, o godz. 23:40:11
> > yac <yac@gentoo.org> napisał(a):
> > 
> > > On Wed, 10 Jul 2013 22:25:50 +0200
> > > Michał Górny <mgorny@gentoo.org> wrote:
> > > 
> > > > First of all: please wrap lines at 72 or 80 chars (even if the
> > > > original eclass didn't do that).
> > > > 
> > > > > if [[ "${CATEGORY}/${PN}" == "dev-python/twisted"* ]]; then
> > > > 
> > > > I know you're not responsible for this but it seems wrong to have
> > > > two different behaviors depending on PN. Is this used somewhere
> > > > actually?
> > > > 
> > > > > 	MY_PV="${MY_PV:-${PV}}"
> > > > > 	MY_P="Twisted${MY_PACKAGE}-${MY_PV}"
> > > > > 
> > > > > 	HOMEPAGE="http://www.twistedmatrix.com/"
> > > > > 	#SRC_URI="http://tmrc.mit.edu/mirror/twisted/${MY_PACKAGE}/$(get_version_component_range
> > > > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > > > SRC_URI="http://twistedmatrix.com/Releases/${MY_PACKAGE}/$(get_version_component_range
> > > > > 1-2 ${MY_PV})/${MY_P}.tar.bz2"
> > > > > 
> > > > > 	LICENSE="MIT"
> > > > > 	SLOT="0"
> > > > > 	IUSE=""
> > > > > 
> > > > > 	S="${WORKDIR}/${MY_P}"
> > > > > 
> > > > > 	TWISTED_PLUGINS="${TWISTED_PLUGINS:-twisted.plugins}"
> > > > > fi
> > > 
> > > That's what I thought when I saw that but I found that all the
> > > dev-python/twisted-* packages are depending on this, so I went with
> > > it.
> > > 
> > > Now I'm thinking I could instead provide functions
> > > twisted-r1_twisted_src_uri and twisted-r1_twisted_build_dir used as
> > > 
> > > SRC_URI="$(twisted-r1_twisted_src_uri)"
> > > S="$(twisted-r1_twisted_build_dir)"
> > > 
> > > in the depender ebuilds.
> > 
> > I'm more-of wondering if that conditional is really necessary useful.
> > Unless I'm missing something, the non-twisted ebuilds will replace
> > those variables anyway, won't them?
> 
> The dev-python/twisted-* packages do rely on this. They will set just
> MY_PACKAGE=
> 
> And the S, SRC_URI, LICENSE, SLOT, IUSE is set by the eclass.
> 
> Other than dev-python/twisted-* packages does not match, so they need
> not to override it.

Well, I meant that the other packages need to set SRC_URI and stuff
anyway, so they will override it anyway. But S is a fair point.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]