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

older
[pacman-dev] compiling pacman and...

Allan McRae

20 Jan 2012 20 Jan '12

5:24 p.m.

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 --- 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 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 return fi -- 1.7.8.4

Show replies by date

Dave Reisner

20 Jan 20 Jan

5:59 p.m.

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 ---

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.

...

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.

...

return fi

-- 1.7.8.4

Allan McRae

23 Jan 23 Jan

6:08 a.m.

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 ---

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

Dave Reisner

6:13 a.m.

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 ---

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

Allan McRae

6:48 a.m.

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 ---

...

...
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!

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

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

Dave Reisner

6:42 a.m.

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 ---

...

...
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!

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

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".

4483

Age (days ago)

4486

Last active (days ago)

List overview

Download

5 comments

2 participants

participants (2)

Allan McRae
Dave Reisner