[gentoo-dev] [PATCH 1/4] distutils-r1.eclass: Introduce install_for_testing --via-root

[gentoo-dev] [PATCH 1/4] distutils-r1.eclass: Introduce install_for_testing --via-root

[Michał Górny]

Introduce a new --via-root mode for distutils_install_for_testing
function.  The legacy --via-home seems to no longer work for a lot
of packages but before we can confirm that --via-root is good enough
for every single one of them, let's have two variants to choose from.

The general recommendation is to try --via-root, and explicitly specify
--via-home if the former does not work.  Once all packages have explicit
--via-*, we will decide how to proceed.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/distutils-r1.eclass | 50 +++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index 25cb67b78a31..9e862a949275 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -492,7 +492,7 @@ esetup.py() {
 }
 
 # @FUNCTION: distutils_install_for_testing
-# @USAGE: [<args>...]
+# @USAGE: [--via-root|--via-home] [<args>...]
 # @DESCRIPTION:
 # Install the package into a temporary location for running tests.
 # Update PYTHONPATH appropriately and set TEST_DIR to the test
@@ -503,11 +503,19 @@ esetup.py() {
 # namespaces (and therefore proper install needs to be done to enforce
 # PYTHONPATH) or tests rely on the results of install command.
 # For most of the packages, tests built in BUILD_DIR are good enough.
+#
+# The function supports two install modes.  The current default is
+# the legacy --via-home mode.  However, it has problems with newer
+# versions of setuptools (50.3.0+).  The --via-root mode generally
+# works for these packages, and it will probably become the default
+# in the future, once we test all affected packages.  Please note
+# that proper testing sometimes requires unmerging the package first.
 distutils_install_for_testing() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	# A few notes:
-	# 1) because of namespaces, we can't use 'install --root',
+	# 1) because of namespaces, we can't use 'install --root'
+	#    (NB: this is probably no longer true with py3),
 	# 2) 'install --home' is terribly broken on pypy, so we need
 	#    to override --install-lib and --install-scripts,
 	# 3) non-root 'install' complains about PYTHONPATH and missing dirs,
@@ -522,14 +530,38 @@ distutils_install_for_testing() {
 	PATH=${bindir}:${PATH}
 	PYTHONPATH=${libdir}:${PYTHONPATH}
 
-	local add_args=(
-		install
-			--home="${TEST_DIR}"
-			--install-lib="${libdir}"
-			--install-scripts="${bindir}"
-	)
+	local install_method=home
+	case ${1} in
+		--via-home)
+			install_method=home
+			shift
+			;;
+		--via-root)
+			install_method=root
+			shift
+			;;
+	esac
+
+	case ${install_method} in
+		home)
+			local add_args=(
+				install
+					--home="${TEST_DIR}"
+					--install-lib="${libdir}"
+					--install-scripts="${bindir}"
+			)
+			mkdir -p "${libdir}" || die
+			;;
+		root)
+			local add_args=(
+				install
+					--root="${TEST_DIR}"
+					--install-lib=lib
+					--install-scripts=scripts
+			)
+			;;
+	esac
 
-	mkdir -p "${libdir}" || die
 	esetup.py "${add_args[@]}" "${@}"
 }
 
-- 
2.29.2

[gentoo-dev] [PATCH 2/4] dev-python/hypothesis: Use dift --via-root API

[Michał Górny]

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 dev-python/hypothesis/hypothesis-5.41.4.ebuild | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/dev-python/hypothesis/hypothesis-5.41.4.ebuild b/dev-python/hypothesis/hypothesis-5.41.4.ebuild
index 30e83a217730..7cd8e4bcc527 100644
--- a/dev-python/hypothesis/hypothesis-5.41.4.ebuild
+++ b/dev-python/hypothesis/hypothesis-5.41.4.ebuild
@@ -48,11 +48,7 @@ python_prepare() {
 }
 
 python_test() {
-	local -x PYTHONPATH="${BUILD_DIR}/install/lib"
-	esetup.py install \
-		--root="${BUILD_DIR}/install" \
-		--install-lib=lib
-
+	distutils_install_for_testing --via-root
 	pytest -vv tests/cover tests/pytest tests/quality \
 		-n "$(makeopts_jobs "${MAKEOPTS}" "$(get_nproc)")" ||
 		die "Tests fail with ${EPYTHON}"
-- 
2.29.2

[gentoo-dev] [PATCH 3/4] distutils-r1.eclass: Accept distutils_enable_tests --install

[Michał Górny]

Add a convenience --install option to distutils_enable_tests to call
distutils_install_for_testing.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/distutils-r1.eclass | 74 +++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 16 deletions(-)

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index 9e862a949275..24fcf13b74d7 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -378,7 +378,7 @@ distutils_enable_sphinx() {
 }
 
 # @FUNCTION: distutils_enable_tests
-# @USAGE: <test-runner>
+# @USAGE: [--install] <test-runner>
 # @DESCRIPTION:
 # Set up IUSE, RESTRICT, BDEPEND and python_test() for running tests
 # with the specified test runner.  Also copies the current value
@@ -389,6 +389,10 @@ distutils_enable_sphinx() {
 # - setup.py: setup.py test (no deps included)
 # - unittest: for built-in Python unittest module
 #
+# Additionally ,if --install is passed as the first parameter,
+# 'distutils_install_for_testing --via-root' is called before running
+# the test suite.
+#
 # This function is meant as a helper for common use cases, and it only
 # takes care of basic setup.  You still need to list additional test
 # dependencies manually.  If you have uncommon use case, you should
@@ -398,33 +402,71 @@ distutils_enable_sphinx() {
 # declared.  Take care not to overwrite the variables set by it.
 distutils_enable_tests() {
 	debug-print-function ${FUNCNAME} "${@}"
-	[[ ${#} -eq 1 ]] || die "${FUNCNAME} takes exactly one argument: test-runner"
 
+	local do_install=
+	case ${1} in
+		--install)
+			do_install=1
+			shift
+			;;
+	esac
+
+	[[ ${#} -eq 1 ]] || die "${FUNCNAME} takes exactly one argument: test-runner"
 	local test_pkg
 	case ${1} in
 		nose)
 			test_pkg=">=dev-python/nose-1.3.7-r4"
-			python_test() {
-				nosetests -v || die "Tests fail with ${EPYTHON}"
-			}
+			if [[ ${do_install} ]]; then
+				python_test() {
+					distutils_install_for_testing --via-root
+					nosetests -v || die "Tests fail with ${EPYTHON}"
+				}
+			else
+				python_test() {
+					nosetests -v || die "Tests fail with ${EPYTHON}"
+				}
+			fi
 			;;
 		pytest)
 			test_pkg=">=dev-python/pytest-4.5.0"
-			python_test() {
-				pytest -vv || die "Tests fail with ${EPYTHON}"
-			}
+			if [[ ${do_install} ]]; then
+				python_test() {
+					distutils_install_for_testing --via-root
+					pytest -vv || die "Tests fail with ${EPYTHON}"
+				}
+			else
+				python_test() {
+					pytest -vv || die "Tests fail with ${EPYTHON}"
+				}
+			fi
 			;;
 		setup.py)
-			python_test() {
-				nonfatal esetup.py test --verbose ||
-					die "Tests fail with ${EPYTHON}"
-			}
+			if [[ ${do_install} ]]; then
+				python_test() {
+					distutils_install_for_testing --via-root
+					nonfatal esetup.py test --verbose ||
+						die "Tests fail with ${EPYTHON}"
+				}
+			else
+				python_test() {
+					nonfatal esetup.py test --verbose ||
+						die "Tests fail with ${EPYTHON}"
+				}
+			fi
 			;;
 		unittest)
-			python_test() {
-				"${EPYTHON}" -m unittest discover -v ||
-					die "Tests fail with ${EPYTHON}"
-			}
+			if [[ ${do_install} ]]; then
+				python_test() {
+					distutils_install_for_testing --via-root
+					"${EPYTHON}" -m unittest discover -v ||
+						die "Tests fail with ${EPYTHON}"
+				}
+			else
+				python_test() {
+					"${EPYTHON}" -m unittest discover -v ||
+						die "Tests fail with ${EPYTHON}"
+				}
+			fi
 			;;
 		*)
 			die "${FUNCNAME}: unsupported argument: ${1}"
-- 
2.29.2

[gentoo-dev] [PATCH 4/4] dev-python/flake8: Use distutils_enable_tests --install

[Michał Górny]

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 dev-python/flake8/flake8-3.8.3-r1.ebuild | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/dev-python/flake8/flake8-3.8.3-r1.ebuild b/dev-python/flake8/flake8-3.8.3-r1.ebuild
index 55e6d64f8d8d..1bfc0de53548 100644
--- a/dev-python/flake8/flake8-3.8.3-r1.ebuild
+++ b/dev-python/flake8/flake8-3.8.3-r1.ebuild
@@ -38,10 +38,4 @@ BDEPEND="${RDEPEND}
 PATCHES=( "${FILESDIR}/${P}-pytest6.patch" )
 
 distutils_enable_sphinx docs/source dev-python/sphinx-prompt dev-python/sphinx_rtd_theme
-distutils_enable_tests pytest
-
-python_test() {
-	# Otherwise some tests fail if the package isn't installed
-	distutils_install_for_testing
-	pytest -vv || die "Tests fail with ${EPYTHON}"
-}
+distutils_enable_tests --install pytest
-- 
2.29.2

Re: [gentoo-dev] [PATCH 1/4] distutils-r1.eclass: Introduce install_for_testing --via-root

[Ulrich Mueller]

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

>>>>> On Sun, 29 Nov 2020, Michał Górny wrote:
 
> +	case ${install_method} in
> +		home)
> +			local add_args=(
> +				install
> +					--home="${TEST_DIR}"
> +					--install-lib="${libdir}"
> +					--install-scripts="${bindir}"
> +			)
> +			mkdir -p "${libdir}" || die
> +			;;
> +		root)
> +			local add_args=(
> +				install
> +					--root="${TEST_DIR}"
> +					--install-lib=lib
> +					--install-scripts=scripts
> +			)
> +			;;
> +	esac

Having the same "local add_args" declaration twice looks strange and may
be error prone. Can you move it outside of the case statement?

Also, why are the array elements at different indent levels?

Ulrich

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

Re: [gentoo-dev] [PATCH 3/4] distutils-r1.eclass: Accept distutils_enable_tests --install

[Ulrich Mueller]

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

>>>>> On Sun, 29 Nov 2020, Michał Górny wrote:

> +# Additionally ,if --install is passed as the first parameter,

s/ ,/, /

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

Re: [gentoo-dev] [PATCH 1/4] distutils-r1.eclass: Introduce install_for_testing --via-root

[Michał Górny]

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

On Sun, 2020-11-29 at 12:02 +0100, Ulrich Mueller wrote:
> > > > > > On Sun, 29 Nov 2020, Michał Górny wrote:
>  
> > +	case ${install_method} in
> > +		home)
> > +			local add_args=(
> > +				install
> > +					--home="${TEST_DIR}"
> > +					--install-lib="${libdir}"
> > +					--install-scripts="${bindir}"
> > +			)
> > +			mkdir -p "${libdir}" || die
> > +			;;
> > +		root)
> > +			local add_args=(
> > +				install
> > +					--root="${TEST_DIR}"
> > +					--install-lib=lib
> > +					--install-scripts=scripts
> > +			)
> > +			;;
> > +	esac
> 
> Having the same "local add_args" declaration twice looks strange and may
> be error prone. Can you move it outside of the case statement?
> 
> Also, why are the array elements at different indent levels?
> 

Because they are options passed to 'install' command.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]