[v2] pacdiff: Don't use $SUDO on temporary files

Message ID ad45bdd80d6b4bcfa945f48737c0f9f6577ed9e1.1617335164.git.liu.denton@gmail.com
State New
Headers show
Series [v2] pacdiff: Don't use $SUDO on temporary files | expand

Commit Message

Denton Liu April 2, 2021, 3:46 a.m. UTC
In 19ab4fa (pacdiff: Add option to use sudo/sudoedit to manage files,
2021-03-27), pacdiff was taught to accept -s to run various commands
with $SUDO. This introduced many instances of $SUDO in merge_file()
where most of them are unnecessary.

In particular, it is not necessary to $SUDO to write the temporary files
as /tmp should be writable by all[0][1].

Also, the usage of sudoedit when comparing the original file with the
merge result is unnecessary. This is because root permissions are not
really required since users should not write to the original file
anyway. The merged file will be used to overwrite the original file at
the end of the function anyway.

Remove these unnecessary usages of $SUDO.

[0]: https://unix.stackexchange.com/a/71625
[1]: https://serverfault.com/a/427290

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Range-diff against v1:
1:  ab12657 < -:  ------- .mailmap: Map Daniel Parks
2:  9287d4e < -:  ------- .mailmap: Map Denton Liu
3:  636e0fb < -:  ------- pacdiff: update --sudo usages
4:  32bb4a1 ! 1:  ad45bdd pacdiff: Don't use $SUDO on temporary files
    @@ Commit message
         where most of them are unnecessary.
     
         In particular, it is not necessary to $SUDO to write the temporary files
    -    as /tmp should be writable by all.
    +    as /tmp should be writable by all[0][1].
     
    -    Also, remove the usage of sudoedit when comparing the original file with
    -    the merge result. This is because the merged file is placed in a
    -    writable directory. Attempting to run sudoedit on this file results in
    -    the following error:
    -
    -            sudoedit: <file>: editing files in a writable directory is not permitted
    -
    -    but root permissions are not really required since users should not
    -    write to the original file anyway. The merged file will be used to
    -    overwrite the original file at the end of the function anyway.
    +    Also, the usage of sudoedit when comparing the original file with the
    +    merge result is unnecessary. This is because root permissions are not
    +    really required since users should not write to the original file
    +    anyway. The merged file will be used to overwrite the original file at
    +    the end of the function anyway.
     
         Remove these unnecessary usages of $SUDO.
     
    +    [0]: https://unix.stackexchange.com/a/71625
    +    [1]: https://serverfault.com/a/427290
    +
      ## src/pacdiff.sh.in ##
     @@ src/pacdiff.sh.in: merge_file() {
      	fi
    @@ src/pacdiff.sh.in: merge_file() {
     +	merged="$(mktemp "$tempdir"/"$basename.merged.XXX")"
      
     -	tar -xOf "$base_tar" "${file#/}" | $SUDO tee "$base" > /dev/null
    --	if $SUDO $mergeprog "$file" "$base" "$pacfile" | $SUDO tee "$merged" > /dev/null; then
    +-	$SUDO $mergeprog "$file" "$base" "$pacfile" | $SUDO tee "$merged" > /dev/null
    +-	if [ "${PIPESTATUS[0]}" -ne "1" ]; then
     +	tar -xOf "$base_tar" "${file#/}" >"$base"
     +	if $mergeprog "$file" "$base" "$pacfile" >"$merged"; then
      		msg2 "Merged without conflicts."

 src/pacdiff.sh.in | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Patch

diff --git a/src/pacdiff.sh.in b/src/pacdiff.sh.in
index 5af7e1c..3c34fdb 100644
--- a/src/pacdiff.sh.in
+++ b/src/pacdiff.sh.in
@@ -117,21 +117,16 @@  merge_file() {
 	fi
 
 	basename="$(basename "$file")"
-	tempdir="$($SUDO mktemp -d --tmpdir "pacdiff-merge-$basename.XXX")"
-	base="$($SUDO mktemp "$tempdir"/"$basename.base.XXX")"
-	merged="$($SUDO mktemp "$tempdir"/"$basename.merged.XXX")"
+	tempdir="$(mktemp -d --tmpdir "pacdiff-merge-$basename.XXX")"
+	base="$(mktemp "$tempdir"/"$basename.base.XXX")"
+	merged="$(mktemp "$tempdir"/"$basename.merged.XXX")"
 
-	tar -xOf "$base_tar" "${file#/}" | $SUDO tee "$base" > /dev/null
-	$SUDO $mergeprog "$file" "$base" "$pacfile" | $SUDO tee "$merged" > /dev/null
-	if [ "${PIPESTATUS[0]}" -ne "1" ]; then
+	tar -xOf "$base_tar" "${file#/}" >"$base"
+	if $mergeprog "$file" "$base" "$pacfile" >"$merged"; then
 		msg2 "Merged without conflicts."
 	fi
 
-	if [[ -n "$SUDO" ]]; then
-		SUDO_EDITOR="$diffprog" sudoedit "$file" "$merged"
-	else
-		$diffprog "$file" "$merged"
-	fi
+	$diffprog "$file" "$merged"
 
 	while :; do
 		ask "Would you like to use the results of the merge? [y/n] "