[pacman-dev] Unusable pacman-key help by default

[Ivan c00kiemon5ter Kanak]

On 29 March 2011 20:08, Dan McGee <dpmcgee at gmail.com> wrote:

> On Mon, Mar 28, 2011 at 6:05 PM, Ivan c00kiemon5ter Kanak
> <ivan.kanak at gmail.com> wrote:
> > On 28 March 2011 20:27, Dan McGee <dpmcgee at gmail.com> wrote:
> >
> >> This is not cool at all, patches welcome from anyone. :)
> >>
> >> dmcgee at clifden ~/projects/pacman (master)
> >> $ ./scripts/pacman-key --help
> >> mkdir: cannot create directory `/etc/pacman.d/gnupg': Permission denied
> >>
> >>
> > Hi all, I'm pretty new here, so I'm not sure how I should send this. I've
> > read the submitting-patches page but there are lots of things to
> experiment
> > with. So I thought I'd just send it as it is now and look more into how
> to
> > submit patches tomorrow and maybe ask for some help on irc. This is
> actually
> > the output of 'git show'.
> git format-patch and git send-email are preferred (required?)-  this
> should be in that document.
>
> They are ;) it was actually 2am for me, so I thought I just send this and
try those today.


>  > also two comments:
> > a. On line 227 we define a default location for the PACMAN_KEYRING_DIR .
> On
> > lines 228-234 we read the command line options and set the
> > PACMAN_KEYRING_DIR to the one specified by the appropriate switch. On
> lines
> > 244-246 we read the configuration file and look for GPGDir variable which
> > sets again the PACMAN_KEYRING_DIR.
> > I find this behaviour a bit strange. I would expect the command line
> options
> > to overide the configuration file settings. Is that a bug or a wanted
> > behavior ?
> Command line should always override config file, so this is a bug.
> default < config file < command line.
>
> right. this is easy to fix, just rearrange the check order.


> This check also seems totally bogus:
>    if [[ ! -r "${CONFIG}" ]]; then
> Why should we care? We have a default anyway if the config file isn't
> there.
>
> > b. I like that we use bash, and not restrict ourselves to sh. Is that
> valid
> > for devtools too ?
> Devtools isn't managed on this list, but as most of those scripts have
> #!/bin/bash, draw your own conclusions there.
> > Are we restricted to some bash version ? I've seen some
> > of the devtools scripts and I think bash would make some things easier
> and
> > cleaner.
> Here we try to stay 3.2+ compatible; devtools are Arch-specific so
> probably no such restriction.
>
> good to know, thanks :)


> > ---
> >
> > commit 4004b8e927705cab0e0d4fefcf7d04cf7630a2e7
> > Author: Ivan Kanakarakis <ivan.kanak at gmail.com>
> > Date:   Tue Mar 29 01:47:07 2011 +0300
> >
> >    This fixes the issue raised by Dan McGee, the wrong processing of -h,
> >         --help switches. It ignores any other switch given if any of
> those
> >         two are present. It also skips the root check. Why would one need
> >         to be root to see the help message ?
> >
> >    Signed-off-by: Ivan Kanakarakis <ivan.kanak at gmail.com>
> >
> > diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> > index 89e52fc..490dc39 100644
> > --- a/scripts/pacman-key.sh.in
> > +++ b/scripts/pacman-key.sh.in
> > @@ -211,6 +211,14 @@ if ! type gettext &>/dev/null; then
> >  }
> >  fi
> >
> > +opts=($@)
> > +for opt in ${opts[@]}; do
> > +    if [[ $opt == "--help" || $opt == "-h" ]]; then
> > +        usage
> > +        exit 0
> > +    fi
> > +done
>
> I hate this special casing. You also omitted -V/--version. And now
> repo-add and this script do it in two completely different ways.
>
> One case statement to parse all opts in both of these scripts should
> be used if possible; delaying any necessary actions until the command
> has been determined. If each of the add/update/etc. ops called a
> "setup_gpg" method first or something that did the config file parsing
> and ensuring the GPG directory exists, that would be best.
>
> Alright, I thought a special casing wouldnt be the best solution here. I'll
look into repo-add script too and try to do as you suggested.


> > +
> >  if [[ $1 != "--version" && $1 != "-V" && $1 != "--help" && $1 != "-h" &&
> $1
> > != "" ]]; then
> >  if type -p gpg >/dev/null 2>&1 = 1; then
> >  error "$(gettext "gnupg does not seem to be installed.")"
> > @@ -316,8 +324,6 @@ case "${command}" in
> >  ${GPG_PACMAN} "$@" || ret=$?
> >  exit $ret
> >  ;;
> > - -h|--help)
> > - usage; exit 0 ;;
> >  -V|--version)
> >  version; exit 0 ;;
> >  *)
> >
> >
>
>