makepkg: fix error on unnecessary -r
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 <allan@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 ' "${current_packagelist[@]}") - <(printf '%s ' "${original_packagelist[@]}") ) ]]; then + <(printf '%s ' "${original_packagelist[@]}") || true) ]]; then warning "$(gettext "Failed to remove installed dependencies.")" return 0 fi local deplist - if ! deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") - <(printf "%s " "${current_pkglist[@]}"))); then + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") + <(printf "%s " "${current_pkglist[@]}") || true)) + if [[ -n deplist ]]; then return fi -- 1.7.8.4 |
makepkg: fix error on unnecessary -r
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 <allan@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 ' "${current_packagelist[@]}") > - <(printf '%s ' "${original_packagelist[@]}") ) ]]; then > + <(printf '%s ' "${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. > > local deplist > - if ! deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > - <(printf "%s " "${current_pkglist[@]}"))); then > + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > + <(printf "%s " "${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. > return > fi > > -- > 1.7.8.4 > > |
makepkg: fix error on unnecessary -r
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 <allan@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 ' "${current_packagelist[@]}") >> - <(printf '%s ' "${original_packagelist[@]}") ) ]]; then >> + <(printf '%s ' "${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 " "${original_pkglist[@]}") >> - <(printf "%s " "${current_pkglist[@]}"))); then >> + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") >> + <(printf "%s " "${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 |
makepkg: fix error on unnecessary -r
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 <allan@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 ' "${current_packagelist[@]}") > >> - <(printf '%s ' "${original_packagelist[@]}") ) ]]; then > >> + <(printf '%s ' "${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 " "${original_pkglist[@]}") > >> - <(printf "%s " "${current_pkglist[@]}"))); then > >> + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > >> + <(printf "%s " "${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 " "${original_pkglist[@]}") <(printf "%s " "${current_pkglist[@]}")) if (( ${#deplist[*]} )); then return fi d |
makepkg: fix error on unnecessary -r
On Mon, Jan 23, 2012 at 12:48:37PM +1000, Allan McRae wrote:
> On 23/01/12 12:13, Dave Reisner wrote: > > 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 <allan@archlinux.org> > >>>> --- > >>>> > ... > >>>> > >>>> local deplist > >>>> - if ! deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > >>>> - <(printf "%s " "${current_pkglist[@]}"))); then > >>>> + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > >>>> + <(printf "%s " "${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! > >> > >> > > > > What about something like... > > > > local dep deplist > > while read -r dep; do > > deplist+=("$dep") > > done < <(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > > <(printf "%s " "${current_pkglist[@]}")) > > > > if (( ${#deplist[*]} )); then > > return > > fi > > I'm not sure the whole "while read" construct is worth it to get rid of > the || true. I'll just leave my patch as it is for now unless you > really feel strongly that I should not do it that way... > > Allan > Yeah, I'm not sure either. Ideally we'd have something decent like an array_diff or array_subtract function, but passing arrays in bash is an undocumented feature and otherwise awful to do "properly". |
makepkg: fix error on unnecessary -r
On 23/01/12 12:13, Dave Reisner wrote:
> 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 <allan@archlinux.org> >>>> --- >>>> ... >>>> >>>> local deplist >>>> - if ! deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") >>>> - <(printf "%s " "${current_pkglist[@]}"))); then >>>> + deplist=($(grep -xvFf <(printf "%s " "${original_pkglist[@]}") >>>> + <(printf "%s " "${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! >> >> > > What about something like... > > local dep deplist > while read -r dep; do > deplist+=("$dep") > done < <(grep -xvFf <(printf "%s " "${original_pkglist[@]}") > <(printf "%s " "${current_pkglist[@]}")) > > if (( ${#deplist[*]} )); then > return > fi I'm not sure the whole "while read" construct is worth it to get rid of the || true. I'll just leave my patch as it is for now unless you really feel strongly that I should not do it that way... Allan |
| All times are GMT. The time now is 08:30 PM. |
VBulletin, Copyright ©2000 - 2013, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.