Spec URL: https://github.com/giuseppe/crun/blob/master/rpm/crun.spec.template SRPM URL: https://copr-be.cloud.fedoraproject.org/results/gscrivano/crun/srpm-builds/00921026/crun-0.6-1.fc30.src.rpm Description: an OCI runtime for running containers written in C. It can be used as a replacement for runc. It works with Podman, CRI-O and Moby. Fedora Account System Username: gscrivano
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=35614001 MUST items ---------- rpmlint output: $ rpmlint crun-0.6-1.fc29.src.rpm crun.src: E: no-changelogname-tag crun.src: W: invalid-url Source0: %{url}/archive/crun-0.6.tar.gz 1 packages and 0 specfiles checked; 1 errors, 1 warnings. $ rpmlint crun-0.6-1.fc29.x86_64.rpm crun.x86_64: E: no-changelogname-tag crun.x86_64: E: missing-call-to-chdir-with-chroot /usr/bin/crun crun.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libcrun.a 1 packages and 0 specfiles checked; 2 errors, 1 warnings. YES - package follows Naming Guidelines YES - spec file name matches base package %{name} NO - package does not meet Packaging Guidelines + %changelog stanza is missing. + Just %make_install should be sufficient. It's unnecessary to set INSTALL to "install -p". $ rpm --eval "%make_install" /usr/bin/make install INSTALL="/usr/bin/install -p ..." + The autogen.sh script doesn't actually use a NOCONFIGURE variable. YES - package is under a Fedora approved license and meets Licensing Guidelines YES - License field meets actual license + libcrun.a uses src/libcrun/cloned_binary.c and src/libcrun/chroot_realpath.c which are under ASL 2.0 and LGPLv2+ respectively. Pedantically speaking, the License tag for the library would be "LGPLv3+ and LGPLv2+ and ASL 2.0". See: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios + libocispec/COPYING contains the GPLv3, but it seems that the only piece of GPLv3-ed code is libocispec/src/validate.c which isn't really the primary artifact. NO - source package includes license text, which is included in %license + Should also include the LGPLv3+ text in COPYING.libcrun in %license, if the library is meant to be packaged. YES - spec file written in American English YES - spec file is legible NO - sources match upstream source + The URL field is wrong and leads to a 404 error: $ wget https://github.com/giuseppe/crun/archive/crun-0.6.tar.gz ... It could be changed to: https://github.com/giuseppe/crun/releases/download/%{version}/%{name}-%{version}.tar.gz The SHA256 hash of the tarball in the SRPM doesn't match any of the tarballs listed on the GitHub releases page. It's likely that the auto-generated GitHub tarballs don't have a stable hash, in which case the manually uploaded tarballs are better. YES - package compiles on all primary architectures YES - there is no need for ExcludeArch YES - all build dependencies in BuildRequires + Duplicated BuildRequires for autoconf, automake and gcc. YES - handles locales properly YES - no need for ldconfig ??? - doesn't bundle system libraries + Is glibc-static really needed? /usr/bin/crun seems to work without having it as a BuildRequire. YES - package is not relocatable YES - package owns all directories that it creates YES - files are listed only once in %files YES - file permissions set properly YES - consistent use of macros YES - contains code and permissable content YES - no need for doc subpackage YES - no chance of items marked as %doc affecting runtime NO - static libraries must be in a -static package + Should the /usr/lib64/libcrun.a static archive really be packaged? I don't see any HEADERS in the build scripts, which makes me think that it's only meant to be consumed by /usr/bin/crun itself. + If it's meant to be packaged, then should it really be a static library? If so, then there should be a crun-static sub-package. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries NO - development files in devel subpackage + If the library is meant to be packaged, then the accompanying header files should be in a crun-devel sub-package. ??? - devel subpackage requires base package + There's no -devel subpackage currently, but it's not clear if there should be one. NO - package removes all libtool archives + /usr/lib64/libcrun.la should be removed in the %install stanza with: find $RPM_BUILD_ROOT -name '*.la' -delete See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries YES - package doesn't need a .desktop file YES - doesn't own files or directories owned by other packages YES - all filenames are valid UTF-8 YES - package doesn't have deprecated dependencies SHOULD items: ------------- YES - upstream provides license text NO - description and summary doesn't have translations YES - package builds in Koji YES - builds on all primary architectures YES - package functions as described YES - package doesn't use scriptlets ??? - subpackages should require base package using fully versioned dependency + It's not clear if sub-packages are needed. YES - no pkgconfig files YES - no file dependencies outside of /etc/, /bin/, /sbin, etc. YES - contains man pages
thanks for the review. I've dropped the libraries from the package, as they are not really needed and addressed the other comments: https://www.scrivano.org/static/crun-rpm/
Great! Thanks for updating the package. Maybe you could expand the %description a bit to match up against runc, etc..? Not a blocker, though. There's this one rpmlint error, but it might be a false positive, and I will leave it to your judgement. $ rpmlint -i /home/rishi/devel/rpmbuild/RPMS/x86_64/crun-0.6-1.fc29.x86_64.rpm crun.x86_64: E: missing-call-to-chdir-with-chroot /usr/bin/crun This executable appears to call chroot without using chdir to change the current directory. This is likely an error and permits an attacker to break out of the chroot by using fchdir. While that's not always a security issue, this has to be checked. 1 packages and 0 specfiles checked; 1 errors, 0 warnings. Again, not a blocker, and you can suppress it in Taskotron tests run by Bodhi by adding a crun.rpmlintrc file: https://fedoraproject.org/wiki/Taskotron/Tasks/dist.rpmlint ACCEPTED
thanks for the review, indeed the chroot seems to be a false negative as the chdir is performed later on. I'll take a look at it through and see if I can change the code to not trigger the warning