[pacman-dev] [PATCH] 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\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
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\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
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\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
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\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
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\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
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\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".
participants (2)
-
Allan McRae
-
Dave Reisner