[pacman-dev] [PATCH] Added --dont-downgrade flag to repo-add

newer
[pacman-dev] [PATCH] Added flag...

older
[pacman-dev] [GIT] The official...

ekarndam＠autistici.org

28 Apr 2019 28 Apr '19

8:07 p.m.

From: ekardnam Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0 # Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")" elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then + + local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then + warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;; -- 2.21.0

Show replies by date

Eli Schwartz

28 Apr 28 Apr

9:06 p.m.

On 4/28/19 2:07 PM, ekarndam@autistici.org wrote:

...

From: ekardnam

Fixes FS#17752

...

Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0

# Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")"

I think I too prefer --no-downgrade.

...

elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then +

Trailing whitespace here, could be removed. Some editors can trim this automatically, so can allan when applying. :D

...

+ local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then

Neither of these actually need to be quoted, although sure, quoting doesn't hurt. But one of them is an integer literal, and vercmp is guaranteed to produce another integer literal, so there is no word splitting, globbing, or parameter expansion to worry about.

...

+ warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;;

Thanks for the patch. Aside for these style nits and possible bikeshedding over the name of the option, the patch looks good to me, and works as expected. -- Eli Schwartz Bug Wrangler and Trusted User

Eli Schwartz

9:11 p.m.

...

...
+ local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then

Neither of these actually need to be quoted, although sure, quoting doesn't hurt. But one of them is an integer literal, and vercmp is guaranteed to produce another integer literal, so there is no word splitting, globbing, or parameter expansion to worry about. ... although since we're not exactly linting the versions to make sure

On 4/28/19 3:06 PM, Eli Schwartz wrote: they are valid, the arguments to vercmp really should be quoted. A package with metadata which makepkg does not allow, could still theoretically be created in some other manner; pacman itself is very forgiving of this. (As mentioned elsewhere, it would be nice if repo-add made use of the linting rules written for makepkg...) -- Eli Schwartz Bug Wrangler and Trusted User

ekardnam

30 Apr 30 Apr

8:32 a.m.

I'm totally going to follow this suggestions, thanks! When it comes to bash I always have doubts about quoting. I'll wait a bit before resubmitting to see opinions about the flag name decision (--no-downgrade or whatever) if that's fine. :) ekarndam On 28/04/19 19:11, Eli Schwartz wrote:

...

...
...
+ local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then Neither of these actually need to be quoted, although sure, quoting doesn't hurt. But one of them is an integer literal, and vercmp is guaranteed to produce another integer literal, so there is no word splitting, globbing, or parameter expansion to worry about. ... although since we're not exactly linting the versions to make sure

On 4/28/19 3:06 PM, Eli Schwartz wrote: they are valid, the arguments to vercmp really should be quoted. A package with metadata which makepkg does not allow, could still theoretically be created in some other manner; pacman itself is very forgiving of this.

(As mentioned elsewhere, it would be nice if repo-add made use of the linting rules written for makepkg...)

Allan McRae

8 May 8 May

2:26 a.m.

On 29/4/19 4:07 am, ekarndam@autistici.org wrote:

...

From: ekardnam

Adding a commit message with a brief description of what this patch does would be great. Also add a line with the number of the bug it fixes. I do not like --dont-downgrade because of the "dont". Abbreviations should be avoided. I'm also not a fan of "--no-downgrade" How about "--prevent-downgrades" and adding a short option? "-p" or "-d" would work. Otherwise, happy with the patch. Quoting the arguments to vercmp would not hurt. Thanks, Allan

...

Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0

# Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")" elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then + + local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then + warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;;

Eli Schwartz

2:47 a.m.

On 5/7/19 8:26 PM, Allan McRae wrote:

...

On 29/4/19 4:07 am, ekarndam@autistici.org wrote:

...
From: ekardnam

Adding a commit message with a brief description of what this patch does would be great. Also add a line with the number of the bug it fixes.

I do not like --dont-downgrade because of the "dont". Abbreviations should be avoided. I'm also not a fan of "--no-downgrade"

How about "--prevent-downgrades" and adding a short option? "-p" or "-d" would work.

The "p" puts emphasis on the prevention, the "d" puts emphasis on the downgrade. So I prefer the former. The "d" also collides with the --delta option that was just removed, which would have surprising results in scripts that still use -d, while not being particularly advantageous as a shortopt name. -- Eli Schwartz Bug Wrangler and Trusted User

Allan McRae

2:52 a.m.

On 8/5/19 10:47 am, Eli Schwartz wrote:

...

On 5/7/19 8:26 PM, Allan McRae wrote:

...
On 29/4/19 4:07 am, ekarndam@autistici.org wrote:

...
From: ekardnam

Adding a commit message with a brief description of what this patch does would be great. Also add a line with the number of the bug it fixes.

I do not like --dont-downgrade because of the "dont". Abbreviations should be avoided. I'm also not a fan of "--no-downgrade"

How about "--prevent-downgrades" and adding a short option? "-p" or "-d" would work.

The "p" puts emphasis on the prevention, the "d" puts emphasis on the downgrade. So I prefer the former.

The "d" also collides with the --delta option that was just removed, which would have surprising results in scripts that still use -d, while not being particularly advantageous as a shortopt name.

Good point about deltas. Go with "-p" as the short option. A

ekarndam＠autistici.org

9:15 a.m.

New subject: [pacman-dev] [PATCH] Added flag --prevent-downgrade to repo-add to prevent adding packages to a database if a newer one is already present.

From: ekardnam Reference bug in the bug tracker is at #FS17752 Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0 # Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")" elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then + + local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then + warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;; -- 2.21.0

Allan McRae

9:55 a.m.

New subject: [pacman-dev] [PATCH] Added flag --prevent-downgrade to repo-add to prevent adding packages to a database if a newer one is already present.

On 8/5/19 5:15 pm, ekarndam@autistici.org wrote:

...

From: ekardnam

Reference bug in the bug tracker is at #FS17752

Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0

Everything is still --dont-downgrade. Not --prevent-downgrade.

...

# Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")" elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then + + local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then + warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;;

ekardnam

10:06 a.m.

New subject: [pacman-dev] [PATCH] Added flag --prevent-downgrade to repo-add to prevent adding packages to a database if a newer one is already present.

feel so dumb i did not save changes on disk, sorry :( On 08/05/19 07:55, Allan McRae wrote:

...

On 8/5/19 5:15 pm, ekarndam@autistici.org wrote:

...
From: ekardnam

Reference bug in the bug tracker is at #FS17752

Signed-off-by: Luca Bertozzi --- scripts/repo-add.sh.in | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 57413df5..f8748f08 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -42,6 +42,7 @@ REPO_DB_SUFFIX= LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y' +DONT_DOWNGRADE=0

Everything is still --dont-downgrade. Not --prevent-downgrade.

...
# Import libmakepkg source "$LIBRARY"/util/message.sh @@ -63,6 +64,7 @@ Multiple packages to add can be specified on the command line.\n")" printf -- "$(gettext "Options:\n")" printf -- "$(gettext " -n, --new only add packages that are not already in the database\n")" printf -- "$(gettext " -R, --remove remove old package file from disk after updating database\n")" + printf -- "$(gettext " --dont-downgrade do not add package to database if a newer version is already present\n")" elif [[ $cmd == "repo-remove" ]] ; then printf -- "$(gettext "Usage: repo-remove [options] <path-to-db> <packagename> ...\n")" printf -- "\n" @@ -248,9 +250,17 @@ db_write_entry() { return 0 fi else - if (( RMEXISTING )); then - pkgentry=$(find_pkgentry "$pkgname") - if [[ -n $pkgentry ]]; then + pkgentry=$(find_pkgentry "$pkgname") + if [[ -n $pkgentry ]]; then + + local version=$(sed -n '/^%VERSION%$/ {n;p;q}' "$pkgentry/desc") + if (( "$(vercmp $version $pkgver)" > "0" )); then + warning "$(gettext "A newer version for '%s' is already present in database")" "$pkgname" + if (( DONT_DOWNGRADE )); then + return 0 + fi + fi + if (( RMEXISTING )); then local oldfilename="$(sed -n '/^%FILENAME%$/ {n;p;q;}' "$pkgentry/desc")" local oldfile="$(dirname "$1")/$oldfilename" fi @@ -618,6 +628,9 @@ while (( $# )); do -v|--verify) VERIFY=1 ;; + --dont-downgrade) + DONT_DOWNGRADE=1 + ;; *) args+=("$1") ;;

1808

Age (days ago)

1818

Last active (days ago)

List overview

Download

9 comments

4 participants

participants (4)

Allan McRae
ekardnam
ekarndam＠autistici.org
Eli Schwartz

[pacman-dev] [PATCH] Added --dont-downgrade flag to repo-add

tags

participants (4)