[gentoo-dev] [PATCH v2 1/4] metadata/install-qa-check.d: add 60tmpfiles-path QA check

[gentoo-dev] [PATCH v2 1/4] metadata/install-qa-check.d: add 60tmpfiles-path QA check

[Sam James]

This adds two tmpfiles related QA checks:
1) Verify packages don't install tmpfiles to /etc/tmpfiles.d, which
is a forbidden (user-configuration) location;

2) Check whether packages inherit tmpfiles.eclass if they're
installing files to /usr/lib/tmpfiles.d.

(This helps to catch packages not calling tmpfiles_process
in pkg_postinst).

Signed-off-by: Sam James <sam@gentoo.org>
---
 metadata/install-qa-check.d/60tmpfiles-paths | 37 ++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 metadata/install-qa-check.d/60tmpfiles-paths

diff --git a/metadata/install-qa-check.d/60tmpfiles-paths b/metadata/install-qa-check.d/60tmpfiles-paths
new file mode 100644
index 000000000000..ed0bdbff8cd5
--- /dev/null
+++ b/metadata/install-qa-check.d/60tmpfiles-paths
@@ -0,0 +1,37 @@
+# Copyright 2021 Gentoo Authors
+# Distributed under the terms of the GNU General Public License v2
+
+# QA check: ensure that packages installing tmpfiles configuration inherit the eclass
+# Maintainer: Sam James <sam@gentoo.org>
+
+# Implements two checks:
+# 1) Installation to /etc/tmpfiles.d (which is a user-customization location);
+# 2) Installation of any tmpfiles to /usr/lib/tmpfiles.d without inheriting the eclass
+#    (needed for tmpfiles_process in pkg_postinst)
+tmpfiles_check() {
+	# Check 1
+	# Scan image for files in /etc/tmpfiles.d which is a forbidden location
+	if [[ -d "${ED}"/etc/tmpfiles.d/ ]] ; then
+		eqawarn "QA Notice: files installed to /etc/tmpfiles.d"
+		eqawarn "tmpfiles configuration files must be installed by ebuilds /usr/lib/tmpfiles.d!"
+	fi
+
+	# Check 2
+	# We're now going to check for whether we install files to /usr/lib/tmpfiles.d without
+	# inheriting the eclass (weak catch for ebuilds not calling tmpfiles_process in pkg_postinst)
+
+	# No need to carry on if we're inheriting the eclass
+	if has tmpfiles ${INHERITED} ; then
+		return
+	fi
+
+	if [[ -d "${ED}"/usr/lib/tmpfiles.d/ ]] ; then
+		eqawarn "QA Notice: package is installing tmpfiles without inheriting tmpfiles.eclass!"
+		eqawarn "Packages must inherit tmpfiles.eclass then call tmpfiles_process in pkg_postinst."
+	fi
+}
+
+tmpfiles_check
+: # guarantee successful exit
+
+# vim:ft=sh
-- 
2.32.0

[gentoo-dev] [PATCH v2 2/4] metadata/install-qa-check.d: only trigger on tmpfiles in forbidden location

[Sam James]

It's okay to use "keepdir" on /etc/tmpfiles.d.

See: https://archives.gentoo.org/gentoo-dev/message/50558b55dc34f37b238807fc4759640d
Signed-off-by: Sam James <sam@gentoo.org>
---
 metadata/install-qa-check.d/60tmpfiles-paths | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/metadata/install-qa-check.d/60tmpfiles-paths b/metadata/install-qa-check.d/60tmpfiles-paths
index ed0bdbff8cd5..5ef56885ebe7 100644
--- a/metadata/install-qa-check.d/60tmpfiles-paths
+++ b/metadata/install-qa-check.d/60tmpfiles-paths
@@ -11,7 +11,12 @@
 tmpfiles_check() {
 	# Check 1
 	# Scan image for files in /etc/tmpfiles.d which is a forbidden location
-	if [[ -d "${ED}"/etc/tmpfiles.d/ ]] ; then
+	# (We use this glob to avoid triggering on keepdir)
+	shopt -s nullglob
+	local files=( "${ED}"/etc/tmpfiles.d/*.conf )
+	shopt -u nullglob
+
+	if [[ ${#files[@]} -gt 0 ]]; then
 		eqawarn "QA Notice: files installed to /etc/tmpfiles.d"
 		eqawarn "tmpfiles configuration files must be installed by ebuilds /usr/lib/tmpfiles.d!"
 	fi
-- 
2.32.0

[gentoo-dev] [PATCH v2 3/4] metadata/install-qa-check.d: add exemptions for some packages wrt inherit

[Sam James]

Both sys-apps/systemd and sys-libs/pam need to install some files
to these directories without inheriting the eclass.

For future work, we should have a standardised way on opting
out of installed files QA checks, but other QA checks are
already suffering from this issue.

See: https://archives.gentoo.org/gentoo-dev/message/0a96793036a4fdd9ac311a46950d7e7b
Signed-off-by: Sam James <sam@gentoo.org>
---
 metadata/install-qa-check.d/60tmpfiles-paths | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/metadata/install-qa-check.d/60tmpfiles-paths b/metadata/install-qa-check.d/60tmpfiles-paths
index 5ef56885ebe7..81286de584a2 100644
--- a/metadata/install-qa-check.d/60tmpfiles-paths
+++ b/metadata/install-qa-check.d/60tmpfiles-paths
@@ -30,6 +30,14 @@ tmpfiles_check() {
 		return
 	fi
 
+	# It's okay for some packages to do this because of circular dependencies and such
+	# See: https://archives.gentoo.org/gentoo-dev/message/0a96793036a4fdd9ac311a46950d7e7b
+	# TODO: Standardize some way of allowing ebuilds to opt-out of checks like this
+	local package=${CATEGORY}/${PN}
+	if [[ ${package} == "sys-apps/systemd" || ${package} == "sys-libs/pam" ]] ; then
+		return
+	fi
+
 	if [[ -d "${ED}"/usr/lib/tmpfiles.d/ ]] ; then
 		eqawarn "QA Notice: package is installing tmpfiles without inheriting tmpfiles.eclass!"
 		eqawarn "Packages must inherit tmpfiles.eclass then call tmpfiles_process in pkg_postinst."
-- 
2.32.0

[gentoo-dev] [PATCH v2 4/4] metadata/install-qa-check.d: add check for missing tmpfiles_process call

[Sam James]

From: Georgy Yakovlev <gyakovlev@gentoo.org>

See: https://archives.gentoo.org/gentoo-dev/message/7bdfdc9a7560fd07436defd0253af0b8
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
Signed-off-by: Sam James <sam@gentoo.org>
---
 metadata/install-qa-check.d/60tmpfiles-paths | 34 ++++++++++++++------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/metadata/install-qa-check.d/60tmpfiles-paths b/metadata/install-qa-check.d/60tmpfiles-paths
index 81286de584a2..aa666dfb7ce5 100644
--- a/metadata/install-qa-check.d/60tmpfiles-paths
+++ b/metadata/install-qa-check.d/60tmpfiles-paths
@@ -3,11 +3,14 @@
 
 # QA check: ensure that packages installing tmpfiles configuration inherit the eclass
 # Maintainer: Sam James <sam@gentoo.org>
+# Maintainer: Georgy Yakovlev <gyakovlev@gentoo.org>
 
 # Implements two checks:
 # 1) Installation to /etc/tmpfiles.d (which is a user-customization location);
 # 2) Installation of any tmpfiles to /usr/lib/tmpfiles.d without inheriting the eclass
-#    (needed for tmpfiles_process in pkg_postinst)
+#    (needed for tmpfiles_process in pkg_postinst);
+# 3) Check for installation of tmpfiles without calling tmpfiles_process in
+#    pkg_postinst.
 tmpfiles_check() {
 	# Check 1
 	# Scan image for files in /etc/tmpfiles.d which is a forbidden location
@@ -17,30 +20,41 @@ tmpfiles_check() {
 	shopt -u nullglob
 
 	if [[ ${#files[@]} -gt 0 ]]; then
-		eqawarn "QA Notice: files installed to /etc/tmpfiles.d"
-		eqawarn "tmpfiles configuration files must be installed by ebuilds /usr/lib/tmpfiles.d!"
+		eqawarn "QA Notice: files installed to /etc/tmpfiles.d found"
+		eqawarn "tmpfiles configuration files supplied by ebuilds must be installed to /usr/lib/tmpfiles.d"
 	fi
 
 	# Check 2
 	# We're now going to check for whether we install files to /usr/lib/tmpfiles.d without
 	# inheriting the eclass (weak catch for ebuilds not calling tmpfiles_process in pkg_postinst)
 
-	# No need to carry on if we're inheriting the eclass
-	if has tmpfiles ${INHERITED} ; then
-		return
-	fi
-
 	# It's okay for some packages to do this because of circular dependencies and such
 	# See: https://archives.gentoo.org/gentoo-dev/message/0a96793036a4fdd9ac311a46950d7e7b
 	# TODO: Standardize some way of allowing ebuilds to opt-out of checks like this
 	local package=${CATEGORY}/${PN}
+
 	if [[ ${package} == "sys-apps/systemd" || ${package} == "sys-libs/pam" ]] ; then
 		return
 	fi
 
 	if [[ -d "${ED}"/usr/lib/tmpfiles.d/ ]] ; then
-		eqawarn "QA Notice: package is installing tmpfiles without inheriting tmpfiles.eclass!"
-		eqawarn "Packages must inherit tmpfiles.eclass then call tmpfiles_process in pkg_postinst."
+		if ! has tmpfiles ${INHERITED} ; then
+			eqawarn "QA Notice: package is installing tmpfiles without inheriting tmpfiles.eclass!"
+			eqawarn "Packages must inherit tmpfiles.eclass then call tmpfiles_process in pkg_postinst."
+			return
+		fi
+
+		# Check 3
+		# Check whether we're installing tmpfiles without explicitly
+		# calling tmpfiles_process in pkg_postinst, but we have inherited
+		# the eclass.
+		# Small risk of false positives if called indirectly.
+		# See: https://archives.gentoo.org/gentoo-dev/message/7bdfdc9a7560fd07436defd0253af0b8
+		local pkg_postinst_body="$(declare -fp pkg_postinst 2>&1)"
+		if [[ ! ${pkg_postinst_body} == *tmpfiles_process* ]] ; then
+			eqawarn "QA Notice: package is installing tmpfiles without calling"
+			eqawarn "tmpfiles_process in pkg_postinst phase"
+		fi
 	fi
 }
 
-- 
2.32.0