July 2011 - Pacman-dev - lists.archlinux.org

[pacman-dev] pacman-key status
by Allan McRae 09 Jul '11

09 Jul '11

Hi, With the patches towards pacman-key now on my working branch ([1] - 16 patches in total!), I have completed everything that I had on my TODO list for pacman-key. This should also cover all patches submitted to the mailing list except the --import{,-trustdb} ones by Pang Yan Han (see below). Please let me know if anything submitted here has been omitted. For the --import{,-trustdb} patches, I am not entirely sure of the status... It would appear that the original patch [2] was reviewd by Dan [3] and a revision submitted [4]. The revised patch looks fine and I am happy to get it applying on top of my current work but is has this comment in it: > NOTE: There is this very strange bug such that when new keys are added, > previously imported keys will fail for signature verification. This happens with > both --add and --import. > > Eg. Say you want to import trustdb.gpg and pubring.gpg from directories "first", > "second" and "third", you'd have to: > > # pacman-key --import first second third > # pacman-key --import first second third However, I am not seeing this behavior with --add, so I am unsure whether it is just this --import patch that is an issue. So that patch is somewhat in limbo at the moment. I will follow this up in reply to the patch. Allan [1] http://projects.archlinux.org/users/allan/pacman.git/log/?h=working [2] http://mailman.archlinux.org/pipermail/pacman-dev/2011-May/013159.html [3] http://mailman.archlinux.org/pipermail/pacman-dev/2011-June/013190.html [4] http://mailman.archlinux.org/pipermail/pacman-dev/2011-June/013321.html

1 0

[pacman-dev] [PATCH] pacman-key: check required permissions on keyring
by Allan McRae 09 Jul '11

09 Jul '11

Makes sure that the pacman keyring is readable and that the user has permissions to create a lock file if lock-never is not specified in the gpg.conf file. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- scripts/pacman-key.sh.in | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 4ea9842..b4e7abc 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -112,6 +112,25 @@ initialize() { fi } +check_keyring() { + if [[ ! -r ${PACMAN_KEYRING_DIR}/pubring.gpg || \ + ! -r ${PACMAN_KEYRING_DIR}/secring.gpg || \ + ! -r ${PACMAN_KEYRING_DIR}/trustdb.gpg ]]; then + error "$(gettext "You do not have sufficient permissions to read the %s keyring...")" "pacman" + msg "$(gettext "Use '%s' to correct the keyring permissions.")" "pacman-key --init" + exit 1 + fi + + if (( (EXPORT || FINGER || LIST || VERIFY) && EUID != 0 )); then + if ! grep -w -q "lock-never" ${PACMAN_KEYRING_DIR}/gpg.conf &>/dev/null; then + error "$(gettext "You do not have sufficient permissions to run this command...")" + msg "$(gettext "Use '%s' to correct the keyring permissions.")" "pacman-key --init" + exit 1 + fi + fi + +} + verify_keyring_input() { local ret=0; @@ -344,6 +363,7 @@ if (( numopt != 1 )); then exit 1 fi +(( ! INIT )) && check_keyring (( ADD )) && ${GPG_PACMAN} --quiet --batch --import "${KEYFILES[@]}" (( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "${KEYIDS[@]}" -- 1.7.6

1 0

[pacman-dev] [PATCH] pacman-key: add --init option
by Allan McRae 09 Jul '11

09 Jul '11

Add an --init option that ensures that the pacman keyring has all the necessary files and they have the correct permissions for being read as a user. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- doc/pacman-key.8.txt | 4 ++++ scripts/pacman-key.sh.in | 34 ++++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 2771ece..cf72b83 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -60,6 +60,10 @@ Options *-h, \--help*:: Output syntax and command line options. +*--init*:: + Ensure the keyring is properly initialized and has the required access + permissions. + *-l, \--list*:: Equivalent to --list-sigs from GnuPG. diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 3c0b5d9..ba817ca 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -32,6 +32,7 @@ DELETE=0 EDITKEY=0 EXPORT=0 FINGER=0 +INIT=0 LIST=0 RECEIVE=0 RELOAD=0 @@ -65,6 +66,7 @@ usage() { echo "$(gettext " --edit-key <keyid(s)> Present a menu for key management task on keyids")" echo "$(gettext " --gpgdir <dir> Set an alternate directory for gnupg")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.d/gnupg" + echo "$(gettest " --init Ensure the keyring is properly initialized")" echo "$(gettext " --reload Reload the default keys")" } @@ -91,6 +93,25 @@ get_from() { return 1 } +initialize() { + # Check for simple existence rather than for a directory as someone + # may want to use a symlink here + [[ -e ${PACMAN_KEYRING_DIR} ]] || mkdir -p -m 755 "${PACMAN_KEYRING_DIR}" + + # keyring files + [[ -f ${PACMAN_KEYRING_DIR}/pubring.gpg ]] || touch ${PACMAN_KEYRING_DIR}/pubring.gpg + [[ -f ${PACMAN_KEYRING_DIR}/secring.gpg ]] || touch ${PACMAN_KEYRING_DIR}/secring.gpg + [[ -f ${PACMAN_KEYRING_DIR}/trustdb.gpg ]] || ${GPG_PACMAN} --update-trustdb + chmod 644 ${PACMAN_KEYRING_DIR}/{{pub,sec}ring,trustdb}.gpg + + # gpg.conf + [[ ! -f ${PACMAN_KEYRING_DIR}/gpg.conf ]] || touch ${PACMAN_KEYRING_DIR}/gpg.conf + if ! grep -w -q "lock-never" ${PACMAN_KEYRING_DIR}/gpg.conf; then + echo "lock-never" >> ${PACMAN_KEYRING_DIR}/gpg.conf + fi + chmod 644 ${PACMAN_KEYRING_DIR}/gpg.conf +} + verify_keyring_input() { local ret=0; @@ -246,7 +267,7 @@ if ! type gettext &>/dev/null; then fi OPT_SHORT="a::d:e:f::hlr:t:uv:V" -OPT_LONG="add,adv:,config:,del:,export::,finger::,gpgdir:,help,list" +OPT_LONG="add,adv:,config:,del:,export::,finger::,gpgdir:,help,init,list" OPT_LONG+=",receive:,reload,trust:,updatedb,verify:,version" if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then echo; usage; exit 1 # E_INVALID_OPTION; @@ -268,6 +289,7 @@ while true; do -e|--export) EXPORT=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; + --init) INIT=1 ;; -l|--list) LIST=1 ;; -r|--receive) RECEIVE=1; shift; KEYSERVER="${1[0]}"; KEYIDS=("${1[@]:1}") ;; --reload) RELOAD=1 ;; @@ -289,7 +311,7 @@ if ! type -p gpg >/dev/null; then exit 1 fi -if (( (ADD || DELETE || EDITKEY || RECEIVE || RELOAD || UPDATEDB) && EUID != 0 )); then +if (( (ADD || DELETE || EDITKEY || INIT || RECEIVE || RELOAD || UPDATEDB) && EUID != 0 )); then error "$(gettext "%s needs to be run as root for this operation.")" "pacman-key" exit 1 fi @@ -304,15 +326,10 @@ fi # file, falling back on a hard default PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")} -# Try to create $PACMAN_KEYRING_DIR if non-existent -# Check for simple existence rather than for a directory as someone may want -# to use a symlink here -[[ -e ${PACMAN_KEYRING_DIR} ]] || mkdir -p -m 755 "${PACMAN_KEYRING_DIR}" - GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" # check only a single operation has been given -numopt=$(( ADD + DELETE + EDITKEY + EXPORT + FINGER + LIST + RECEIVE + RELOAD + UPDATEBD + VERIFY )) +numopt=$(( ADD + DELETE + EDITKEY + EXPORT + FINGER + INIT + LIST + RECEIVE + RELOAD + UPDATEBD + VERIFY )) if (( ! numopt )); then error "$(gettext "No operations specified")" @@ -333,6 +350,7 @@ fi (( EDITKEY )) && edit_keys (( EXPORT )) && ${GPG_PACMAN} --armor --export "${KEYIDS[@]}" (( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "${KEYIDS[@]}" +(( INIT )) && initialize (( LIST )) && ${GPG_PACMAN} --batch --list-sigs "${KEYIDS[@]}" (( RECEIVE )) && receive_keys (( RELOAD )) && reload_keyring -- 1.7.6

1 0

[pacman-dev] [PATCH] pacman-key: refactor get_from
by Dave Reisner 09 Jul '11

09 Jul '11

This function had a variety of pitfalls, including the inability to successfully find a key=value pair where no whitespace surrounded the equals sign. Make it more robust by splitting the line on the equals itself, and performing whitespace trimming on the resulting key/value pair. Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> --- scripts/pacman-key.sh.in | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b884870..9fae0d8 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -74,17 +74,19 @@ This is free software; see the source for copying conditions.\n\ There is NO WARRANTY, to the extent permitted by law.\n")" } -# Read provided file and search for values matching the given key -# The contents of the file are expected to be in this format: key = value -# 'key', 'equal sign' and 'value' can be surrounded by random whitespace -# Usage: get_from "$file" "$key" # returns the value for the first matching key in the file +# read the config file "$1" which has key=value pairs, and return the key which +# matches "$2". the equals sign between pairs may be surrounded by any amount +# of whitespace. get_from() { - while read key _ value; do - if [[ $key = $2 ]]; then - echo "$value" - break + while IFS='=' read -r key value; do + [[ -z $key || ${key:0:1} = '#' ]] && continue + + if [[ ${key%% *} = "$2" && -n ${value##* } ]]; then + echo "${value##* }" + return 0 fi done < "$1" + return 1 } reload_keyring() { -- 1.7.6

2 1

[pacman-dev] [PATCH 4/4] pacman-key: add --verify option
by Allan McRae 09 Jul '11

09 Jul '11

Signed-off-by: Allan McRae <allan(a)archlinux.org> --- doc/pacman-key.8.txt | 5 ++++- scripts/pacman-key.sh.in | 10 +++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index f961bc2..2771ece 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -72,7 +72,10 @@ Options *-u, \--updatedb*:: Equivalent to \--check-trustdb in GnuPG. -*-v, \--version*:: +* -v, \--verify* <signature>:: + Verify the given signature file. + +*-V, \--version*:: Displays the program version. diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 55c2abe..5324418 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -36,6 +36,7 @@ LIST=0 RECEIVE=0 RELOAD=0 UPDATEDB=0 +VERIFY=0 m4_include(library/output_format.sh) @@ -57,6 +58,7 @@ usage() { echo "$(gettext " -l, --list List keys")" echo "$(gettext " -r, --receive <keyserver> <keyid(s)> Fetch the specified keyids")" echo "$(gettext " -u, --updatedb Update the trustdb of pacman")" + echo "$(gettext " -v, --verify <signature> Verify the file specified by the signature")" echo "$(gettext " -V, --version Show program version")" echo "$(gettext " --config <file> Use an alternate config file")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.conf" @@ -241,9 +243,9 @@ if ! type gettext &>/dev/null; then } fi -OPT_SHORT="a::d:e:f::hlr:t:uV" +OPT_SHORT="a::d:e:f::hlr:t:uv:V" OPT_LONG="add,adv:,config:,del:,export::,finger::,gpgdir:,help,list" -OPT_LONG+=",receive:,reload,trust:,updatedb,version" +OPT_LONG+=",receive:,reload,trust:,updatedb,verify:,version" if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then echo; usage; exit 1 # E_INVALID_OPTION; fi @@ -268,6 +270,7 @@ while true; do -r|--receive) RECEIVE=1; shift; KEYSERVER="${1[0]}"; KEYIDS=("${1[@]:1}") ;; --reload) RELOAD=1 ;; -u|--updatedb) UPDATEDB=1 ;; + -v|--verify) VERIFY=1; shift; SIGNATURE=$1 ;; -h|--help) usage; exit 0 ;; -V|--version) version; exit 0 ;; @@ -309,7 +312,7 @@ PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" # check only a single operation has been given -numopt=$(( ADD + DELETE + EDITKEY + EXPORT + FINGER + LIST + RECEIVE + RELOAD + UPDATEBD )) +numopt=$(( ADD + DELETE + EDITKEY + EXPORT + FINGER + LIST + RECEIVE + RELOAD + UPDATEBD + VERIFY )) if (( ! numopt )); then error "$(gettext "No operations specified")" @@ -334,5 +337,6 @@ fi (( RECEIVE )) && receive_keys (( RELOAD )) && reload_keyring (( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb +(( VERIFY )) && ${GPG_PACMAN} --verify $SIGNATURE # vim: set ts=2 sw=2 noet: -- 1.7.6

1 0

[pacman-dev] [PATCH 1/4] pacman-key: move --edit-key and --receive processing to functions
by Allan McRae 09 Jul '11

09 Jul '11

This moves the processing of the --edit-key and --receive options to functions, keeping the final option processing to be all single line statements. Also rework the --edit-key option to validate all input before processing. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- scripts/pacman-key.sh.in | 47 +++++++++++++++++++++++++-------------------- 1 files changed, 26 insertions(+), 21 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b2b5669..c8f5111 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -202,6 +202,30 @@ reload_keyring() { ${GPG_PACMAN} --batch --check-trustdb } +receive_keys() { + if [[ -z ${KEYIDS[@]} ]]; then + error "$(gettext "You need to specify the keyserver and at least one key identifier")" + exit 1 + fi + ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}" +} + +edit_keys() { + local errors=0; + for key in ${KEYIDS[@]}; do + # Verify if the key exists in pacman's keyring + if ! ${GPG_PACMAN} --list-keys "$key" &>/dev/null; then + error "$(gettext "The key identified by %s does not exist")" "$key" + errors=1; + fi + done + (( errors )) && exit 1; + + for key in ${KEYIDS[@]}; do + ${GPG_PACMAN} --edit-key "$key" + done +} + # PROGRAM START if ! type gettext &>/dev/null; then gettext() { @@ -279,31 +303,12 @@ GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" (( ADD )) && ${GPG_PACMAN} --quiet --batch --import "${KEYFILES[@]}" (( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "${KEYIDS[@]}" +(( EDITKEY )) && edit_keys (( EXPORT )) && ${GPG_PACMAN} --armor --export "${KEYIDS[@]}" (( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "${KEYIDS[@]}" (( LIST )) && ${GPG_PACMAN} --batch --list-sigs "${KEYIDS[@]}" +(( RECEIVE )) && receive_keys (( RELOAD )) && reload_keyring (( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb -if (( RECEIVE )); then - if [[ -z ${KEYIDS[@]} ]]; then - error "$(gettext "You need to specify the keyserver and at least one key identifier")" - exit 1 - fi - ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}" -fi - -if (( EDITKEY )); then - for key in ${KEYIDS[@]}; do - # Verify if the key exists in pacman's keyring - if ${GPG_PACMAN} --list-keys "$key" &>/dev/null; then - ${GPG_PACMAN} --edit-key "$key" - else - error "$(gettext "The key identified by %s does not exist")" "$key" - exit 1 - fi - shift - done -fi - # vim: set ts=2 sw=2 noet: -- 1.7.6

1 2

[pacman-dev] [PATCH 0/7] pacman-key patch series
by Allan McRae 08 Jul '11

08 Jul '11

This is a patch series that I hope gets pacman-key into near ready territory. The big first patch is to make pacman-key use our option parser. That fixes a lot of issues that Denis and Ivan were trying to work around with --config and --gpgdir and whether they needed to be specified first. The remaining patches are all fairly small. They should capture almost all the work that has been submitted to this list over the past few months (the exception being the --import patches by Pang Yan Han that I am yet to look at in detail). When the submitted patches applied somewhat cleanly, the patch authorship is obviously maintained. When the major changes due to option parsing meant I needed to scrape out the still needed parts of patches and rewrite them, I have given criedit for the original work in the commit messages. A further patch is to follow updating the man page for the changes made here. Allan McRae (5): pacman-key: use our option parser pacman-key: remove the --adv option pacman-key: rename --del to --delete pacman-key: allow the export of all key ids pacman-key: rename --trust to --edit-key Ivan Kanakarakis (2): pacman-key: fix quotation on several variable assignments pacman-key: hide output of executed commands on logic checks scripts/pacman-key.sh.in | 210 ++++++++++++++++++++++----------------------- 1 files changed, 103 insertions(+), 107 deletions(-) -- 1.7.6

3 15

[pacman-dev] [PATCH] pacman-key: update man page
by Allan McRae 08 Jul '11

08 Jul '11

Update man page to reflect current options. Also add a description on how to manually interact with the pacman keyring with gpg. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- doc/pacman-key.8.txt | 57 +++++++++++++++++++++++-------------------------- 1 files changed, 27 insertions(+), 30 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 892f14d..c024923 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -12,65 +12,62 @@ pacman-key - manage pacman's list of trusted keys Synopsis -------- -'pacman-key' [options] <command> [arguments] +'pacman-key' [options] Description ----------- -'pacman-key' is a script used to manage pacman's keyring, which is the collection -of GnuPG keys used to check signed packages. It provides the ability to import -and export keys, fetch keys from keyservers and update the key trust database. +'pacman-key' is a wrapper script for GnuPG used to manage pacman's keyring, which +is the collection of PGP keys used to check signed packages and databases. It +provides the ability to import and export keys, fetch keys from keyservers and +update the key trust database. +More complex keyring management can be achieved using GnuPG directly combined with +the `--homedir` option pointing at the pacman keyring (located in ++{sysconfdir}/pacman.d/gnupg+ by default). -Options -------- -*\--config* <file>:: - Use an alternate config file instead of the +{sysconfdir}/pacman.conf+ - default. -*\--gpgdir* <dir>:: - Set an alternate home directory for GnuPG. If unspecified, the value is - read from +{sysconfdir}/pacman.conf+. - - -Commands +Options ------- -*-a, \--add* file ...:: +*-a, \--add* [file(s)]:: Add the key(s) contained in the specified file or files to pacman's keyring. If a key already exists, update it. -*\--adv* param ...:: - Use this option to issue particular GnuPG actions to pacman's keyring. This - option should be used with care as it can modify pacman's trust in - packages' signatures. +*\--config* <file>:: + Use an alternate config file instead of the +{sysconfdir}/pacman.conf+ + default. -*-d, \--del* keyid ...:: - Remove the key(s) identified by the specified keyid or keyids from pacman's +*-d, \--del* <keyid(s)>:: + Remove the key(s) identified by the specified keyid(s) from pacman's keyring. -*-e, \--export* [keyid ...]:: - Export key(s) identified by the specified keyid to 'stdout'. If no keyid is - specified, all keys will be exported. +*-e, \--export* [keyid(s)]:: + Export key(s) identified by the specified keyid(s) to 'stdout'. If no keyid + is specified, all keys will be exported. -*-f, \--finger* [keyid ...]:: +*\--edit-key * <keyid(s)>:: + Set the trust level of the given key. + +*-f, \--finger* [keyid(s)]:: List a fingerprint for each specified keyid, or for all known keys if no keyids are specified. +*\--gpgdir* <dir>:: + Set an alternate home directory for GnuPG. If unspecified, the value is + read from +{sysconfdir}/pacman.conf+. + *-h, \--help*:: Output syntax and command line options. *-l, \--list*:: Equivalent to --list-sigs from GnuPG. -*-r, \--receive* keyserver keyid ...:: +*-r, \--receive* <keyserver> <keyid(s)>:: Fetch the specified keyids from the specified key server URL. *\--reload*:: Reloads the keys from the keyring package. -*-t, \--trust* keyid:: - Set the trust level of the given key. - *-u, \--updatedb*:: Equivalent to \--check-trustdb in GnuPG. -- 1.7.6

2 2

[pacman-dev] [PATCH 1/3] makepkg: remove unused -C option from option list
by Dave Reisner 08 Jul '11

08 Jul '11

We nuke it from the completion file as well. Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> --- contrib/bash_completion.in | 2 +- scripts/makepkg.sh.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/bash_completion.in b/contrib/bash_completion.in index 7f47f4b..3093c02 100644 --- a/contrib/bash_completion.in +++ b/contrib/bash_completion.in @@ -35,7 +35,7 @@ _makepkg() { opts=('allsource asroot clean cleancache config force geninteg help holdver ignorearch install log nobuild nocolor noconfirm nodeps noextract noprogressbar pkg repackage rmdeps skipinteg source syncdeps' - 'A C L R c d e f g h i m o p r s') + 'A L R c d e f g h i m o p r s') _arch_ptr2comp opts fi true diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 1b132a9..cae6aa8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1747,7 +1747,7 @@ fi ARGLIST=("$@") # Parse Command Line Options. -OPT_SHORT="AcCdefFghiLmop:rRsV" +OPT_SHORT="AcdefFghiLmop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps" -- 1.7.6

5 7

[pacman-dev] [PATCH] Make alpm_db_set_pkgreason() arguments more sane
by Dan McGee 08 Jul '11

08 Jul '11

This can only ever operate on the local database, and a local package at that. Change the function signature to take a handle and package object, add the relevant asserts, and ensure the frontend can detect the package not found condition when finding packages to pass to this method. Signed-off-by: Dan McGee <dan(a)archlinux.org> --- lib/libalpm/alpm.h | 7 ++++--- lib/libalpm/db.c | 34 +++++++++++++++++++--------------- src/pacman/database.c | 3 ++- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index c7cab04..6e1e4bc 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -476,12 +476,13 @@ alpm_list_t *alpm_db_get_groupcache(alpm_db_t *db); alpm_list_t *alpm_db_search(alpm_db_t *db, const alpm_list_t* needles); /** Set install reason for a package in db. - * @param db pointer to the package database - * @param name the name of the package + * @param handle the context handle + * @param pkg the package to update * @param reason the new install reason * @return 0 on success, -1 on error (pm_errno is set accordingly) */ -int alpm_db_set_pkgreason(alpm_db_t *db, const char *name, alpm_pkgreason_t reason); +int alpm_db_set_pkgreason(alpm_handle_t *handle, alpm_pkg_t *pkg, + alpm_pkgreason_t reason); /** @} */ diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index af27047..0c2b26a 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -241,12 +241,17 @@ int SYMEXPORT alpm_db_get_valid(alpm_db_t *db) /** Get a package entry from a package database. */ alpm_pkg_t SYMEXPORT *alpm_db_get_pkg(alpm_db_t *db, const char *name) { + alpm_pkg_t *pkg; ASSERT(db != NULL, return NULL); db->handle->pm_errno = 0; ASSERT(name != NULL && strlen(name) != 0, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, NULL)); - return _alpm_db_get_pkgfromcache(db, name); + pkg = _alpm_db_get_pkgfromcache(db, name); + if(!pkg) { + RET_ERR(db->handle, ALPM_ERR_PKG_NOT_FOUND, NULL); + } + return pkg; } /** Get the package cache of a package database. */ @@ -287,19 +292,18 @@ alpm_list_t SYMEXPORT *alpm_db_search(alpm_db_t *db, const alpm_list_t* needles) } /** Set install reason for a package in db. */ -int SYMEXPORT alpm_db_set_pkgreason(alpm_db_t *db, const char *name, alpm_pkgreason_t reason) +int SYMEXPORT alpm_db_set_pkgreason(alpm_handle_t *handle, alpm_pkg_t *pkg, + alpm_pkgreason_t reason) { - ASSERT(db != NULL, return -1); - db->handle->pm_errno = 0; - /* TODO assert db == db_local ? shouldn't need a db param at all here... */ - ASSERT(name != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1)); - - alpm_pkg_t *pkg = _alpm_db_get_pkgfromcache(db, name); - if(pkg == NULL) { - RET_ERR(db->handle, ALPM_ERR_PKG_NOT_FOUND, -1); - } - - _alpm_log(db->handle, ALPM_LOG_DEBUG, "setting install reason %u for %s/%s\n", reason, db->treename, name); + CHECK_HANDLE(handle, return -1); + ASSERT(pkg != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); + ASSERT(pkg->origin == PKG_FROM_LOCALDB, + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); + ASSERT(pkg->origin_data.db == handle->db_local, + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); + + _alpm_log(handle, ALPM_LOG_DEBUG, + "setting install reason %u for %s\n", reason, pkg->name); if(alpm_pkg_get_reason(pkg) == reason) { /* we are done */ return 0; @@ -307,8 +311,8 @@ int SYMEXPORT alpm_db_set_pkgreason(alpm_db_t *db, const char *name, alpm_pkgrea /* set reason (in pkgcache) */ pkg->reason = reason; /* write DESC */ - if(_alpm_local_db_write(db, pkg, INFRQ_DESC)) { - RET_ERR(db->handle, ALPM_ERR_DB_WRITE, -1); + if(_alpm_local_db_write(handle->db_local, pkg, INFRQ_DESC)) { + RET_ERR(handle, ALPM_ERR_DB_WRITE, -1); } return 0; diff --git a/src/pacman/database.c b/src/pacman/database.c index 41efdd3..3e4a672 100644 --- a/src/pacman/database.c +++ b/src/pacman/database.c @@ -66,7 +66,8 @@ int pacman_database(alpm_list_t *targets) db_local = alpm_option_get_localdb(config->handle); for(i = targets; i; i = alpm_list_next(i)) { char *pkgname = i->data; - if(alpm_db_set_pkgreason(db_local, pkgname, reason) == -1) { + alpm_pkg_t *pkg = alpm_db_get_pkg(db_local, pkgname); + if(!pkg || alpm_db_set_pkgreason(config->handle, pkg, reason)) { pm_printf(ALPM_LOG_ERROR, _("could not set install reason for package %s (%s)\n"), pkgname, alpm_strerror(alpm_errno(config->handle))); retval = 1; -- 1.7.6

2 1