I just updated the pkgbuild following your recommendations. I just have a question: Why not to put `autoreconf -fi` and `configure --prefix=/usr` under prepare() function? Thanks for your time!, Joaquin 2018-01-07 16:12 GMT-05:00 Eli Schwartz via aur-general < aur-general@archlinux.org>:
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