FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 03-28-2011, 05:27 PM
Dan McGee
 
Default Unusable pacman-key help by default

This is not cool at all, patches welcome from anyone.

dmcgee@clifden ~/projects/pacman (master)
$ ./scripts/pacman-key --help
mkdir: cannot create directory `/etc/pacman.d/gnupg': Permission denied
 
Old 03-28-2011, 11:05 PM
Ivan c00kiemon5ter Kanak
 
Default Unusable pacman-key help by default

On 28 March 2011 20:27, Dan McGee <dpmcgee@gmail.com> wrote:

> This is not cool at all, patches welcome from anyone.
>
> dmcgee@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'.

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 ?
b. I like that we use bash, and not restrict ourselves to sh. Is that valid
for devtools too ? 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.

---

commit 4004b8e927705cab0e0d4fefcf7d04cf7630a2e7
Author: Ivan Kanakarakis <ivan.kanak@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@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
+
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 ;;
*)
 
Old 03-29-2011, 05:08 PM
Dan McGee
 
Default Unusable pacman-key help by default

On Mon, Mar 28, 2011 at 6:05 PM, Ivan c00kiemon5ter Kanak
<ivan.kanak@gmail.com> wrote:
> On 28 March 2011 20:27, Dan McGee <dpmcgee@gmail.com> wrote:
>
>> This is not cool at all, patches welcome from anyone.
>>
>> dmcgee@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.

> 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.

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.

> ---
>
> commit 4004b8e927705cab0e0d4fefcf7d04cf7630a2e7
> Author: Ivan Kanakarakis <ivan.kanak@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@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.

> +
> *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 ;;
> **)
>
>
 
Old 03-29-2011, 05:22 PM
Ivan c00kiemon5ter Kanak
 
Default Unusable pacman-key help by default

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

> On Mon, Mar 28, 2011 at 6:05 PM, Ivan c00kiemon5ter Kanak
> <ivan.kanak@gmail.com> wrote:
> > On 28 March 2011 20:27, Dan McGee <dpmcgee@gmail.com> wrote:
> >
> >> This is not cool at all, patches welcome from anyone.
> >>
> >> dmcgee@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@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@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 ;;
> > *)
> >
> >
>
>
 

Thread Tools




All times are GMT. The time now is 02:38 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org