[pacman-dev] [PATCH 3/3] repo-add: do not allow pkgnames to start with a minus
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/repo-add.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index eb8837c..2133fed 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile") - # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi -- 1.6.3
On Tue, May 12, 2009 at 4:02 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/repo-add.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index eb8837c..2133fed 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile")
- # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then
Isn't this a bit late to be catching this? Shouldn't we be doing this in makepkg instead?
error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi --
Dan McGee wrote:
On Tue, May 12, 2009 at 4:02 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- scripts/repo-add.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index eb8837c..2133fed 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile")
- # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then
Isn't this a bit late to be catching this? Shouldn't we be doing this in makepkg instead?
I agree makepkg should be doing this. But what is the reason for this patch? As Dan and I discussed earlier, pkgnames are allowed to have hyphens in them, so why not at the start? Allan
Commandline arguments starting with a hyphen are usally recognized as options by unix tools. Therefore, allowing hyphens at the beginning of a package name requires a different handling of pkgnames as suggested by rm's manpage. It would be possible to make the scripts 'hyphen-safe', but hyphen-prefixed packages will cause trouble for pacman users which do not know these tricks. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- po/pacman.pot | 3 +++ scripts/makepkg.sh.in | 4 ++++ scripts/repo-add.sh.in | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) Allan McRae wrote:
I agree makepkg should be doing this. But what is the reason for this patch? As Dan and I discussed earlier, pkgnames are allowed to have hyphens in them, so why not at the start?
You two are right. It should definitely be handled in makepkg, too. Actually, I use repo-add to generate databases directly from PKGBUILDs and it happened unintentionally that a pkgname started with a hyphen, this is why I patched only repo-add and not makepkg. I added a check to makepkg and made the commit message more verbose in the new patch. The check in repo-add is probably unnecessary for most of you, but I think it do not harm to check twice. I hope the new commit message makes my intention more clear. For repo-add, the directory creation can currently fail and the depends and desc files are created in the database's base directory: # create package directory mkdir "$pkgname-$pkgver" cd "$pkgname-$pkgver" An example: $ mkdir "-test-1.2-1" mkdir: invalid option -- 't' Try `mkdir --help' for more information. $ cd "-test-1.2-1" bash: cd: -t: invalid option cd: usage: cd [-L|-P] [dir] This can be fixed, but in my opinion it is not worth the effort, because such packages are not that easy to handle. $ pacman -S -bla error: problem setting dbpath 'la' (could not find or read directory) $ pacman -S -- -bla -bla package not found, searching for group... error: '-bla': not found in sync db By the way, there are discussions to disallow several characters in unix/linux filenames, mainly whitespaces such as the new line and the null characters, but also a hyphen at the beginning of a filename [1]. Cedric [1] http://www.dwheeler.com/essays/fixing-unix-linux-filenames.html diff --git a/po/pacman.pot b/po/pacman.pot index f4cc3e1..03641c6 100644 --- a/po/pacman.pot +++ b/po/pacman.pot @@ -1297,6 +1297,9 @@ msgstr "" msgid "%s is not allowed to be empty." msgstr "" +msgid "%s is not allowed to start with a hyphen." +msgstr "" + msgid "%s is not allowed to contain hyphens." msgstr "" diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cb63f9a..9d9441a 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1569,6 +1569,10 @@ if [ -z "$pkgrel" ]; then error "$(gettext "%s is not allowed to be empty.")" "pkgrel" exit 1 fi +if [ "${pkgname:0:1}" == "-" ]; then + error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" + exit 1 +fi if [ "$pkgver" != "${pkgver//-/}" ]; then error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" exit 1 diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 14bd00e..2cfe986 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile") - # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi -- 1.6.3.1
Commandline arguments starting with a hyphen are usally recognized as options by unix tools. Therefore, allowing hyphens at the beginning of a package name requires a different handling of pkgnames as suggested by rm's manpage. It would be possible to make the scripts 'hyphen-safe', but hyphen-prefixed packages will cause trouble for pacman users which do not know these tricks. Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- po/pacman.pot | 3 +++ scripts/makepkg.sh.in | 4 ++++ scripts/repo-add.sh.in | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) rebased to reflect latest git changes diff --git a/po/pacman.pot b/po/pacman.pot index f4cc3e1..03641c6 100644 --- a/po/pacman.pot +++ b/po/pacman.pot @@ -1297,6 +1297,9 @@ msgstr "" msgid "%s is not allowed to be empty." msgstr "" +msgid "%s is not allowed to start with a hyphen." +msgstr "" + msgid "%s is not allowed to contain hyphens." msgstr "" diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f46b7f8..37a60f2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1117,6 +1117,10 @@ check_sanity() { error "$(gettext "%s is not allowed to be empty.")" "pkgrel" return 1 fi + if [ "${pkgname:0:1}" == "-" ]; then + error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" + return 1 + fi if [ "$pkgver" != "${pkgver//-/}" ]; then error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" return 1 diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 7c12aaf..1a0bd6d 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile") - # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi -- 1.6.3.2
Cedric Staniewski wrote:
Commandline arguments starting with a hyphen are usally recognized as options by unix tools. Therefore, allowing hyphens at the beginning of a package name requires a different handling of pkgnames as suggested by rm's manpage. It would be possible to make the scripts 'hyphen-safe', but hyphen-prefixed packages will cause trouble for pacman users which do not know these tricks.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- po/pacman.pot | 3 +++ scripts/makepkg.sh.in | 4 ++++ scripts/repo-add.sh.in | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-)
rebased to reflect latest git changes
diff --git a/po/pacman.pot b/po/pacman.pot index f4cc3e1..03641c6 100644 --- a/po/pacman.pot +++ b/po/pacman.pot @@ -1297,6 +1297,9 @@ msgstr "" msgid "%s is not allowed to be empty." msgstr ""
+msgid "%s is not allowed to start with a hyphen." +msgstr "" + msgid "%s is not allowed to contain hyphens." msgstr ""
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f46b7f8..37a60f2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1117,6 +1117,10 @@ check_sanity() { error "$(gettext "%s is not allowed to be empty.")" "pkgrel" return 1 fi + if [ "${pkgname:0:1}" == "-" ]; then + error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" + return 1 + fi if [ "$pkgver" != "${pkgver//-/}" ]; then error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" return 1
Looks good.
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 7c12aaf..1a0bd6d 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile")
- # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi
Do we really need the check here too? I figure makepkg is enough. I'm leaning towards -1 here but Dan can have final say. Allan
On Thu, Jun 11, 2009 at 6:42 AM, Allan McRae<allan@archlinux.org> wrote:
Cedric Staniewski wrote:
Commandline arguments starting with a hyphen are usally recognized as options by unix tools. Therefore, allowing hyphens at the beginning of a package name requires a different handling of pkgnames as suggested by rm's manpage. It would be possible to make the scripts 'hyphen-safe', but hyphen-prefixed packages will cause trouble for pacman users which do not know these tricks.
Signed-off-by: Cedric Staniewski <cedric@gmx.ca> --- po/pacman.pot | 3 +++ scripts/makepkg.sh.in | 4 ++++ scripts/repo-add.sh.in | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-)
rebased to reflect latest git changes
diff --git a/po/pacman.pot b/po/pacman.pot index f4cc3e1..03641c6 100644 --- a/po/pacman.pot +++ b/po/pacman.pot @@ -1297,6 +1297,9 @@ msgstr "" msgid "%s is not allowed to be empty." msgstr "" +msgid "%s is not allowed to start with a hyphen." +msgstr "" + msgid "%s is not allowed to contain hyphens." msgstr "" diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f46b7f8..37a60f2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1117,6 +1117,10 @@ check_sanity() { error "$(gettext "%s is not allowed to be empty.")" "pkgrel" return 1 fi + if [ "${pkgname:0:1}" == "-" ]; then + error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" + return 1 + fi if [ "$pkgver" != "${pkgver//-/}" ]; then error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" return 1
Looks good.
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 7c12aaf..1a0bd6d 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -216,8 +216,8 @@ db_write_entry() md5sum="$(openssl dgst -md5 "$pkgfile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$pkgfile") - # ensure $pkgname and $pkgver variables were found - if [ -z "$pkgname" -o -z "$pkgver" ]; then + # ensure $pkgname and $pkgver variables were found and pkgname does not start with a minus + if [ -z "$pkgname" -o "${pkgname:0:1}" == "-" -o -z "$pkgver" ]; then error "$(gettext "Invalid package file '%s'.")" "$pkgfile" return 1 fi
Do we really need the check here too? I figure makepkg is enough. I'm leaning towards -1 here but Dan can have final say.
I think I'm with Allan here. I'll keep the makepkg check and drop this one in the patch I apply. -Dan
Dan McGee wrote:
I think I'm with Allan here. I'll keep the makepkg check and drop this one in the patch I apply.
-Dan
I do not have a strong opinion about adding this check, but I think it is as useful/useless as the ones for empty pkgname or pkgver. Actually a hyphen-prefixed pkgname would break repo-add in the same way as an empty pkgname would do, but if one assumes that the .PKGINFO is always correct, the checks for empty pkgname or pkgver are useless, too. Cedric
Any chance to get this applied before the string freeze? Cheers
On Thu, Jul 23, 2009 at 10:12 AM, Cedric Staniewski<cedric@gmx.ca> wrote:
Any chance to get this applied before the string freeze? Applied.
... what is the reason for this patch? As Dan and I discussed earlier, pkgnames are allowed to have hyphens in them, so why not at the start? My guess is that "pacman -S -package" might have "-p" match as a
Allan McRae wrote: parameter instead of the whole word "-package" and might then throw an error that -a isn't recognised as a valid Sync parameter. -- __________ Brendan Hide
Seems Cedric already gave a more thorough explanation. My apologies. Brendan Hide wrote:
... what is the reason for this patch? As Dan and I discussed earlier, pkgnames are allowed to have hyphens in them, so why not at the start? My guess is that "pacman -S -package" might have "-p" match as a
Allan McRae wrote: parameter instead of the whole word "-package" and might then throw an error that -a isn't recognised as a valid Sync parameter. --
Brendan Hide
participants (4)
-
Allan McRae
-
Brendan Hide
-
Cedric Staniewski
-
Dan McGee