[gentoo-dev] RFC v2: eutils.eclass: More reliable return status for e*_clean.

[Ulrich Mueller <ulm@gentoo.org>]

[-- Attachment #1.1: message body text --]
[-- Type: text/plain, Size: 1322 bytes --]

>>>>> On Fri, 16 Feb 2018, Michał Górny wrote:

> Could you please use quotes instead of backslash escapes all the
> way? They're generally less confusing since escapes trigger
> different behavior in different contexts in bash.

Done for the parentheses, and removed escaping of the plus sign which
is no special character in bash.

>>>>> On Fri, 16 Feb 2018, Michael Orlitzky wrote:

> To make maintenance easier, I would go one step further and say that
> unless two functions need the same variables or call one another,
> they belong in separate eclasses. Since ecvs_clean, esvn_clean, and
> egit_clean are completely independent of one another, they could go
> in separate eclasses -- it's not like you'll need more than one of
> them in your ebuild. Then in the future if we need to change
> egit_clean, we will know precisely which ebuilds are affected.

If these were 200 line functions then I would agree. However, they
consist of one line each, are all about the same theme, and too much
fragmentation of eclasses should be avoided. In fact, I am already
reluctant about splitting off these tiny functions from eutils.

Anyway, updated patches are attached. First patch changes the code,
second patch splits the functions off into their own eclass (with no
code changes).

Ulrich


[-- Attachment #1.2: 0001-eutils.eclass-More-reliable-return-status-for-e-_cle.patch --]
[-- Type: text/plain, Size: 1739 bytes --]

From 5cd1763df2dc385524da0163bdcadecfaf93675f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Fri, 16 Feb 2018 07:47:46 +0100
Subject: [PATCH 1/2] eutils.eclass: More reliable return status for e*_clean
 functions.

In ecvs_clean, combine the two find commands into one, so that the
exit status of the first one won't be ignored.

Also use find -exec rather then find | xargs, so we don't have to
check the exit status of all commands in the pipeline.
---
 eclass/eutils.eclass | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/eclass/eutils.eclass b/eclass/eutils.eclass
index 8bbd561015ad..0a5bf3853582 100644
--- a/eclass/eutils.eclass
+++ b/eclass/eutils.eclass
@@ -44,8 +44,8 @@ fi
 # internal CVS directories.  Defaults to $PWD.
 ecvs_clean() {
 	[[ $# -eq 0 ]] && set -- .
-	find "$@" -type d -name 'CVS' -prune -print0 | xargs -0 rm -rf
-	find "$@" -type f -name '.cvs*' -print0 | xargs -0 rm -rf
+	find "$@" '(' -type d -name 'CVS' -prune -o -type f -name '.cvs*' ')' \
+		-exec rm -rf '{}' +
 }
 
 # @FUNCTION: esvn_clean
@@ -55,7 +55,7 @@ ecvs_clean() {
 # internal Subversion directories.  Defaults to $PWD.
 esvn_clean() {
 	[[ $# -eq 0 ]] && set -- .
-	find "$@" -type d -name '.svn' -prune -print0 | xargs -0 rm -rf
+	find "$@" -type d -name '.svn' -prune -exec rm -rf '{}' +
 }
 
 # @FUNCTION: egit_clean
@@ -65,7 +65,7 @@ esvn_clean() {
 # contains internal Git directories.  Defaults to $PWD.
 egit_clean() {
 	[[ $# -eq 0 ]] && set -- .
-	find "$@" -type d -name '.git*' -prune -print0 | xargs -0 rm -rf
+	find "$@" -type d -name '.git*' -prune -exec rm -rf '{}' +
 }
 
 # @FUNCTION: emktemp
-- 
2.16.1


[-- Attachment #1.3: 0002-vcs-clean.eclass-Split-off-clean-helpers-from-eutils.patch --]
[-- Type: text/plain, Size: 3808 bytes --]

From 5056a7a455bac8a5b5bdadc357cc67cf0f6865fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Fri, 16 Feb 2018 18:48:13 +0100
Subject: [PATCH 2/2] vcs-clean.eclass: Split off clean helpers from
 eutils.eclass.

Split off functions ecvs_clean, esvn_clean, and egit_clean into
a dedicated vcs-clean.eclass. No code changes.

For backwards compatibility, eutils inherits the new eclass in
existing EAPIs.
---
 eclass/eutils.eclass    | 34 ++--------------------------------
 eclass/vcs-clean.eclass | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 32 deletions(-)
 create mode 100644 eclass/vcs-clean.eclass

diff --git a/eclass/eutils.eclass b/eclass/eutils.eclass
index 0a5bf3853582..7840afbb77b9 100644
--- a/eclass/eutils.eclass
+++ b/eclass/eutils.eclass
@@ -20,7 +20,8 @@ _EUTILS_ECLASS=1
 # implicitly inherited (now split) eclasses
 case ${EAPI:-0} in
 0|1|2|3|4|5|6)
-	inherit desktop epatch estack ltprune multilib preserve-libs toolchain-funcs
+	inherit desktop epatch estack ltprune multilib preserve-libs \
+		toolchain-funcs vcs-clean
 	;;
 esac
 
@@ -37,37 +38,6 @@ if ! declare -F eqawarn >/dev/null ; then
 	}
 fi
 
-# @FUNCTION: ecvs_clean
-# @USAGE: [list of dirs]
-# @DESCRIPTION:
-# Remove CVS directories recursiveley.  Useful when a source tarball contains
-# internal CVS directories.  Defaults to $PWD.
-ecvs_clean() {
-	[[ $# -eq 0 ]] && set -- .
-	find "$@" '(' -type d -name 'CVS' -prune -o -type f -name '.cvs*' ')' \
-		-exec rm -rf '{}' +
-}
-
-# @FUNCTION: esvn_clean
-# @USAGE: [list of dirs]
-# @DESCRIPTION:
-# Remove .svn directories recursiveley.  Useful when a source tarball contains
-# internal Subversion directories.  Defaults to $PWD.
-esvn_clean() {
-	[[ $# -eq 0 ]] && set -- .
-	find "$@" -type d -name '.svn' -prune -exec rm -rf '{}' +
-}
-
-# @FUNCTION: egit_clean
-# @USAGE: [list of dirs]
-# @DESCRIPTION:
-# Remove .git* directories/files recursiveley.  Useful when a source tarball
-# contains internal Git directories.  Defaults to $PWD.
-egit_clean() {
-	[[ $# -eq 0 ]] && set -- .
-	find "$@" -type d -name '.git*' -prune -exec rm -rf '{}' +
-}
-
 # @FUNCTION: emktemp
 # @USAGE: [temp dir]
 # @DESCRIPTION:
diff --git a/eclass/vcs-clean.eclass b/eclass/vcs-clean.eclass
new file mode 100644
index 000000000000..649a9e3039b1
--- /dev/null
+++ b/eclass/vcs-clean.eclass
@@ -0,0 +1,40 @@
+# Copyright 1999-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+# @ECLASS: vcs-clean.eclass
+# @MAINTAINER:
+# base-system@gentoo.org
+# @AUTHOR:
+# Benedikt Böhm <hollow@gentoo.org>
+# @BLURB: helper functions to remove VCS directories
+
+# @FUNCTION: ecvs_clean
+# @USAGE: [list of dirs]
+# @DESCRIPTION:
+# Remove CVS directories and .cvs* files recursively.  Useful when a
+# source tarball contains internal CVS directories.  Defaults to ${PWD}.
+ecvs_clean() {
+	[[ $# -eq 0 ]] && set -- .
+	find "$@" '(' -type d -name 'CVS' -prune -o -type f -name '.cvs*' ')' \
+		-exec rm -rf '{}' +
+}
+
+# @FUNCTION: esvn_clean
+# @USAGE: [list of dirs]
+# @DESCRIPTION:
+# Remove .svn directories recursively.  Useful when a source tarball
+# contains internal Subversion directories.  Defaults to ${PWD}.
+esvn_clean() {
+	[[ $# -eq 0 ]] && set -- .
+	find "$@" -type d -name '.svn' -prune -exec rm -rf '{}' +
+}
+
+# @FUNCTION: egit_clean
+# @USAGE: [list of dirs]
+# @DESCRIPTION:
+# Remove .git* directories recursively.  Useful when a source tarball
+# contains internal Git directories.  Defaults to ${PWD}.
+egit_clean() {
+	[[ $# -eq 0 ]] && set -- .
+	find "$@" -type d -name '.git*' -prune -exec rm -rf '{}' +
+}
-- 
2.16.1


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