[aur-general] PKGBUILD review

Thu Oct 19 14:34:06 UTC 2017

On Wed, Oct 18, 2017 at 10:18 PM, Eli Schwartz <eschwartz at archlinux.org> wrote:
> On 10/18/2017 06:13 PM, Garrett Battaglia via aur-general wrote:
>> This is the first time I am packaging for the AUR I would like someone
>> to look over my PKGBUILD below.
>>
>> Thanks,
>> Garrett
>>
>> #Maintainer: Garrett Battaglia <garrett at garrettbattaglia.info>
>> pkgname=sysdig-falco
>> pkgver=0.8.1
>> _sysdigver=0.19.1
>> pkgrel=1
>> pkgdesc="Behavioral Activity Monitoring With Container Support"
>> arch=('x86_64')
>
> Why x86_64 only? It is available for both, even the prebuilt binaries.
>
>> url=https://github.com/draios/falco
>> license=('GPL')
>> depends=('dkms')
>> makedepends=('git' 'cmake' 'linux-headers')
>> provides=('sysdig-falco')
>>
>> source=(
>> "http://download.draios.com/stable/rpm/${arch}/falco-${pkgver}-${arch}.rpm"
>> "https://github.com/draios/falco/archive/${pkgver}.tar.gz"
>> "https://github.com/draios/sysdig/archive/${_sysdigver}.tar.gz"
>> "sysdig-falco.install"
>> "sysdig-falco.service")
>>
>> install=$pkgname.install
>
> You haven't shown us the install file or the service file. install files
> are not supposed to be listed in source=()
>
> More than that, files named eg. 0.8.1.tar.gz should be renamed, by
> listing the source as e.g.
> "${pkgname}-${pkgver}.tar.gz::http://github.com/draios/falco/archive/${pkgver}.tar.gz"
>
> Why on earth are you downloading an rpm for a package that has source
> code available in git?
>
> If for some reason you desperately have to, DO NOT use ${arch} in the
> filename, it doesn't do what you think it does and only worked through
> sheer accident. That gets you the first array element of arch=() which
> only works as intended when there is ONLY one element and is not
> documented for use, it also breaks sane handling of multi-arch sources. Use:
> source_i686=("http://download.draios.com/stable/rpm/i386/falco-${pkgver}-i686.rpm")
> source_x86_64=("http://download.draios.com/stable/rpm/x86_64/falco-${pkgver}-x86_64.rpm")
>
>> md5sums=(
>> '5e017c747184101a0cc93ffc5b19ca47'
>> 'f3c654ded00f3186f3ff92320204a747'
>> '6ad8b4a7d1b0aa10cd62397318117a67'
>> '8bdb4c61dadd116f4901fa15c20da728'
>> '58fe0d406874c6f565f648d3b10da62a')
>>
>> prepare() {
>> cd ${srcdir}
>> mv sysdig-${_sysdigver} sysdig
>> mv falco-${pkgver} falco
>> rm -rf ./etc/rc.d
>> cd ./falco
>> mkdir build
>>
>> }
>
> All this moving around is ugly, if upstream depends on out-of-tree
> sources from another repo you should probably either clone a git tag or
> perhaps more ideally `ln -sf sysdig-${_sysdigver} sysdig`.
>
> Currently building a new version without clearing the old version will
> break, while that maybe shouldn't be done at all, it is always nice to
> do things in a way that plays nicely anyway.
>
>> build() {
>> cd $srcdir/falco/build
>> cmake ..
>> make driver
>> }
>>
>> package() {
>>
>> cd $srcdir
>> mkdir $pkgdir/usr
>> cp -r ./usr/* $pkgdir/usr/
>> mkdir $pkgdir/etc
>> cp -r ./etc/* $pkgdir/etc/
>> mkdir -p "$pkgdir/var/lib/dkms/falco/${pkgver}/${kernelVer}/${arch}/module/"
>> cp "$srcdir/falco/build/driver/falco-probe.ko"
>> "$pkgdir/var/lib/dkms/falco/${pkgver}/${kernelVer}/${arch}/module/falco-probe.ko"
>> mkdir -p $pkgdir/usr/lib/systemd/system/
>> cp sysdig-falco.service $pkgdir/usr/lib/systemd/system/
>> }
>
> If I read this correctly, the only thing you ever did here was build a
> copy of sysdig-probe.ko (provided by the sysdig package via dkms) named
> falco-probe.ko, and then use the prebuilt binary artifacts from some rpm.
>
> This should be built properly, against system libraries. Either that or
> name this package "sysdig-falco-bin" to indicate it is a package
> containing prebuilt binaries, and to clear the namespace for someone
> else to upload a proper built-from-source package.
>
> Also you repeatedly fail to quote $srcdir and $pkgdir, which means your
> PKGBUILD will fail to compile if built from a directory with spaces e.g.
> "~/AUR packages/".
> And on a stylistic level, you erratically switch from $variable to
> ${variable}, please choose one style and stick to it.
>
> --
> Eli Schwartz
>

I have updated my PKGBUILD. Thanks for the corrections. The reason I'm
using the RPM package is because the source doesn't build and I don't
have the skills to pull apart the cmake files to figure out why. The
best I can tell is it pulls in the src for all it's dependencies and
builds them when it builds JQ for static linking it fails and I don't
know how to fix the error. As for cleaning, would deleting the srcdir
be appropriate. Below are PKGBUILD, .install and service files.

Garrett Battaglia

PKGBUILD:
#Maintainer: Garrett Battaglia <garrett at garrettbattaglia.info>

pkgname=sysdig-falco-bin
pkgver=0.8.1
_sysdigver=0.19.1
pkgrel=1
pkgdesc="Behavioral Activity Monitoring With Container Support"
arch=('x86_64' 'i686')
url=https://github.com/draios/falco
license=('GPL')
depends=('dkms')
makedepends=('cmake' 'linux-headers')
provides=('sysdig-falco')

source_i686=(http://download.draios.com/stable/rpm/i386/falco-${pkgver}-i686.rpm)
source_x86_64=("http://download.draios.com/stable/rpm/x86_64/falco-${pkgver}-x86_64.rpm")
source=(
"${pkgname}-${pkgver}.tar.gz"::"https://github.com/draios/falco/archive/${pkgver}.tar.gz"
"sysdig-${_sysdigver}.tag.gz"::"https://github.com/draios/sysdig/archive/${_sysdigver}.tar.gz"
"sysdig-falco.service")
install="${pkgname}.install"

md5sums_x86_64=('5e017c747184101a0cc93ffc5b19ca47')
md5sums_i686=('07b77acbac557930a462b3527b555219')
md5sums=(
'f3c654ded00f3186f3ff92320204a747'
'6ad8b4a7d1b0aa10cd62397318117a67'
'58fe0d406874c6f565f648d3b10da62a')

prepare() {
cd "${srcdir}"
ln -sf "sysdig-${_sysdigver}" sysdig
ln -sf "falco-${pkgver}" falco
rm -rf ./etc/rc.d
cd ./falco
mkdir build

}

build() {
cd "${srcdir}/falco/build"
cmake ..
make driver
}

package() {

cd "${srcdir}"
mkdir "${pkgdir}/usr"
cp -r ./usr/* "${pkgdir}/usr/"
mkdir "${pkgdir}/etc"
cp -r ./etc/* "${pkgdir}/etc/"
mkdir -p "${pkgdir}/var/lib/dkms/falco/${pkgver}/${kernelVer}/${carch}/module/"
cp "${srcdir}/falco/build/driver/falco-probe.ko"
"${pkgdir}/var/lib/dkms/falco/${pkgver}/${kernelVer}/${carch}/module/falco-probe.ko"
mkdir -p "${pkgdir}/usr/lib/systemd/system/"
cp sysdig-falco.service "${pkgdir}/usr/lib/systemd/system/"
}

sysdig-falco-bin.install:
post_install() {
falco-probe-loader
}

post_upgrade() {
post_install
}

pre_remove() {
rmmod falco-probe
}

sysdig-falco.service:
[Unit]
Description=run sysdig falco

[Service]
Type=simple
ExecStart=/usr/bin/falco

[Install]
WantedBy=multi-user.target