[aur-general] Package Review

[Joaquin Garmendia Cabrera]

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 at 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
>
>