diff mbox

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

Message ID 20180208045601.18949-1-eschwartz@archlinux.org
State Accepted, archived
Headers show

Commit Message

Eli Schwartz Feb. 8, 2018, 4:56 a.m. UTC
Additionally provide a separate error for failure to create the
directory vs lack of write permissions on a pre-existing 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.

Fixes FS#43537

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

v4: instead of complex directory/file checking, just abort on
"failure to create a directory"

 scripts/libmakepkg/util/util.sh.in | 17 +++++++++++++++++
 scripts/makepkg.sh.in              | 25 +++++++------------------
 2 files changed, 24 insertions(+), 18 deletions(-)

Comments

Allan McRae March 14, 2018, 7:48 a.m. UTC | #1
On 08/02/18 14:56, Eli Schwartz wrote:
> Additionally provide a separate error for failure to create the
> directory vs lack of write permissions on a pre-existing 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.
> 
> Fixes FS#43537
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> 
> v4: instead of complex directory/file checking, just abort on
> "failure to create a directory"
> 


This version looks good.  Pulled to my patchqueue branch.

A
diff mbox

Patch

diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in
index d676249d..6ff8f406 100644
--- a/scripts/libmakepkg/util/util.sh.in
+++ b/scripts/libmakepkg/util/util.sh.in
@@ -83,3 +83,20 @@  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_writable_dir() {
+	local dirtype="$1" dirpath="$2"
+
+	if ! mkdir -p "$dirpath" 2>/dev/null; then
+		if [[ -d $dirpath && ! -w $dirpath ]]; then
+			error "$(gettext "You do not have write permission for the directory \$%s (%s).")" "$dirtype" "$dirpath"
+		else
+			error "$(gettext "Failed to create the directory \$%s (%s).")" "$dirtype" "$dirpath"
+		fi
+		return 1
+	fi
+	return 0
+}
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 837d4362..a22bcce2 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1361,35 +1361,25 @@  else
 fi
 
 
-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 $E_FS_PERMISSIONS
-	fi
-	chmod a-s "$BUILDDIR"
-fi
-if [[ ! -w $BUILDDIR ]]; then
-	error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR"
+# check that all settings directories are user-writable
+if ! ensure_writable_dir "BUILDDIR" "$BUILDDIR"; then
 	plain "$(gettext "Aborting...")"
 	exit $E_FS_PERMISSIONS
 fi
+chmod a-s "$BUILDDIR"
 
-if (( ! (NOBUILD || GENINTEG) )) && [[ ! -w $PKGDEST ]]; then
-	error "$(gettext "You do not have write permission to store packages in %s.")" "$PKGDEST"
+if (( ! (NOBUILD || GENINTEG) )) && ! ensure_writable_dir "PKGDEST" "$PKGDEST"; then
 	plain "$(gettext "Aborting...")"
 	exit $E_FS_PERMISSIONS
 fi
 
-if [[ ! -w $SRCDEST ]] ; then
-	error "$(gettext "You do not have write permission to store downloads in %s.")" "$SRCDEST"
+if ! ensure_writable_dir "SRCDEST" "$SRCDEST" ; then
 	plain "$(gettext "Aborting...")"
 	exit $E_FS_PERMISSIONS
 fi
 
 if (( SOURCEONLY )); then
-	if [[ ! -w $SRCPKGDEST ]]; then
-		error "$(gettext "You do not have write permission to store source tarballs in %s.")" "$SRCPKGDEST"
+	if ! ensure_writable_dir "SRCPKGDEST" "$SRCPKGDEST"; then
 		plain "$(gettext "Aborting...")"
 		exit $E_FS_PERMISSIONS
 	fi
@@ -1399,8 +1389,7 @@  if (( SOURCEONLY )); then
 	IGNOREARCH=1
 fi
 
-if (( LOGGING )) && [[ ! -w $LOGDEST ]]; then
-	error "$(gettext "You do not have write permission to store logs in %s.")" "$LOGDEST"
+if (( LOGGING )) && ! ensure_writable_dir "LOGDEST" "$LOGDEST"; then
 	plain "$(gettext "Aborting...")"
 	exit $E_FS_PERMISSIONS
 fi