Re: [pacman-dev] [PATCH] makepkg: fix error on unnecessary -r

[Dave Reisner]

On Mon, Jan 23, 2012 at 12:08:14PM +1000, Allan McRae wrote:
> On 20/01/12 23:59, Dave Reisner wrote:
> > On Fri, Jan 20, 2012 at 11:24:23PM +1000, Allan McRae wrote:
> >> The grep statement used to check for a difference between the
> >> installed package list before and after resolving dependencies
> >> returns 1 if there is no difference.  This sets of the error
> >> trap when "-r" is used "unnecessarily".
> >>
> >> Signed-off-by: Allan McRae <al...@archlinux.org>
> >> ---
> >>
> >> Dave: would you prefer this fix or just using $(set +E; grep ...)
> >>
> >>  scripts/makepkg.sh.in |    7 ++++---
> >>  1 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> >> index f6d8294..4792c5c 100644
> >> --- a/scripts/makepkg.sh.in
> >> +++ b/scripts/makepkg.sh.in
> >> @@ -507,14 +507,15 @@ remove_deps() {
> >>    # check for packages removed during dependency install (e.g. due to 
> >> conflicts)
> >>    # removing all installed packages is risky in this case
> >>    if [[ -n $(grep -xvFf <(printf '%s\n' "${current_packagelist[@]}") \
> >> -                  <(printf '%s\n' "${original_packagelist[@]}") ) ]]; then
> >> +                  <(printf '%s\n' "${original_packagelist[@]}") || true) 
> >> ]]; then
> >>            warning "$(gettext "Failed to remove installed dependencies.")"
> >>            return 0
> >>    fi
> > 
> > I'd prefer we fix the need for this hackery and not embed grep inside a
> > test.
> > 
> > if grep -qxvFf <(...) <(...); then
> > 
> > grep exits 0 when output is generated, and we can throw the warning.
> > Since it's guarded by an 'if' block, there's no need to worry about it
> > "failing" setting off the ERR trap.
> > 
> 
> Made this change on my working branch.
> 
> >>  
> >>    local deplist
> >> -  if ! deplist=($(grep -xvFf <(printf "%s\n" "${original_pkglist[@]}") \
> >> -                  <(printf "%s\n" "${current_pkglist[@]}"))); then
> >> +  deplist=($(grep -xvFf <(printf "%s\n" "${original_pkglist[@]}") \
> >> +                  <(printf "%s\n" "${current_pkglist[@]}") || true))
> >> +  if [[ -n deplist ]]; then
> > 
> > My brain tells me there's a way to avoid this, but I'm not sure I trust
> > it right now.
> > 
> 
> I have not thought of one...  let me know if your brain is working
> better now and has come up with the answer!
> 
> Allan
> 
> 

What about something like...

  local dep deplist
  while read -r dep; do
    deplist+=("$dep")
  done < <(grep -xvFf <(printf "%s\n" "${original_pkglist[@]}") \
      <(printf "%s\n" "${current_pkglist[@]}"))

  if (( ${#deplist[*]} )); then
    return
  fi

d

pgpvteCpoQjrB.pgp
Description: PGP signature