Hide Forgot
Spec URL: http://in.waw.pl/~zbyszek/fedora/libkcapi.spec SRPM URL: http://in.waw.pl/~zbyszek/fedora/libkcapi-0.12.0-1.fc26.src.rpm Description: libkcapi allows user-space to access the Linux kernel crypto API. This library uses the netlink interface and exports easy to use APIs so that a developer does not need to consider the low-level netlink interface handling. The library does not implement any cipher algorithms. All consumer requests are sent to the kernel for processing. Results from the kernel crypto API are returned to the consumer via the library API. The kernel interface and therefore this library can be used by unprivileged processes. Fedora Account System Username: zbyszek koji build w/o arm64: http://koji.fedoraproject.org/koji/taskinfo?taskID=16538967 (Note that the build seems stuck on all 32bit arches, I'm looking into this.)
- BuildRequires: valgrind actually it's available not on all arches, so you can wrap it with %ifarch %{valgrind_arches} (same in %check) - ln -s libkcapi.so.0 %{buildroot}%{_libdir}/libkcapi.so I'm not sure what SONAME that library will have (I guess wrong)
See also: https://github.com/smuellerDD/libkcapi/pull/5 SONAME is set during build: gcc -shared -Wl,-soname,libkcapi.so.0 -o libkcapi.so.0.12.0 kcapi-kernel-if.o kdf.o -Wl,-z,relro,-z,now -Wl,--version-script,version.lds I think this is correct. Pfff, I forgot about valgrind. But this is a debugging measure anyway, hopefully it'll go away soon.
I'm not a packager yet, hence the review is unofficial. - Patchs https://fedoraproject.org/wiki/Packaging:Guidelines#Patch_Guidelines Patch01 -> Patch1 0001.... -> remove 0001, etc 0002-build-sys-create-all-destination-directories-and-do-.patch -> why an ending - ? For each patch, please add the issue URL to upstream - %description devel The %{name}-devel package contains header files for developing applications that use %{name}. => Header files for developing applications that use %{name} - %build Why are you forcing cflags instead of making patchs ? - %check %ifarch aarch64 ./test.sh || : %else ./test.sh || : %endif => why this "if" with same inside block ? - %license Have you forgotten COPYING.bsd ? - %doc %{_mandir}/man3/kcapi_*.3* => man pages are not %doc
Also : - BuildRequires gcc make are not needed - manpage should go to main package
(In reply to Thomas Andrejak from comment #3) > I'm not a packager yet, hence the review is unofficial. > > - Patchs > https://fedoraproject.org/wiki/Packaging:Guidelines#Patch_Guidelines > Patch01 -> Patch1 > 0001.... -> remove 0001, etc > 0002-build-sys-create-all-destination-directories-and-do-.patch -> why an > ending - ? > For each patch, please add the issue URL to upstream That's a git thing, it truncates patch subjects at a certain width. I'll add a link to the upstream PR. > - %description devel > The %{name}-devel package contains header files for developing > applications that use %{name}. > => Header files for developing applications that use %{name} OK. > - %build > Why are you forcing cflags instead of making patchs ? Hm, I don't understand. Why should I make patches? > > - %check > %ifarch aarch64 > ./test.sh || : > %else > ./test.sh || : > %endif > => why this "if" with same inside block ? Typo ;) > - %license > Have you forgotten COPYING.bsd ? No, COPYING already contains the text of the BSD license. But I guess it's safer to include both, I'll add it COPYING.bsd too. > - %doc %{_mandir}/man3/kcapi_*.3* > => man pages are not %doc Indeed, will fix. (In reply to Thomas Andrejak from comment #4) > Also : > > - BuildRequires gcc make are not needed No, fedora-review is misleading you. This changed a few months ago. See https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2. > - manpage should go to main package I don't think so. Those are all man pages in section 3, only useful for development.
(In reply to Thomas Andrejak from comment #3) > - %doc %{_mandir}/man3/kcapi_*.3* > => man pages are not %doc Oh, I misread. I though you meant that I forgot to put %doc on those pages, not that I should remove it. I don't see why you think %doc is not applicable, this is clearly documentation.
Updated in place: * Sun Nov 20 2016 Zbigniew Jędrzejewski-Szmek <zbyszek.pl> - 0.12.0-1 - Add more patches and link to upstream pull request - Update %%description - Drop valgrind invocation, it wasn't helping - Skip tests on 32 bits - Add BSD license file for completeness Spec URL: http://in.waw.pl/~zbyszek/fedora/libkcapi.spec SRPM URL: http://in.waw.pl/~zbyszek/fedora/libkcapi-0.12.0-1.fc26.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=16545948
For %doc and manpages, in https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages the example do not put the %doc macro. That's why I said it. The wiki is wrong ? > - %build > Why are you forcing cflags instead of making patchs ? Hm, I don't understand. Why should I make patches? => I don't think that overwriting CFLAGS is a good thing because the environment can set it. (I may think wrong). For example for hardening. So I try to never do this and prefer patch makefile or other file to do what I need to do. > - manpage should go to main package I don't think so. Those are all man pages in section 3, only useful for development. => Also in https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages , the example put manpage in main package. But you are right, this man page is for developpeurs, not users. I agree with you.
(In reply to Thomas Andrejak from comment #8) > For %doc and manpages, in > https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages the example do > not put the %doc macro. That's why I said it. The wiki is wrong ? There's no clear-cut guideline. In the example it's a page in section 1, so it makes some sense to install it even with --nodoc. It is also quite likely that %doc was simply forgotten. It's hard to say. Also note that the example is just that, it's the text that is authoritative. > > - %build > > Why are you forcing cflags instead of making patchs ? > > Hm, I don't understand. Why should I make patches? > => I don't think that overwriting CFLAGS is a good thing because the > environment can set it. (I may think wrong). For example for hardening. So I > try to never do this and prefer patch makefile or other file to do what I > need to do. To patch, to sed, or to set variables, is just a mechanism. Most things can be achieved with any of them, it's just a question which is the most convenient. In this case, I'd say, settings CLFAGS *is* that. Note that the hardening flags are used (via %optflags). This should always be verified by looking at the build log: gcc -fPIC -std=gnu99 -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -fwrapv -fvisibility=hidden -c -o kdf.o kdf.c
(BTW: I plan to return to this soon...)
A spec file working on Fedora 26 (tested with mockbuild) is now available upstream: https://github.com/smuellerDD/libkcapi/blob/master/libkcapi.spec
Hmm, I wanted to package libkcapi as a dependency for another project, but the direction shifted. I won't have time for this in the near future, so if somebody wants to take over, feel free.
(In reply to Zbigniew Jędrzejewski-Szmek from comment #12) > Hmm, I wanted to package libkcapi as a dependency for another project, but > the direction shifted. I won't have time for this in the near future, so if > somebody wants to take over, feel free. *** This bug has been marked as a duplicate of bug 1533929 ***