[aur-general] PKGBUILD review

Thu Oct 19 16:01:08 UTC 2017

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

Then you should consider either backtracking from this task or learn.
You _have_ to understand how the underlying buildsystem works to be able
to correctly adopt the package for AUR.

First and foremost, get familiar with CMake on a basic level.

Second, look through project's set()s and option()s. By the time step
one is completed, you will have understood, why. They'll point you at
required changes that have to be done against the default configuration.

Third, use official repository or AUR packages to provide dependencies.
Not bundled ones.

Good luck.

On 19.10.2017 16:34, Garrett Battaglia via aur-general wrote:
> 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
> 

-- 
Regards,
Oleksii Vilchanskyi
PGP:0x8D3A0E046BDE941F2A53867CE3FD952D48C0B338

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20171019/2e0694a7/attachment.asc>