[aur-general] Package Review

[Eli Schwartz]

On 01/07/2018 02:59 PM, Joaquin Garmendia Cabrera wrote:
> Hello!,
> 
> Today, I adopted a little package and update some things that weren't
> according to packaging standards. I would be very grateful if somebody
> could suggest improvements.
> PKGBUILD is in [1].
> 
> Best Regards,
> Joaquin
> 
> [1]
> https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=gtk-arc-flatabulous-theme-git

Your first change was to create a merge commit from your own git repo,
rather than remove all files from the package repo and replace them with
your own. This created a merge commit of two unrelated git repositories,
which is kind of messy and makes it difficult to see what actually happened.

Other than that you basically just moved some dependencies to
optdepends. This seems fair as gtk3 is needed for building but other
than that they're just a bunch of data files... but surely gtk3 should
be an optdepends too, then?
You also removed the useless makedepends on packages in the base-devel
group, so good job on that!

Regarding pre-existing issues which should probably be fixed:

1) You can simplify to `source=("git+${url}")` as the default download
name is already the last component of the url, which coincidentally is
*also* the value of ${_pkgname}. ;)

2) Instead of using ./autogen.sh in build, use the standard tool
`autoreconf -fi`, which autotools build systems are supposed to migrate
to. This one actually does use autoreconf, but keeps a legacy autogen.sh
which just runs autoreconf -fi and then runs configure automatically.
Note that `autoreconf -fi` should properly be run in prepare() and then
`./configure --prefix=/usr` should be separately run in build()

3) The pkgver currently uses:
echo "$(git rev-list --count HEAD).$(git rev-parse --short HEAD)"

But ideally it would be prepended with an "r", as such:
printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short
HEAD)"

See https://wiki.archlinux.org/index.php/VCS_package_guidelines#Git for
details and general guidance. The reason for this, is because if the
repository ever gains tags, then it will be easy to upgrade from
r709.1b1b7c9 to 1.0 or whatever, because the r is automatically less
than 1. However, 709 is far more than 1, so there is now no way to
upgrade to tags without either causing users to "downgrade" or adding
the PKGBUILD "epoch" keyword when updating to a newly incompatible
version schema.

There is unfortunately no nice way to fix this. So I would leave it
as-is. If upstream ever adds tagged releases, you will need to update
the pkgver() to use the tags as per the wiki instructions, and
additionally add an `epoch=1` after the pkgver and pkgrel.

-- 
Eli Schwartz

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20180107/f9aef1ba/attachment.asc>