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

[Daniel M. Capella]

On 5/23/21 7:18 AM, Denton Liu wrote:
> 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 at 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(-)
>
> 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] "
>
Pushed, thank you!

-- 
Best,
Daniel <https://danielcapella.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xEA4F7B321A906AD9.asc
Type: application/pgp-keys
Size: 15463 bytes
Desc: OpenPGP public key
URL: <https://lists.archlinux.org/pipermail/pacman-contrib/attachments/20210727/64e9c353/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-contrib/attachments/20210727/64e9c353/attachment.sig>