On Tue, May 12, 2009 at 4:02 AM, Cedric Staniewski wrote:

Signed-off-by: Cedric Staniewski ---  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 --

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

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