Thank you very much for the feedback, seems like I still have some work to do. Unfortunately I didn't have time today, I'll try to take a better look and implement the changes tomorrow and will report back with the new and improved version. 2016-09-05 20:06 GMT+02:00 Eli Schwartz via aur-general :

On 09/05/2016 12:17 PM, Patrick Ziegler via aur-general wrote:

...

Hello everyone,

This is my first mail in here and I am ready to submit my first PKGBUILD to the AUR. But before I do that, I would like you to have a look at it, to make sure that nothing is wrong with it. I have already been through all the related wiki pages, namcap only complains about unnecessary dependencies (since this is a python project, we can ignore that) and the package seems to build and install fine on my system.

Nothing wrong with the package, but I can think of several things re: standards, coding, and duplication that you might want to fix.

...

# Maintainer: Patrick Ziegler # Contributor: Patrick Ziegler

Contributors are people who helped write the PKGBUILD but are not maintainers. Don't list yourself twice. :)

...

pkgname="notification-mount" pkgver=1.0.0 pkgrel=1 pkgdesc="Script to show notification for a block device with mount option" arch=('any') url="https://github.com/patrick96/notification-mount" license=('GPL3') depends=('python3' 'python-gobject' 'python-notify2' 'udevil')

The package is "python", not "python3". Also, it is implied by the other python modules it depends on, although it is okay to explicitly depend on it.

...

makedepends=('git') provides=('notification-mount') conflicts=('notification-mount')

Why does this package provide and conflict *itself*??? Save that for the *-git version.

And for the *-git version, I prefer to copy-paste across all my PKGBUILDs:

provides=("${pkgname%-git}") conflicts=("${pkgname%-git}")

...

source=("${pkgname}::git+${url}.git#tag=v${pkgver}")

Do not do this... download the tarball.

source=("${pkgname}-${pkgver}.tar.gz::${url}/archive/v${pkgver}.tar.gz")

...

md5sums=("SKIP")

There is no good reason to use ancient hash methods when sha256sums or similar exists. :) This provides more protection against malicious downloads, rather than just accidental corruption like the weak md5sums does.

...

package() { cd "$srcdir/$pkgname" || exit install -D -m755 "./notification-mount.py" "$pkgdir/usr/bin/notification-mount.py" ln -s "notification-mount.py" "$pkgdir/usr/bin/notification-mount"

Why are you installing the script twice, with and without a .py extension? Just imagine if every script in /usr/bin/ ended with .sh ;)

...

for _f in "./examples/*" do install -D -m644 $_f "$pkgdir/usr/share/$pkgname/examples/$(basename $_f)" done install -D -m644 ./examples/notification-mount.service \ "$pkgdir/usr/lib/systemd/user/notification-mount.service" }

There is only one example, and it is a systemd unit. Install the systemd unit, but don't copy it into /usr/share as well...

-- Eli Schwartz

Thank you again very much for your insights. I have made the necessary changes and will try to submit it to the AUR later in the evening, I will also post the new PKGBUILDs here again just so you can confirm that there is nothing I still have missed. notification-mount: # Maintainer: Patrick Ziegler pkgname="notification-mount" pkgver=1.0.1 pkgrel=1 pkgdesc="Script to show notification for a block device with mount option" arch=('any') url="https://github.com/patrick96/$pkgname" license=('GPL3') depends=('python' 'python-gobject' 'python-notify2' 'udevil') conflicts=("${pkgname}-git") source=("${pkgname}-${pkgver}.tar.gz::${url}/archive/v${pkgver}.tar.gz") sha256sums=("3cd4421a03e997ae6a99e79851ccfcf8b76a7885821bb982cc59407ea828b138") package() { cd "$srcdir/${pkgname}-${pkgver}" install -D -m755 "./notification-mount" "$pkgdir/usr/bin/notification-mount" install -D -m644 ./examples/notification-mount.service \ "$pkgdir/usr/lib/systemd/user/notification-mount.service" } # vim:set ts=2 sw=2 et: notification-mount-git: # Maintainer: Patrick Ziegler _pkgname="notification-mount" pkgname="${_pkgname}-git" pkgver=1.0.1 pkgrel=1 pkgdesc="Script to show notification for a block device with mount option" arch=('any') url="https://github.com/patrick96/$_pkgname" license=('GPL3') depends=('python' 'python-gobject' 'python-notify2' 'udevil') makedepends=('git') provides=("$_pkgname") conflicts=("$_pkgname") source=("${_pkgname}::git+${url}.git") md5sums=("SKIP") pkgver() { cd "$_pkgname" git describe --long --tags | sed 's/^v//;s/\([^-]*-g\)/r\1/;s/-/./g' } package() { cd "$srcdir/$_pkgname" install -D -m755 "./notification-mount" "$pkgdir/usr/bin/notification-mount" install -D -m644 ./examples/notification-mount.service \ "$pkgdir/usr/lib/systemd/user/notification-mount.service" } # vim:set ts=2 sw=2 et: 2016-09-06 19:57 GMT+02:00 Rafael Fontenelle :

Just as complement of what Eli said:

...

makedepends=('git')

If not fetching the source code via git (like to Eli suggested for notification-mount package), please remember to remove this setting from your PKGBUILD.

...

cd "$srcdir/$pkgname" || exit

There is no need for verification like "|| exit" or "|| false" as error status are already identified and treat. Try adding a command to change to a nonexistent directory and see what makepkg returns. ;)

Best Regards, Rafael Fontenelle

On purely visual grounds, you may wish to stick a line of whitespace in between the Maintainer line and the beginning of the PKGBUILD variables, to make it stand out better. The wiki says to minimize the amount the number of whitespaces, so I

2016-09-07 20:00 GMT+02:00 Eli Schwartz via aur-general : think I will leave it like this.

Non *-git packages have no need to conflict the *-git version, the *-git version, being the exception, should declare the conflict. And only one package needs to declare the conflict for pacman to figure it out. When I was testing it, it gave me an error that the files already exist, but after another test it seems what you are saying is true, so I'll remove it.

And if you actually want the *-git version to not include .r0.g<commit> then you may want to remove --long from the `git describe` invocation in pkgver() -- but personally I prefer it to be there. As it is now, you seem to have manually removed it but it will automatically be added back as soon as the PKGBUILD is used. I don't see a point in using the .r0.g suffix in the repo since I have a pkgver function that generates that automatically when the user builds the package. And whenever I update my code, I can just bump the pkgrel, which is much more convenient than figuring out the right suffix

2016-09-07 23:39 GMT+02:00 Doug Newgard :

Just run makepkg -o, nothing to figure out. Yes, you are right. I might do something with pre-commit hooks, but for now I will just use the pkgrel variable.

2016-09-08 2:42 GMT+02:00 Eli Schwartz via aur-general :

If you do do something with pre-commit hooks, make sure to check out my setup at https://github.com/eli-schwartz/pkgbuilds

I was actually looking at this yesterday, didn't realize it was from you though. I was only looking at the pre-commit hook but the other stuff looks really cool too, I will probably start using it. I have one question though, in what way is the pkgbuild-introspection package superior to just calling `makepkg --printsrcinfo > .SRCINFO`?

On 09/08/2016 07:49 AM, Patrick Ziegler via aur-general wrote:

I was actually looking at this yesterday, didn't realize it was from you though. I was only looking at the pre-commit hook but the other stuff looks really cool too, I will probably start using it. I have one question though, in what way is the pkgbuild-introspection package superior to just calling `makepkg --printsrcinfo > .SRCINFO`?

The hook existed before makepkg gained a --printsrcinfo flag, otherwise there is no real difference. I never changed it after pacman 5.0 was released. Also, Bruno Pagani is wrong to say mksrcinfo is slower. The current *-git version of pkgbuild-introspection is actually a wrapper around makepkg, and incurs the cost of initializing makepkg (source libmakepkg and run a few things that are only relevant for building packages, etc.) but those changes have not gone into a tagged release. Meanwhile, mksrcinfo v8 is a lot slimmer. When pkgbuild-introspection 9 is released, I may update that script to use makepkg instead, in the meantime I see no reason to *actively* switch from what worked, just to run things a little *slower* without requiring an extra package (which I already have installed, not to mention I would have had to alias it for interactive use anyway ;)). You are of course free to adapt the hook any way you choose. -- Eli Schwartz

On 09/08/2016 01:01 PM, Patrick Ziegler via aur-general wrote:

v8 also adds a header w/ a timestamp so you cannot run mksrcinfo on every commit, you have to actively check that PKGBUILD was changed, but the *-git does not do that anymore so it really doesn't matter for me.

True. And there my dirty secret reveals itself -- I use/d a patched copy of pkgbuild-introspection-git which doesn't write the header and uses its own logic instead of farming it out to makepkg. :o -- Eli Schwartz