[gentoo-dev] [PATCH 1/4] desktop.eclass: domenu, fix dying on non-existing files

[gentoo-dev] [PATCH 1/4] desktop.eclass: domenu, fix dying on non-existing files

[Michał Górny]

The weird logic in domenu had an explicit separate clause
for unsuccessful return on non-existing files.  This worked fine before
EAPI 4 since '|| die' was mandatory.  However, since 'doins' started
dying on its own, developers have assumed the same for 'domenu'
and stopped checking the exit status.  As a result, missing files
are now silently ignored.

Change the logic to explicitly die when the file does not exist.
To provide the best interoperability and avoid code duplication, just
let 'doins' die on its own.
---
 eclass/desktop.eclass | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/eclass/desktop.eclass b/eclass/desktop.eclass
index 91521b85a821..1684a21d21f7 100644
--- a/eclass/desktop.eclass
+++ b/eclass/desktop.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: desktop.eclass
@@ -248,16 +248,14 @@ domenu() {
 	insopts -m 0644
 	insinto /usr/share/applications
 	for i in "$@" ; do
-		if [[ -f ${i} ]] ; then
-			doins "${i}"
-			((ret+=$?))
-		elif [[ -d ${i} ]] ; then
+		if [[ -d ${i} ]] ; then
 			for j in "${i}"/*.desktop ; do
 				doins "${j}"
 				((ret+=$?))
 			done
 		else
-			((++ret))
+			doins "${i}"
+			((ret+=$?))
 		fi
 	done
 	exit ${ret}
-- 
2.18.0

[gentoo-dev] [PATCH 2/4] desktop.eclass: domenu, fix potential overflow in exit status

[Michał Górny]

While increasing exit status for each failure may seem brilliant
at first, it serves no purpose and has an overflow risk.  For example,
if domenu counted 256 failures, the exit status would be truncated to 0
(success).
---
 eclass/desktop.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/desktop.eclass b/eclass/desktop.eclass
index 1684a21d21f7..8f2c6d55c293 100644
--- a/eclass/desktop.eclass
+++ b/eclass/desktop.eclass
@@ -251,11 +251,11 @@ domenu() {
 		if [[ -d ${i} ]] ; then
 			for j in "${i}"/*.desktop ; do
 				doins "${j}"
-				((ret+=$?))
+				((ret|=$?))
 			done
 		else
 			doins "${i}"
-			((ret+=$?))
+			((ret|=$?))
 		fi
 	done
 	exit ${ret}
-- 
2.18.0

[gentoo-dev] [PATCH 3/4] desktop.eclass: domenu, remove unnecessary nested loop

[Michał Górny]

---
 eclass/desktop.eclass | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/eclass/desktop.eclass b/eclass/desktop.eclass
index 8f2c6d55c293..08899b4a4607 100644
--- a/eclass/desktop.eclass
+++ b/eclass/desktop.eclass
@@ -244,15 +244,13 @@ domenu() {
 	(
 	# wrap the env here so that the 'insinto' call
 	# doesn't corrupt the env of the caller
-	local i j ret=0
+	local i ret=0
 	insopts -m 0644
 	insinto /usr/share/applications
 	for i in "$@" ; do
 		if [[ -d ${i} ]] ; then
-			for j in "${i}"/*.desktop ; do
-				doins "${j}"
-				((ret|=$?))
-			done
+			doins "${i}"/*.desktop
+			((ret|=$?))
 		else
 			doins "${i}"
 			((ret|=$?))
-- 
2.18.0

[gentoo-dev] [PATCH 4/4] desktop.eclass: Add missing ||die when writing to files

[Michał Górny]

---
 eclass/desktop.eclass | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/eclass/desktop.eclass b/eclass/desktop.eclass
index 08899b4a4607..6fc72ab8ec03 100644
--- a/eclass/desktop.eclass
+++ b/eclass/desktop.eclass
@@ -174,7 +174,7 @@ make_desktop_entry() {
 		icon=${icon%.*}
 	fi
 
-	cat <<-EOF > "${desktop}"
+	cat <<-EOF > "${desktop}" || die
 	[Desktop Entry]
 	Name=${name}
 	Type=Application
@@ -190,7 +190,9 @@ make_desktop_entry() {
 		ewarn "make_desktop_entry: update your 5th arg to read Path=${fields}"
 		fields="Path=${fields}"
 	fi
-	[[ -n ${fields} ]] && printf '%b\n' "${fields}" >> "${desktop}"
+	if [[ -n ${fields} ]]; then
+		printf '%b\n' "${fields}" >> "${desktop}" || die
+	fi
 
 	(
 		# wrap the env here so that the 'insinto' call
@@ -217,7 +219,7 @@ make_session_desktop() {
 	local desktop=${T}/${wm:-${PN}}.desktop
 	shift 2
 
-	cat <<-EOF > "${desktop}"
+	cat <<-EOF > "${desktop}" || die
 	[Desktop Entry]
 	Name=${title}
 	Comment=This session logs you into ${title}
-- 
2.18.0

Re: [gentoo-dev] [PATCH 1/4] desktop.eclass: domenu, fix dying on non-existing files

[Mike Gilbert]

On Thu, Jul 26, 2018 at 7:35 AM Michał Górny <mgorny@gentoo.org> wrote:
>
> The weird logic in domenu had an explicit separate clause
> for unsuccessful return on non-existing files.  This worked fine before
> EAPI 4 since '|| die' was mandatory.  However, since 'doins' started
> dying on its own, developers have assumed the same for 'domenu'
> and stopped checking the exit status.  As a result, missing files
> are now silently ignored.
>
> Change the logic to explicitly die when the file does not exist.
> To provide the best interoperability and avoid code duplication, just
> let 'doins' die on its own.

This patch series looks good to me.