Hi Dan, sorry for the late reply, some family matters cropped up over the past week.
Do you think I should reroll this with your suggestions? iirc, the original intent of this patch was to allow the user to have a way to have a trustdb.gpg for pacman to carry out signature verification and to change it if need be. On Thu, Jun 2, 2011 at 2:02 AM, Dan McGee <[email protected]> wrote: > On Sat, May 28, 2011 at 9:37 AM, Pang Yan Han <[email protected]> > wrote: > > When pacman is installed, an empty trustdb is created if it is > non-existent. > > The --import-trustdb option allows users to import their own trustdb into > > pacman's gpgdir to facilitate signature verification. > > The name "import" leads me to believe it doesn't overwrite, but I > think this in fact does that. Is there any way we can actually do a > merge of the existing with the trust db one is pulling from? > > > Signed-off-by: Pang Yan Han <[email protected]> > > --- > > doc/pacman-key.8.txt | 4 ++ > > scripts/pacman-key.sh.in | 81 > ++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 85 insertions(+), 0 deletions(-) > > > > diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt > > index 8a08480..234e060 100644 > > --- a/doc/pacman-key.8.txt > > +++ b/doc/pacman-key.8.txt > > @@ -59,6 +59,10 @@ Commands > > *-h, \--help*:: > > Output syntax and command line options. > > > > +*\--import-trustdb* <db>:: > > + Overrides the trustdb with db. Confirmation from the user is > required before > > + the trustdb is overwritten, unless the trustdb is empty or > non-existent. > > + > > *-l, \--list*:: > > Equivalent to --list-sigs from GnuPG. > > > > diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in > > index e795aad..7a1fa42 100644 > > --- a/scripts/pacman-key.sh.in > > +++ b/scripts/pacman-key.sh.in > > @@ -70,10 +70,26 @@ usage() { > > echo "$(gettext " -u, --updatedb Update the trustdb of > pacman")" > > echo "$(gettext " -V, --version Show program > version")" > > echo "$(gettext " --adv <params> Use pacman's keyring > with advanced gpg commands")" > > + echo "$(gettext " --import-trustdb <db> Override pacman's > trustdb")" > > printf "$(gettext " --reload Reload the default > keys")" > > echo > > } > > > > +## From makepkg > > +# usage: in_array( $needle, $haystack ) > > +# return : 0 - found > > +# 1 - not found > > +## > > +in_array() { > > + local needle=$1; shift > > + [[ -z $1 ]] && return 1 # Not found > > + local item > > + for item in "$@"; do > > + [[ $item = $needle ]] && return 0 # Found > > + done > > + return 1 # Not found > > +} > > + > > version() { > > printf "pacman-key (pacman) %s\n" "${myver}" > > printf "$(gettext "\ > > @@ -228,6 +244,56 @@ if [[ $1 != "--version" && $1 != "-V" && $1 != > "--help" && $1 != "-h" && $1 != " > > fi > > fi > > > > +import_trustdb() { > > + local choice= > > + local valid_choices=('n' 'no' 'y' 'yes') > > + > > + warning "$(gettext "This option will overwrite your existing > trustdb at $PACMAN_KEYRING_DIR/trustdb.gpg with a new one.")" > > + > > + while ! in_array "$choice" "${valid_choices[@]}"; do > > + echo -n "$(gettext "==> Do you wish to continue (y/n) ")" > > + read choice > > + choice=$(echo "$choice" | tr '[:upper:]' '[:lower:]') > > + done > > + > > + if [[ $choice = 'n' || $choice = 'no' ]]; then > > + msg "$(gettext "Your original trustdb at > ${PACMAN_KEYRING_DIR}/trustdb.gpg is preserved.")" > > + exit 0 > > + fi > Hmm. None of this works for translated messages. Please follow the > pattern established in makepkg in the CLEANCACHE code (line ~1810). > This way the same strings can be used for gettext, and "N" is the > obvious default, which I think it should be. You also didn't translate > 'n' or 'no'; we should just use the Y/YES from makepkg over again. > Don't even worry about the valid choice stuff; if you don't type y or > yes, you lose. > > I'd also ditch the unnecessary "is preserved" message unless other > people think it is required. > > > + > > + # Reset choice > > + choice= > > + echo > > + > > + if [[ ! -e "${PACMAN_KEYRING_DIR}/trustdb.gpg" ]]; then > > + msg "$(gettext "No trustdb found at > ${PACMAN_KEYRING_DIR}/trustdb.gpg.")" > > + msg "$(gettext "Importing $1...")" > > + cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg > > + msg "$(gettext "Successfully imported $1 to > ${PACMAN_KEYRING_DIR}/trustdb.gpg")" > > + elif [[ $(stat -c "%s" "${PACMAN_KEYRING_DIR}/trustdb.gpg") = "0" > ]]; then > > + msg "$(gettext "Empty trustdb at > ${PACMAN_KEYRING_DIR}/trustdb.gpg.")" > > + msg "$(gettext "Importing $1...")" > > + cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg > > + msg "$(gettext "Successfully imported $1 to > ${PACMAN_KEYRING_DIR}/trustdb.gpg")" > I don't think this is the proper way to move owner trust values > around- shouldn't we be using a combo and pipe of --export-ownertrust > and --import-ownertrust? > > > + else > > + warning "$(gettext "trustdb at > \"${PACMAN_KEYRING_DIR}/trustdb.gpg\" is not empty.")" > > + while ! in_array "$choice" "${valid_choices[@]}" ; do > > + echo -n "$(gettext "==> Do you wish to overwrite > your pacman trustdb? (y/n) ")" > > + read choice > > + choice=$(echo "$choice" | tr '[:upper:]' > '[:lower:]') > > + done > Wait- why do we have two questions for this? Seems totally silly to > me. If I tell you to import and I have nothing, you should just do it. > If we determine we don't overwrite owner trust values via > export/import (but amend), then we shouldn't do questions at all. > > > + > > + echo > > + if [[ $choice = 'y' || $choice = 'yes' ]]; then > > + cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg > > + msg "$(gettext "Successfully imported $1 to > ${PACMAN_KEYRING_DIR}/trustdb.gpg")" > > + else > > + msg "$(gettext "$1 is not imported")" > > + msg "$(gettext "Your original trustdb at > ${PACMAN_KEYRING_DIR}/trustdb.gpg is preserved.")" > > + fi > > + fi > > +} > > + > > # Parse global options > > CONFIG="@sysconfdir@/pacman.conf" > > PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" > > @@ -322,6 +388,21 @@ case "${command}" in > > ;; > > -h|--help) > > usage; exit 0 ;; > > + --import-trustdb) > > + if (( $# != 1 )); then > > + error "$(gettext "You need to specify exactly one > trustdb!")" > We don't use exclamation points on other messages in this section, so > we shouldn't start the trend here. > > > + exit 1 > > + elif [[ ! -e $1 ]]; then > > + error "$(gettext "$1 does not exist!")" > > + exit 1 > > + elif [[ -d $1 ]]; then > > + error "$(gettext "$1 is a directory and cannot be > imported!")" > > + exit 1 > Why not just one -f check and a single "%s is not a trust DB file" > message? Also, don't use $1 directly in gettext, you need to use > substitution vars as is done everywhere else. > > > + fi > > + > > + import_trustdb $1 > > + > > + ;; > > -V|--version) > > version; exit 0 ;; > > *) > > -- > > 1.7.5.rc0.101.g3d23c > >
