PKGBUILD review request: quickcurver-git

Magnus2552 · 2017-03-23 18:35:01

Hey, can someone please review my PKGBUILD? https://github.com/magnus-gross/quickcu … r/PKGBUILD

# Maintainer: Magnus Groß magnus.gross21@gmail.com
_pkgname=quickcurver
pkgname="$_pkgname"-git
pkgver=VERSION
pkgrel=1
pkgdesc="Qt Material design implementation of Achtung die Kurve with online multiplayer"
arch=('i686' 'x86_64')
url="https://github.com/magnus-gross/$_pkgname"
license=('GPL3')
depends=('qt5-base>=5.7 qt5-declarative qt5-svg qt5-quickcontrols qt5-quickcontrols2 qt5-graphicaleffects')
makedepends=('git make gcc sed')
optdepends=()
source=("git+https://github.com/magnus-gross/$_pkgname.git")
md5sums=('SKIP') #autofill using updpkgsums

build() {
  cd "$srcdir/$_pkgname"
  git submodule init
  git submodule update
  ./build.sh
}

pkgver() {
  cd "$srcdir/$_pkgname"
  ( set -o pipefail
    git describe --long 2>/dev/null | sed 's/\([^-]*-g\)/r\1/;s/-/./g' ||
    printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
  )
}

check() {
  ls "$srcdir/$_pkgname/build/QuickCurver"
}

package() {
  install -D "$srcdir/$_pkgname/build/QuickCurver" "$pkgdir/usr/bin/$_pkgname"
}

Thanks in advance!

Slithery · 2017-03-23 18:44:36

You can remove the version dependency from qt5-base, the version in the repos is already higher than this.

You can remove make, gcc, and sed from the makedepends as they're in the base or base-devel groups which are already assumed to be installed.

Get rid of the check function, it doesn't do anything.

Scimmia · 2017-03-23 18:44:37

See the VCS package guidelines page for info on handling git submodules. You should not be downloading it in the build function.

Your quoting in the depends and makedepends arrays is wrong. The entire depends array is a single entry that could never be satisfied. This won't even build as written.

You don't need the version requirement on qt5-base

make, gcc, and sed are all part of the base-devel group and should not be included in the makedepends.

You should delete empty arrays.

Functions are guaranteed to start in $srcdir, there's no need to include that in the cd command.

That check function is totally useless

It's strange putting the functions in that order, but not really a problem.

Last edited by Scimmia (2017-03-23 18:45:39)

Magnus2552 · 2017-03-23 20:00:31

Thanks a lot for your help!
Can you please check out my corrected PKGBUILD one more time?

# Maintainer: Magnus Groß magnus.gross21@gmail.com
_pkgname=quickcurver
pkgname="$_pkgname"-git
pkgver=VERSION
pkgrel=1
pkgdesc="Qt Material design implementation of Achtung die Kurve with online multiplayer"
arch=('i686' 'x86_64')
url="https://github.com/magnus-gross/$_pkgname"
license=('GPL3')
depends=(qt5-base qt5-declarative qt5-svg qt5-quickcontrols qt5-quickcontrols2 qt5-graphicaleffects)
makedepends=(git)
source=("git+https://github.com/magnus-gross/$_pkgname.git"
		"git+https://github.com/magnus-gross/qml-material")
md5sums=('SKIP'
		'SKIP')

pkgver() {
  cd "$_pkgname"
  ( set -o pipefail
    git describe --long 2>/dev/null | sed 's/\([^-]*-g\)/r\1/;s/-/./g' ||
    printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
  )
}

prepare() {
  cd "$_pkgname"
  git submodule init
  git config submodule.qml-material.url $srcdir/qml-material
  git submodule update
}

build() {
  cd "$_pkgname"
  ./build.sh
}

package() {
  install -D "$srcdir/$_pkgname/build/QuickCurver" "$pkgdir/usr/bin/$_pkgname"
}

Also why did I have to remove the version requirement at qt5-base? What if someone tries to install my package, while having Qt 5.6 installed AND ignoring it in pacman.conf? If version checks can removed once the package hits the stable arch repos, what is even the purpose of version checks?

Slithery · 2017-03-23 20:09:39

Magnus2552 wrote:

What if someone tries to install my package, while having Qt 5.6 installed AND ignoring it in pacman.conf?

Then they're doing it wrong, partial upgrades are unsupported.

Trilby · 2017-03-23 20:17:30

Magnus2552 wrote:

If version checks can removed once the package hits the stable arch repos, what is even the purpose of version checks?

Less-than-or-equal-to version checks are likely more common. These are for when an upcoming change to a dependency will not work with the code packaged by the PKGBUILD. Users will then get a warning when updating about breaking a dependency of the AUR package and they'll know they have to deal with it.

There can also be strict-equal-to dependencies when a PKGBUILD's package is made in tandem with some other project and they will only work with very specific versions of their partner.

Greater-than-or-equal-to version checks can happen when one might need something newer than what is currently in the repos. Perhaps this dependency would also need to be built from the AUR. These, however, can be removed as soon as the repo version catches up to the requirement.

Versioned deps serve to inform users when something other than what is currently (or may eventually be) in the repos is needed. If what's in the repos works and there's no reason to suspect an update of the dependency will no longer be adequate, then leave off the versions.

Magnus2552 · 2017-03-23 20:42:24

Ok thanks for the explanation.
So, my PKGBUILD is good to go now?

Xyne · 2017-03-23 23:15:22

slithery wrote:

they're in the base or base-devel groups which are already assumed to be installed

Only the base-devel group is assumed to be installed.

Slithery · 2017-03-23 23:33:11

Xyne wrote:

Only the base-devel group is assumed to be installed.

Fair point, but anyone who doesn't have the entire base group installed should be sufficiently savvy to troubleshoot any issues that arise from not having it.

Last edited by Slithery (2017-03-23 23:38:52)

Xyne · 2017-03-24 00:34:56

slithery wrote:

Xyne wrote:
Only the base-devel group is assumed to be installed.
Fair point, but anyone who doesn't have the entire base group installed should be sufficiently savvy to troubleshoot any issues that arise from not having it.

There's an ongoing discussion about this on the aur-general mailing list. My contention is that only base-devel is assumed to be installed and other deps should be listed at least indirectly for technical correctness. Anything less is just laziness to avoid typing a few extra words and leads to a package ecosystem with incomplete metadata that prevents pacman/makepkg from doing what they are designed to do (dependency resolution). Expecting a full base installation in e.g. chroots to avoid issues with implicit deps is unacceptable. Even expecting all users to install the full base installation on a "real" system goes against the so-called "Arch way", but that is a minor philosophical issue.

There is a simple technical problem (dependency resolution) and a simple technical solution (provide necessary metadata). Competent users could figure out what packages they need even if pacman provided no dependency resolution, but it does so why not make it complete? I don't get the opposition to technical correctness and completeness, especially just to save a few keystrokes.

caleb · 2020-02-21 10:50:18

Xyne wrote:

slithery wrote:
they're in the base or base-devel groups which are already assumed to be installed
Only the base-devel group is assumed to be installed.

For the sake of anybody else coming across this post from links suggesting it's the canonical answer ... here is fresher news.

Since base is no longer a package group with (possibly) extraneous packages in it, and now a slimmed down metapackage that is effectively required to have a supported Arch system on any platform, it is automatically assumed in the depends=() section PKGBUILDs just like base-devel is assumed as part of makedepends=().

Last edited by caleb (2020-02-21 10:50:53)

Arch Linux

#1 2017-03-23 18:35:01

PKGBUILD review request: quickcurver-git

#2 2017-03-23 18:44:36

Re: PKGBUILD review request: quickcurver-git

#3 2017-03-23 18:44:37

Re: PKGBUILD review request: quickcurver-git

#4 2017-03-23 20:00:31

Re: PKGBUILD review request: quickcurver-git

#5 2017-03-23 20:09:39

Re: PKGBUILD review request: quickcurver-git

#6 2017-03-23 20:17:30

Re: PKGBUILD review request: quickcurver-git

#7 2017-03-23 20:42:24

Re: PKGBUILD review request: quickcurver-git

#8 2017-03-23 23:15:22

Re: PKGBUILD review request: quickcurver-git

#9 2017-03-23 23:33:11

Re: PKGBUILD review request: quickcurver-git

#10 2017-03-24 00:34:56

Re: PKGBUILD review request: quickcurver-git

#11 2020-02-21 10:50:18

Re: PKGBUILD review request: quickcurver-git

Board footer