You are not logged in.

#1 2013-06-07 20:28:38

jeadorf
Member
Registered: 2012-05-12
Posts: 4
Website

[SOLVED] Would like to have a review of my first PKGBUILD for luksctl

Since this is the first PKGBUILD I have written with the intention to submit it to the AUR, I would really appreciate if someone with more experience can have a look on it to see whether there are obvious ways to improve it. I followed the Arch Packaging Standards and the AUR wiki page to the best of my knowledge in order to create the following PKGBUILD:

# Maintainer: Julius Adorf <jeadorf@gmail.com>
pkgname=luksctl
pkgver=0.0.2
pkgrel=1
pkgdesc="Control devices with LVM on top of LUKS."
arch=('any')
url="https://bitbucket.org/jeadorf/luksctl"
license=('GPL')
depends=('python2' 'util-linux' 'cryptsetup' 'lvm2')
source=(https://bitbucket.org/jeadorf/luksctl/downloads/luksctl-0.0.2.tar.gz)
md5sums=('e8ddb91798484bded80d2925bd178a2c')

build() {
  :
}

check() {
  :
}

package() {
    mkdir -p $pkgdir/usr/bin
    cp $srcdir/luksctl $pkgdir/usr/bin
    mkdir -p $pkgdir/etc/luksctl
    cp -r $srcdir/examples $pkgdir/etc/luksctl
    mkdir -p $pkgdir/usr/share/man/man5/
    cp $srcdir/doc/luksctl.profile.5.gz $pkgdir/usr/share/man/man5/
    mkdir -p $pkgdir/usr/share/man/man8/
    cp $srcdir/doc/luksctl.8.gz $pkgdir/usr/share/man/man8/
}

Last edited by jeadorf (2013-06-10 20:51:29)

Offline

#2 2013-06-07 20:59:31

HalosGhost
Member
From: Twin Cities, MN
Registered: 2012-06-22
Posts: 1,486
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

First, if you're not going to use the build and check functions, remove them. Second, use `install -d` instead of `mkdir` when operating on $pkgdir. Third, quote all your variables (including "$pkgdir" and "$srcdir"). Fourth, take advantage of the $pkgname and $pkgver variables and use them in your source array and package function (less work for future updates). Finally, anytime you don't need to recursively copy a directory (that is, anytime you just want to put one file in the $pkgdir, use `install` instead of `cp`.

Also, this is nit-picky, but I personally prefer it when PKGBUILDs use sha256sums instead of md5sums, but that might just be me.

Other than that, it's a great start.

All the best,

-HG

Last edited by HalosGhost (2013-06-07 21:06:41)


"All errors are ᴘᴇʙᴋᴀᴄ errors—It's just a matter of narrowing down which keyboard and chair." -Trilby
\ldots

Offline

#3 2013-06-07 21:12:27

Scimmia
Bug Wrangler
Registered: 2012-09-01
Posts: 4,984

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

To take it a step further than HalosGhost

    mkdir -p $pkgdir/usr/bin
    cp $srcdir/luksctl $pkgdir/usr/bin

Can just be

    install -Dm755 "$srcdir/luksctl" "$pkgdir/usr/bin/luksctl"

No need for a separate install -d command to make the dir, using -D will make the required dirs when installing the file.

Last edited by Scimmia (2013-06-07 21:13:48)

Offline

#4 2013-06-07 21:13:18

HalosGhost
Member
From: Twin Cities, MN
Registered: 2012-06-22
Posts: 1,486
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

Scimmia wrote:

No need for a separate install -d command to make the dir.

That's what I was going for. Good call Scimmia.

All the best,

-HG


"All errors are ᴘᴇʙᴋᴀᴄ errors—It's just a matter of narrowing down which keyboard and chair." -Trilby
\ldots

Offline

#5 2013-06-10 20:51:03

jeadorf
Member
Registered: 2012-05-12
Posts: 4
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

Thank you both very much. The package is now online and I will mark the thread as "solved".

Offline

#6 2013-06-10 21:11:30

msthev
Member
Registered: 2012-04-05
Posts: 177

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

HalosGhost wrote:

take advantage of the $pkgname and $pkgver variables and use them in your source array and package function (less work for future updates).

$pkgver definitely, but please don't do this with $pkgname. Sources and directories are determined by program's name, not package's name, therefore they shouldn't contain $pkgname (exception: /usr/share/licenses/$pkgname). If you do this, then anyone wanting to create a custom version of the package (e.g. `pkgname=luksctl-whatever`) will have to do much more work with replacing $pkgname by $_origpkgname or something similiar.

Example:
Original dmenu package (using $pkgname): https://projects.archlinux.org/svntogit … ages/dmenu
Now any package from this list has to replace `$pkgname` with `dmenu`. And it doesn't help the original package at all, since $pkgname won't ever change there.

Last edited by msthev (2013-06-10 21:22:35)

Offline

#7 2013-06-10 21:23:18

HalosGhost
Member
From: Twin Cities, MN
Registered: 2012-06-22
Posts: 1,486
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev wrote:

$pkgver definitely, but please don't do this with $pkgname. Sources and directories are determined by program's name, not package's name, therefore they shouldn't contain $pkgname (exception: /usr/share/licenses/$pkgname). If you do this, then anyone wanting to create a custom version of the package (e.g. `pkgname=luksctl-whatever`) will have to do much more work with replacing $pkgname by $_origpkgname or something similiar

To all their own. I try to utilize the widely-used variables as much as possible in my PKGBUILDs; it might make my life easier in the future, and if someone wants to create a new version based on one of my packages, then they are welcome to do so, and it will only take them a few seconds to edit it as necessary (if they even have to, which they might not; it depends on how the source was archived upstream).

All the best,

-HG

P.S., you can clean up your source array even more by doing something like the following:

source=("$url/downloads/$pkgname-$pkgver.tar.gz")

And, you really should quote your source array if there are variables in it.

Last edited by HalosGhost (2013-06-10 21:26:05)


"All errors are ᴘᴇʙᴋᴀᴄ errors—It's just a matter of narrowing down which keyboard and chair." -Trilby
\ldots

Offline

#8 2013-06-10 21:27:44

Scimmia
Bug Wrangler
Registered: 2012-09-01
Posts: 4,984

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev wrote:
HalosGhost wrote:

take advantage of the $pkgname and $pkgver variables and use them in your source array and package function (less work for future updates).

$pkgver definitely, but please don't do this with $pkgname. Sources and directories are determined by program's name, not package's name, therefore they shouldn't contain $pkgname (exception: /usr/share/licenses/$pkgname). If you do this, then anyone wanting to create a custom version of the package (e.g. `pkgname=luksctl-whatever`) will have to do much more work with replacing $pkgname by $_origpkgname or something similiar.

much more work? sed -i 's/$pkgname/$_origpkgname/g' PKGBUILD

Offline

#9 2013-06-10 21:43:29

msthev
Member
Registered: 2012-04-05
Posts: 177

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

OK, that was obviously an exaggeration. I just can't understand the reason to do so — it won't help at all, and it makes some of the things harder.
Another thing it introduces: inconsistency, visible for example in links with leading letters repeated in them. livestreamer — notice the `/l/` in the URL? It's the first letter of the program's name. Now who is going to write ${pkgname::1} there?…


@OP
Besides quoting mentioned by HG, you should also install non-executable files with -m644.

Last edited by msthev (2013-06-10 21:47:00)

Offline

#10 2013-06-10 21:47:42

HalosGhost
Member
From: Twin Cities, MN
Registered: 2012-06-22
Posts: 1,486
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev wrote:

OK, that was obviously an exaggeration. I just can't understand the reason to do so — it won't help at all, and it makes some of the things harder.
Another thing it introduces: inconsistency, visible for example in links with leading letters repeated in them. livestreamer — notice the `/l/` in the URL? It's the first letter of the program's name. Now who is going to write ${pkgname::1} there?…

Wait, what is inconsistent about thatare you talking about?

All the best,

-HG

Last edited by HalosGhost (2013-06-10 21:49:18)


"All errors are ᴘᴇʙᴋᴀᴄ errors—It's just a matter of narrowing down which keyboard and chair." -Trilby
\ldots

Offline

#11 2013-06-10 21:54:27

msthev
Member
Registered: 2012-04-05
Posts: 177

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

pypi.python.org puts packages in directories named with package's first letter. If I had a program `bar`, it's URL would be `…/source/b/bar/…`.
If it was to be consistent, the link should either be

http://pypi.python.org/packages/source/l/livestreamer/livestreamer-$pkgver.tar.gz

or

http://pypi.python.org/packages/source/${pkgname::1}/$pkgname/$pkgname-$pkgver.tar.gz

Offline

#12 2013-06-10 21:54:34

Scimmia
Bug Wrangler
Registered: 2012-09-01
Posts: 4,984

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev, I don't do it to the level that HalosGhost does, but it's usually for code reuse. Especially when I'm working on multiple PKGBUILDs from one author/team, I can usually take large chunks from one and just drop it in the next. Even for completely different packages, though, it's often as simple as cut and paste, maybe with one or two small changes.

Last edited by Scimmia (2013-06-10 21:56:56)

Offline

#13 2013-06-10 21:58:43

HalosGhost
Member
From: Twin Cities, MN
Registered: 2012-06-22
Posts: 1,486
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev wrote:
http://pypi.python.org/packages/source/${pkgname::1}/$pkgname/$pkgname-$pkgver.tar.gz

Ahh, yes, that would be silly. But I've already mentioned why I do it, and the reusage of code between PKGBUILDs is a nice plus (most of my PKGBUILDs are extremely similar, and for good reason).

All the best,

-HG


"All errors are ᴘᴇʙᴋᴀᴄ errors—It's just a matter of narrowing down which keyboard and chair." -Trilby
\ldots

Offline

#14 2013-06-10 21:59:18

jeadorf
Member
Registered: 2012-05-12
Posts: 4
Website

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

msthev wrote:

Besides quoting mentioned by HG, you should also install non-executable files with -m644.

My fault, I should have caught that one myself.

Regarding the question about where to use the $pkgname variable: I have used the variable rather sparingly on purpose even though one might argue that the package name, the executable and the man pages should have a consistent name. The consistency is expressed by using the $pkgname variable in all these places. I have left it for now as it is, particularly because I cannot exactly figure out what future problems may arise. given my little packaging experience until now.

Offline

#15 2013-06-10 22:05:12

msthev
Member
Registered: 2012-04-05
Posts: 177

Re: [SOLVED] Would like to have a review of my first PKGBUILD for luksctl

@Scimmia, @HG
That's a valid point. But since you, Scimmia, told me about this sed line… you know wink

I am of course complaining about something not at all important. It's just my OCD that makes me a little bit angry.


@jeadorf
You'll create more of those and you'll naturally see the problems (e.g. using mkdir/cp/mv vs install — they all have their purpose).


I think I'm done with the $pkgname debate, I won't be complaining more.

Offline

Board footer

Powered by FluxBB