[Bug 889505] Review Request: libkqueue - A userspace implementation of the kqueue kernel event notification mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=889505

--- Comment #10 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
A bit of feedback on the reviewer comments would be very welcome, too, and
could result in faster progress and even better guiding/hints. For example, one
reviewer has told you not to package the .a and .la file, another pointed out
that the package would violate the static library guidelines, but you've not
commented on that yet.

Many things in the packaging guidelines are not in there just for fun, but with
the goal of avoiding packaging pitfalls.


> %package static

_Why_ is the static lib being built?

[Regardless of that question, a -static subpackage would not need to depend on
the base package (a run-time shared library package), but the -devel package.]


> rpmbuild is creating an empty -debuginfo package. I don't yet know why this
> is happening.

Perhaps the "redhat-rpm-config" package is not installed. One can get it also
as a dependency when installing the "fedora-packager" package. Or the
test-build is not done with Mock (or the Fedora Build System "koji" if you
follow the
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers process
steps), or the built code applies its own insufficient compiler flags, or it
strips the built code.


> License:    MIT and BSD

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

That's one of the early items on the following list:
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines


> %{_configure} --prefix=%{_prefix}

The %{_configure} macro expands to just "./configure", so using the macro
doesn't add any value. Better would be to add a comment to the spec file that
explains _why_ "./configure" is used. Is it a custom-made configure script and
probably incompatible with the more commonly used %{configure} macro? Check
out:

  $ rpm --eval %_configure
  $ rpm --eval %configure


> make install DESTDIR=$RPM_BUILD_ROOT LIBDIR=$RPM_BUILD_ROOT%{_libdir}
> MANDIR=$RPM_BUILD_ROOT%{_mandir} INCLUDEDIR=$RPM_BUILD_ROOT%{_includedir}

Does it use DESTDIR at all? As a prefix for the "install" target? If so, that
raises the question why to override the other variables, too? Btw, there's the
%{make_install} macro these days, which saves some typing in the spec file.


> the base package which contains the man pages.

That's the wrong package for them. They are development related. Section 2
(system calls).


> %{_includedir}/kqueue/sys/event.h

The "unowned" directory issue is still present.


> %{_libdir}/libkqueue.so

This file is included in the wrong package.


> %{_libdir}/libkqueue.la

The previously linked packaging guidelines say something about these files.


> %{_libdir}/pkgconfig/libkqueue.pc

%{_libdir} is not substituted correctly in this file. It's hardcoded to
/usr/lib here for x86_64

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=kYDIlmrSrs&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]