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 <[email protected]> > ---
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_pkgli...@]}") \ > + <(printf "%s\n" "${current_pkgli...@]}") | 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_pkgli...@]} \ | comm -23 - <(printf "%s\n" ${current_pkgli...@]})) ]] Note the code above is untested. > - msg "Removing installed dependencies..." > + local deplist=($(diff <(printf "%s\n" "${original_pkgli...@]}") \ > + <(printf "%s\n" "${current_pkgli...@]}") | grep ">")) > + deplist=(${depli...@]#>}) local deplist=($(printf "%s\n" ${original_pkgli...@]} \ | comm -13 - <(printf "%s\n" ${current_pkgli...@]}))) > + [ ${#depli...@]} -eq 0 ] && return (( ${#depli...@]} == 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 ${depli...@]}; 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 ${makedepen...@]} || 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
