Arch Linux

marmoser

Member

From: Vienna, Austria

Registered: 2015-12-02

Posts: 13

Website

Please review my PKGBUILDs

After more than 10 years of using Arch I've created my first PKGBUILDs.

Here are the PKGBUILDs for the OpenACS toolkit ( http://openacs.org/ ) and Naviserver ( https://bitbucket.org/naviserver/naviserver/ ), which is the web server OpenACS runs on + the connector module for postgresql.

https://github.com/marmoser/pkgbuilds

Please let me know your thoughts. Did I miss something?

Thanks

respiranto

Member

Registered: 2015-05-15

Posts: 479

Website

Re: Please review my PKGBUILDs

openacs:

pkgdesc="OpenACS is a toolkit for building scalable, community-oriented web applications."

When creating a package description for a package, do not include the package name in a self-referencing way. For example, "Nedit is a text editor for X11" could be simplified to "A text editor for X11". Also try to keep the descriptions to ~80 characters or less.

---

cp -dr --no-preserve=ownership "${srcdir}/$pkgname-$pkgver/." "${pkgdir}/usr/lib/$pkgname/"

Could be simplified to:

cp -dr "${srcdir}/$pkgname-$pkgver" "${pkgdir}/usr/lib/$pkgname/"

Apart from that it looks fine, just like the other PKGBUILDs and files, though it is not common to set the pkgname as an array, unless it is a split package.

[0] https://wiki.archlinux.org/index.php/Ar … _etiquette

Scimmia

Fellow

Registered: 2012-09-01

Posts: 11,583

Re: Please review my PKGBUILDs

provides=('openacs') is redundant, as are all the version numbers in the depends array. You only need those if you need something other than the latest stable.

arch=('i686' 'x86_64') seems wrong to me, since you only have one source tarball and are just copying files to /usr/lib.

"openacs.org" is not a url.

You should be consistent with your use of braces on variables.

You should use the $pkgver variable in the source array so you don't have to change it in multiple places on updates.

The package is already out of date

marmoser

Member

From: Vienna, Austria

Registered: 2015-12-02

Posts: 13

Website

Re: Please review my PKGBUILDs

openacs:

pkgdesc="OpenACS is a toolkit for building scalable, community-oriented web applications."

When creating a package description for a package, do not include the package name in a self-referencing way. For example, "Nedit is a text editor for X11" could be simplified to "A text editor for X11". Also try to keep the descriptions to ~80 characters or less.

I adapted pkgdesc for openacs. Seems I overlooked it this time.

---

cp -dr --no-preserve=ownership "${srcdir}/$pkgname-$pkgver/." "${pkgdir}/usr/lib/$pkgname/"

Could be simplified to:

cp -dr "${srcdir}/$pkgname-$pkgver" "${pkgdir}/usr/lib/$pkgname/"

Wouldn't this result in  /usr/lib/$pkgname/$pkgname-$pkgver, which is not the correct path for this package?
${srcdir}/$pkgname-$pkgver/xyz should go to "${pkgdir}/usr/lib/$pkgname/xyz
Apart from that, I removed the -no-preserve=ownership

Apart from that it looks fine, just like the other PKGBUILDs and files, though it is not common to set the pkgname as an array, unless it is a split package.

[0] https://wiki.archlinux.org/index.php/Ar … _etiquette

Changed it.

Thanks for your help!

marmoser

Member

From: Vienna, Austria

Registered: 2015-12-02

Posts: 13

Website

Re: Please review my PKGBUILDs

provides=('openacs') is redundant, as are all the version numbers in the depends array. You only need those if you need something other than the latest stable.

I removed the provides. I'm not really sure if I should remove the version numbers, since users can downgrade packages which can render the package unusable like using an old version of postgres.

arch=('i686' 'x86_64') seems wrong to me, since you only have one source tarball and are just copying files to /usr/lib.

"openacs.org" is not a url.

You should be consistent with your use of braces on variables.

You should use the $pkgver variable in the source array so you don't have to change it in multiple places on updates.

Fixed it.

The package is already out of date

The package now installs the lastest version, 5.9.0 .

Thanks a lot for your help.

respiranto

Member

Registered: 2015-05-15

Posts: 479

Website

Re: Please review my PKGBUILDs

cp -dr --no-preserve=ownership "${srcdir}/$pkgname-$pkgver/." "${pkgdir}/usr/lib/$pkgname/"

Could be simplified to:

cp -dr "${srcdir}/$pkgname-$pkgver" "${pkgdir}/usr/lib/$pkgname/"

Wouldn't this result in  /usr/lib/$pkgname/$pkgname-$pkgver, which is not the correct path for this package?
${srcdir}/$pkgname-$pkgver/xyz should go to "${pkgdir}/usr/lib/$pkgname/xyz
Apart from that, I removed the -no-preserve=ownership

Yes, I simply did not know that <path>/. has a different meaning than <path> to cp and did not look closely enough to check for the intended behaviour.

It doesn't really make sense to me though, since <path>/. and <path> both have the same inodes, respectively are hard links of each other.

progandy

Member

Registered: 2012-05-17

Posts: 5,201

Re: Please review my PKGBUILDs

It doesn't really make sense to me though, since <path>/. and <path> both have the same inodes, respectively are hard links of each other.

If the given target already exists as a directory, read the command as if it appends the last element of the given source path (the literal given value, not the canonical form) . In case of $foo/. or $foo/.. the last element is "." or "..", so it is unnamed and therefore not appended.

Last edited by progandy (2015-12-04 13:16:15)

---

| alias CUTF='LANG=en_XX.UTF-8@POSIX ' |

respiranto

Member

Registered: 2015-05-15

Posts: 479

Website

Re: Please review my PKGBUILDs

It doesn't really make sense to me though, since <path>/. and <path> both have the same inodes, respectively are hard links of each other.

If the given target already exists as a directory, read the command as if it appends the last element of the given source path (the literal given value, not the canonical form) . In case of $foo/. or $foo/.. the last element is "." or "..", so it is unnamed and therefore not appended.

Thanks, I though the semantics would be rather to copy the actual file specified by $src into the directory specified as $dest, provided that $dest is a directory.