[pacman-dev] [PATCH] makepkg: refactor checking for write permissions into a utility function

Eli Schwartz eschwartz at archlinux.org
Sun Oct 8 07:05:10 UTC 2017


Additionally provide a separate error for the confusing if unlikely
event that the user tries to use an existing file as a package output
directory.

This also means we now consistently try to create any nonexistent *DEST
directories as needed before aborting with E_FS_PERMISSIONS. Previously
only $BUILDDIR received that kindness.

In the process, move the handling of $extra_environment[@] to where it
always belonged, above $BUILDDIR.

Fixes FS#43537

Signed-off-by: Eli Schwartz <eschwartz at archlinux.org>
---

Am I trying to do way, way too much here? mkdir will hopefully already
provide a somewhat useful error message in the case of an existing file,
as we don't quiet it.

And this will need to be rebased on top of escondida's error codes patch

 scripts/libmakepkg/util/util.sh.in | 19 +++++++++++++++++++
 scripts/makepkg.sh.in              | 29 +++++++++++------------------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in
index d676249d..a8813338 100644
--- a/scripts/libmakepkg/util/util.sh.in
+++ b/scripts/libmakepkg/util/util.sh.in
@@ -83,3 +83,22 @@ cd_safe() {
 		exit 1
 	fi
 }
+
+# Try to create directory if one does not yet exist. Fails if the directory
+# exists but has no write permissions, or if there is an existing file with
+# the same name.
+ensure_dir() {
+	local parent=$1 dirname=$1
+
+	until [[ -e $parent ]]; do
+		parent="${parent%/*}"
+	done
+	if [[ -f $parent ]]; then
+		error "$(gettext "Cannot create needed directory %s because it is already a file.")" "$parent"
+		return 1
+	fi
+	if ( [[ ! -d $dirname ]] && ! mkdir -p "$dirname" ) || [[ ! -w $dirname ]]; then
+		return 1
+	fi
+	return 0
+}
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 6449accd..b25e4106 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1359,32 +1359,25 @@ else
 	unset ALL_OFF BOLD BLUE GREEN RED YELLOW
 fi
 
+# override settings from extra variables on commandline, if any
+if (( ${#extra_environment[*]} )); then
+	export "${extra_environment[@]}"
+fi
+
 
 # override settings with an environment variable for batch processing
 BUILDDIR=${_BUILDDIR:-$BUILDDIR}
 BUILDDIR=${BUILDDIR:-$startdir} #default to $startdir if undefined
-if [[ ! -d $BUILDDIR ]]; then
-	if ! mkdir -p "$BUILDDIR"; then
-		error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR"
-		plain "$(gettext "Aborting...")"
-		exit 1
-	fi
-	chmod a-s "$BUILDDIR"
-fi
-if [[ ! -w $BUILDDIR ]]; then
+if ! ensure_dir "$BUILDDIR"; then
 	error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR"
 	plain "$(gettext "Aborting...")"
 	exit 1
 fi
-
-# override settings from extra variables on commandline, if any
-if (( ${#extra_environment[*]} )); then
-	export "${extra_environment[@]}"
-fi
+chmod a-s "$BUILDDIR"
 
 PKGDEST=${_PKGDEST:-$PKGDEST}
 PKGDEST=${PKGDEST:-$startdir} #default to $startdir if undefined
-if (( ! (NOBUILD || GENINTEG) )) && [[ ! -w $PKGDEST ]]; then
+if (( ! (NOBUILD || GENINTEG) )) && ! ensure_dir "$PKGDEST"; then
 	error "$(gettext "You do not have write permission to store packages in %s.")" "$PKGDEST"
 	plain "$(gettext "Aborting...")"
 	exit 1
@@ -1392,7 +1385,7 @@ fi
 
 SRCDEST=${_SRCDEST:-$SRCDEST}
 SRCDEST=${SRCDEST:-$startdir} #default to $startdir if undefined
-if [[ ! -w $SRCDEST ]] ; then
+if ! ensure_dir "$SRCDEST" ; then
 	error "$(gettext "You do not have write permission to store downloads in %s.")" "$SRCDEST"
 	plain "$(gettext "Aborting...")"
 	exit 1
@@ -1401,7 +1394,7 @@ fi
 SRCPKGDEST=${_SRCPKGDEST:-$SRCPKGDEST}
 SRCPKGDEST=${SRCPKGDEST:-$startdir} #default to $startdir if undefined
 if (( SOURCEONLY )); then
-	if [[ ! -w $SRCPKGDEST ]]; then
+	if ! ensure_dir "$SRCPKGDEST"; then
 		error "$(gettext "You do not have write permission to store source tarballs in %s.")" "$SRCPKGDEST"
 		plain "$(gettext "Aborting...")"
 		exit 1
@@ -1414,7 +1407,7 @@ fi
 
 LOGDEST=${_LOGDEST:-$LOGDEST}
 LOGDEST=${LOGDEST:-$startdir} #default to $startdir if undefined
-if (( LOGGING )) && [[ ! -w $LOGDEST ]]; then
+if (( LOGGING )) && ! ensure_dir "$LOGDEST"; then
 	error "$(gettext "You do not have write permission to store logs in %s.")" "$LOGDEST"
 	plain "$(gettext "Aborting...")"
 	exit 1
-- 
2.14.2


More information about the pacman-dev mailing list