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-t...
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-t...
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.
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
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:
- 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}. ;)
- 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()
- 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
On 01/07/2018 07:11 PM, Joaquin Garmendia Cabrera wrote:
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
Strictly speaking, the prepare() function is meant to perform final changes to the source code in order to get it ready for building. This includes things like applying patches or sed commands, as well as autoreconf as autoreconf is used to create "dist" tarballs. It copies various m4 macros into the source tree, and generates the ./configure script as well as various other bits and pieces of an autotools system, and the final result is a source tree that is ready for distribution as "ready to build", since it has no need of external tools like aclocal, autoconf, autoheader, automake etc.
In comparison, ./configure is where the packaging starts. You use ./configure to specify things for *your* specific build, e.g. the --prefix, enabling and disabling optional features, toggling out-of-tree builds.
Semantically, autoreconf/autogen.sh are very much preparatory tools, whereas /.configure is very much a build tool. I like to be exact about things like this.
I understand your point and think it is correct. I was learning about autotools today and I have a better view of the situation. I was confused as well because in some places I readed the analogy: `./configure` -> prepare(), `make` -> build() and `make install` - > package().
Thanks for the help!. Joaquin
aur-general@lists.archlinux.org