Bug 1396819

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