[pacman-dev] [PATCH] makepkg: avoid using comm for diff'ing package lists
Whereas comm will check inputs to see if they're sorted (and warn when they aren't), grep doesn't even care about ordering. In this particular instance -- neither do we. We're only interested that the two lists are equivalent. Fixes FS#26580. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/makepkg.sh.in | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 26a2789..0fa05dc 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -506,15 +506,17 @@ 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 $(comm -23 <(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}")) ]]; then - warning "$(gettext "Failed to remove installed dependencies.")" - return 0 + if [[ -n $(grep -xvFf <(printf '%s\n' "${current_packagelist[@]}") \ + <(printf '%s\n' "${original_packagelist[@]}") ) ]]; then + warning "$(gettext "Failed to remove installed dependencies.")" + return 0 fi - local deplist=($(comm -13 <(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}"))) - (( ${#deplist[@]} == 0 )) && return + local deplist + if ! deplist=($(grep -xvFf <(printf "%s\n" "${original_pkglist[@]}") \ + <(printf "%s\n" "${current_pkglist[@]}"))); then + return + fi msg "Removing installed dependencies..." # exit cleanly on failure to remove deps as package has been built successfully -- 1.7.7.4
On 22/11/11 15:02, Dave Reisner wrote:
Whereas comm will check inputs to see if they're sorted (and warn when they aren't), grep doesn't even care about ordering. In this particular instance -- neither do we. We're only interested that the two lists are equivalent.
Fixes FS#26580.
Signed-off-by: Dave Reisner<dreisner@archlinux.org> ---
I had not seen this when I sent my patch. Would comm not be more efficient than grep for doing this?
scripts/makepkg.sh.in | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 26a2789..0fa05dc 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -506,15 +506,17 @@ 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 $(comm -23<(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}")) ]]; then - warning "$(gettext "Failed to remove installed dependencies.")" - return 0 + if [[ -n $(grep -xvFf<(printf '%s\n' "${current_packagelist[@]}") \ + <(printf '%s\n' "${original_packagelist[@]}") ) ]]; then + warning "$(gettext "Failed to remove installed dependencies.")" + return 0 fi
- local deplist=($(comm -13<(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}"))) - (( ${#deplist[@]} == 0 ))&& return + local deplist + if ! deplist=($(grep -xvFf<(printf "%s\n" "${original_pkglist[@]}") \ + <(printf "%s\n" "${current_pkglist[@]}"))); then + return + fi
msg "Removing installed dependencies..." # exit cleanly on failure to remove deps as package has been built successfully
On Tue, Nov 22, 2011 at 12:33 AM, Allan McRae <allan@archlinux.org> wrote:
On 22/11/11 15:02, Dave Reisner wrote:
Whereas comm will check inputs to see if they're sorted (and warn when they aren't), grep doesn't even care about ordering. In this particular instance -- neither do we. We're only interested that the two lists are equivalent.
Fixes FS#26580.
Signed-off-by: Dave Reisner<dreisner@archlinux.org> ---
I had not seen this when I sent my patch. Would comm not be more efficient than grep for doing this? I don't think your patch actually fixes the underlying problem, which is that the sorts need to come back too and be consistent with whatever we tell comm it should expect. No matter what, makepkg shouldn't really assume anything regarding the ordering of pacman output- The LC_COLLATE=C should be added to both the comm calls and re-add a `| LC_COLLATE=C sort` call to -Qq output.
grep avoids this completely; ordering doesn't matter at all. Perhaps it is a bit less efficient, but this is 1) an operation that runs once, and 2) the grep call is sticking to POSIX-conforming flags.
scripts/makepkg.sh.in | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 26a2789..0fa05dc 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -506,15 +506,17 @@ 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 $(comm -23<(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}")) ]]; then - warning "$(gettext "Failed to remove installed dependencies.")" - return 0 + if [[ -n $(grep -xvFf<(printf '%s\n' "${current_packagelist[@]}") \ + <(printf '%s\n' "${original_packagelist[@]}") ) ]]; then + warning "$(gettext "Failed to remove installed dependencies.")" + return 0 fi
- local deplist=($(comm -13<(printf "%s\n" "${original_pkglist[@]}") \ - <(printf "%s\n" "${current_pkglist[@]}"))) - (( ${#deplist[@]} == 0 ))&& return + local deplist + if ! deplist=($(grep -xvFf<(printf "%s\n" "${original_pkglist[@]}") \ + <(printf "%s\n" "${current_pkglist[@]}"))); then + return + fi
msg "Removing installed dependencies..." # exit cleanly on failure to remove deps as package has been built successfully
On 22/11/11 16:47, Dan McGee wrote:
On Tue, Nov 22, 2011 at 12:33 AM, Allan McRae<allan@archlinux.org> wrote:
On 22/11/11 15:02, Dave Reisner wrote:
Whereas comm will check inputs to see if they're sorted (and warn when they aren't), grep doesn't even care about ordering. In this particular instance -- neither do we. We're only interested that the two lists are equivalent.
Fixes FS#26580.
Signed-off-by: Dave Reisner<dreisner@archlinux.org> ---
I had not seen this when I sent my patch. Would comm not be more efficient than grep for doing this? I don't think your patch actually fixes the underlying problem, which is that the sorts need to come back too and be consistent with whatever we tell comm it should expect. No matter what, makepkg shouldn't really assume anything regarding the ordering of pacman output- The LC_COLLATE=C should be added to both the comm calls and re-add a `| LC_COLLATE=C sort` call to -Qq output.
grep avoids this completely; ordering doesn't matter at all. Perhaps it is a bit less efficient, but this is 1) an operation that runs once, and 2) the grep call is sticking to POSIX-conforming flags.
OK, I'm convinced. Ack-by-Allan
On Tue, Nov 22, 2011 at 4:23 AM, Allan McRae <allan@archlinux.org> wrote:
On 22/11/11 16:47, Dan McGee wrote:
On Tue, Nov 22, 2011 at 12:33 AM, Allan McRae<allan@archlinux.org> wrote:
On 22/11/11 15:02, Dave Reisner wrote:
Whereas comm will check inputs to see if they're sorted (and warn when they aren't), grep doesn't even care about ordering. In this particular instance -- neither do we. We're only interested that the two lists are equivalent.
Fixes FS#26580.
Signed-off-by: Dave Reisner<dreisner@archlinux.org> ---
I had not seen this when I sent my patch. Would comm not be more efficient than grep for doing this?
I don't think your patch actually fixes the underlying problem, which is that the sorts need to come back too and be consistent with whatever we tell comm it should expect. No matter what, makepkg shouldn't really assume anything regarding the ordering of pacman output- The LC_COLLATE=C should be added to both the comm calls and re-add a `| LC_COLLATE=C sort` call to -Qq output.
grep avoids this completely; ordering doesn't matter at all. Perhaps it is a bit less efficient, but this is 1) an operation that runs once, and 2) the grep call is sticking to POSIX-conforming flags.
OK, I'm convinced.
Ack-by-Allan
I should add I basically suggested everything your patch did, before Dave presented this solution. :) I'll stick this on maint. -Dan
participants (3)
-
Allan McRae
-
Dan McGee
-
Dave Reisner