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 < [email protected]>: > 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 > >
