[pacman-dev] [PATCH] makepkg: improve removal of installed dependencies

[Cedric Staniewski]

On 12/03/2009 03:33 PM, Allan McRae wrote:
> Compare a list of packages on the system before and after dependency resolution
> in order to get a complete list of packages to remove.  This allows makepkg to
> remove packages installed due to provides.
> 
> Bail in cases where packages that were on the system originally have been
> removed as there is a risk of breaking the system when removing the new
> packages
> 
> Fixes FS#15144
> 
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---

I have not tested the patch yet, so just some comments on the code for now.

> 
> This moves the generation of the installed package list to directly after
> the installation of deps to prevent issues if the user installs packages
> while the build is occurring:
> http://bugs.archlinux.org/task/15144#comment53938
> 
> Rebased on Cedric's run_pacman and $PACMAN patches.
> 
>  doc/makepkg.8.txt     |    6 +++---
>  scripts/makepkg.sh.in |   39 +++++++++++++++++++++------------------
>  2 files changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
> index ad27bca..168b369 100644
> --- a/doc/makepkg.8.txt
> +++ b/doc/makepkg.8.txt
> @@ -166,9 +166,9 @@ Environment Variables
>  ---------------------
>  *PACMAN*::
>  	The command that will be used to check for missing dependencies and to
> -	install and remove packages. Pacman's -U, -T, -S and -Rns operations
> -	must be supported by this command. If the variable is not set or
> -	empty, makepkg will fall back to `pacman'.
> +	install and remove packages. Pacman's -Qq, -Rns, -S, -T, and -U
> +	operations must be supported by this command. If the variable is not
> +	set or empty, makepkg will fall back to `pacman'.
>  
>  
>  Additional Features
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index c730150..5743ea4 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -27,8 +27,10 @@
>  
>  # makepkg uses quite a few external programs during its execution. You
>  # need to have at least the following installed for makepkg to function:
> -#   bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils),
> -#   getopt (util-linux), gettext, grep, gzip, openssl, sed, tput (ncurses)
> +#   bsdtar (libarchive), bzip2, coreutils, diff (diffutils), fakeroot,
> +#   find (findutils), getopt (util-linux), gettext, grep, gzip, openssl,
> +#   sed, tput (ncurses)
> +
>  
>  # gettext initialization
>  export TEXTDOMAIN='pacman'
> @@ -343,7 +345,7 @@ download_file() {
>  
>  run_pacman() {
>  	local ret=0
> -	if (( ! ASROOT )) && [[ $1 != "-T" ]] && sudo -l $PACMAN &>/dev/null; then
> +	if (( ! ASROOT )) && [[ $1 != "-T" && $1 != "-Qq" ]] && sudo -l $PACMAN &>/dev/null; then
>  		sudo $PACMAN $PACMAN_OPTS "$@" || ret=$?
>  	else
>  		$PACMAN $PACMAN_OPTS "$@" || ret=$?
> @@ -398,7 +400,6 @@ handle_deps() {
>  }
>  
>  resolve_deps() {
> -	# $pkgdeps is a GLOBAL variable, used by remove_deps()
>  	local R_DEPS_SATISFIED=0
>  	local R_DEPS_MISSING=1
>  
> @@ -408,7 +409,6 @@ resolve_deps() {
>  	fi
>  
>  	if handle_deps $deplist; then
> -		pkgdeps="$pkgdeps $deplist"
>  		# check deps again to make sure they were resolved
>  		deplist="$(check_deps $*)"
>  		[[ -z $deplist ]] && return $R_DEPS_SATISFIED
> @@ -425,23 +425,25 @@ resolve_deps() {
>  	return $R_DEPS_MISSING
>  }
>  
> -# fix flyspray bug #5923
>  remove_deps() {
> -	# $pkgdeps is a GLOBAL variable, set by resolve_deps()
>  	(( ! RMDEPS )) && return
> -	[[ -z $pkgdeps ]] && return
>  
> -	local dep depstrip deplist
> -	deplist=""
> -	for dep in $pkgdeps; do
> -		depstrip="${dep%%[<=>]*}"
> -		deplist="$deplist $depstrip"
> -	done
> +	# check for packages removed during dependency install (e.g. due to conflicts)
> +	# removing all installed packages is risky in this case
> +	if (( $(diff <(printf "%s\n" "${original_pkglist[@]}") \
> +		<(printf "%s\n" "${current_pkglist[@]}") | grep "<" | wc -l) > 0 )); then
> +	  warning "$(gettext "Failed to remove installed dependencies.")"
> +	  return 0
> +	fi
>

You could use comm here. The advantage would be that it do not pull in
a new dependency as it is in coreutils too and you do not need grep to
get the unique lines.

if [[ -n $(printf "%s\n" ${original_pkglist[@]} \
	| comm -23 - <(printf "%s\n" ${current_pkglist[@]})) ]]

Note the code above is untested.

> -	msg "Removing installed dependencies..."
> +	local deplist=($(diff <(printf "%s\n" "${original_pkglist[@]}") \
> +		<(printf "%s\n" "${current_pkglist[@]}") | grep ">"))
> +	deplist=(${deplist[@]#>})

local deplist=($(printf "%s\n" ${original_pkglist[@]} \
	| comm -13 - <(printf "%s\n" ${current_pkglist[@]})))

> +	[ ${#deplist[@]} -eq 0 ] && return

(( ${#deplist[@]} == 0 )) :)

> 
> +	msg "Removing installed dependencies..."
>  	# exit cleanly on failure to remove deps as package has been built successfully
> -	if ! run_pacman -Rns $deplist; then
> +	if ! run_pacman -Rn ${deplist[@]}; then
>  		warning "$(gettext "Failed to remove installed dependencies.")"
>  		return 0
>  	fi
> @@ -1874,14 +1876,13 @@ if (( SOURCEONLY )); then
>  	exit 0
>  fi
>  
> -# fix flyspray bug #5973
>  if (( NODEPS || NOBUILD || REPKG )); then
>  	# no warning message needed for nobuild, repkg
>  	if (( NODEPS )); then
>  		warning "$(gettext "Skipping dependency checks.")"
>  	fi
>  elif [ $(type -p "${PACMAN%% *}") ]; then
> -	unset pkgdeps # Set by resolve_deps() and used by remove_deps()
> +	original_pkglist=($(run_pacman -Qq))    # required by remove_deps

Could be skipped if RMDEPS == 0 but I do not know how fast / slow this
pacman operation actually is.

>  	deperr=0
>  
>  	msg "$(gettext "Checking Runtime Dependencies...")"
> @@ -1890,6 +1891,8 @@ elif [ $(type -p "${PACMAN%% *}") ]; then
>  	msg "$(gettext "Checking Buildtime Dependencies...")"
>  	resolve_deps ${makedepends[@]} || deperr=1
>  
> +	current_pkglist=($(run_pacman -Qq))    # required by remove_deps
> +

Same as above.

>  	if (( deperr )); then
>  		error "$(gettext "Could not resolve all dependencies.")"
>  		exit 1