Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec SRPM URL: https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm Description: libuEv is a small event loop that wraps the Linux epoll() family of APIs. It is similar to the more established libevent, libev and the venerable Xt(3) event loop. The µ in the name refers to both its limited feature set and the size impact of the library. Fedora Account System Username: alciregi
It is needed in order to build uredir (https://github.com/troglobit/uredir), an UDP port redirector.
Koji scratch build https://koji.fedoraproject.org/koji/taskinfo?taskID=64631903
I will review. This looks simple enough, but is the first time I encounter libtool and pkgconfig, so please be patient if I need a bit of time to understand the build system. Issues found by running fedora-review and inspecting output follow. This is just the first look at the package, so there may still be other issues that automation cannot detect: - Libtool archives (.la) should not be packaged Note: libuev : /usr/lib64/libuev.la See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries - Development (unversioned) .so files should be in the -devel subpackage. Note: Unversioned so-files directly in %_libdir. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages - Static library libuev.a should not be included at all, the shared object should be used by all dependencies. If the static library is really required for some reason, it should go to -static subpackage. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries - Rpmlint reveals the following problems: libuev.x86_64: W: incoherent-version-in-changelog 3.3-1 ['2.3.2-1.fc35', '2.3.2-1'] libuev.src:33: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 33) - Complains about the use of AM_CONFIG_HEADER found in: libuev-2.3.2/configure.ac:7 This is just an extra item, however it would be nice to contact upstream about using its replacement. See: https://ftp.gnu.org/old-gnu/Manuals/automake-1.7.2/html_chapter/automake_5.html#SEC23 Also I noticed a strange thing: the rpm both Requires and Provides libuev.so.2()(64bit). I need to look deeper into that, how that happened and is that a problem.
Thank you very much. New files: Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec SRPM URL: https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm
Great, it is getting better. Another set of findings still: - Main package documentation could be added: AUTHORS, ChangeLog, README - Documentation for -devel: - There is %{_pkgdocdir} which resolves to %{_docdir}/%{name} - But do not do that if also %doc is used, reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation - At the moment, part of the material goes to /usr/share/doc/libuev, other part to /usr/share/doc/libuev-devel. Everything marked as -devel package docs should go to -devel directory. I think using %doc only would correct this. - In examples, .gitignore is not useful and should not be included. Probably the same also holds for Makefiles - Directories without known owners: /usr/share/doc/libuev I think this comes from the problem above where -devel installs docs to main package's directory. You could run fedora-review after the changes and see if the error remains. Or submit the new files and I will run it as usual. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership I slightly worry over licensing and libuev-2.3.2/src/bench.c which has different license and copyright owner than the rest. But, it seem to be built separately from the library and not installed, so it does not require License entry for binary rpm, and includes the license in the file itself, so source rpm licensing is ok as well. So, I think it can be like it is. Apart from these, everything looks good. fedora-review was unable to download Source, but perhaps that was a transient failure. I will ignore that for now and see how it goes for the next version.
Ok. I have a doubt. If I don't put LICENSE under %doc, I get Installed (but unpackaged) file(s) found: /usr/share/doc/libuev/LICENSE So, %license macro is still necessary? Or is it fine to have the LICENSE file under /usr/share/doc/libuev/LICENSE and under /usr/share/licenses/libuev/LICENSE? New files: Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec SRPM URL: https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm
%license must be always used, it puts the license to standard license file location in Fedora, which is /usr/share/licenses. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/ The complain from unstalled but unpackaged files appears, because upstream build script installs the LICENSE to buildroot (to docs it seems), then rpm detects that there is a file installed in buildroot that is not handled in %files. There are at least two ways to fix this: Copy the license to docs as you were already doing. The it appears both in /usr/share/licenses and /usr/share/doc. That is kind of redundant, but not a big deal. Or get rid of the file with 'rm /unwanted/file' in the end of %install section, which avoids the duplication. The warning can also be silenced by using %exclude in %files, but apparently it will stop working in the future: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/RWUGI5HAPH6FTM46QUC534ZE4HLMMOSG/ I will take a look at the new files later.
Great. Thank you for the explanation.
I found one important issue still. I apologize for not bringing this up earlier, I simply missed it before now: - Shared object should not be installed with glob %{_libdir}/%{name}.so.*, %{_libdir}/%{name}.so.2* would be better to avoid accidentally making an abi breaking update. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files An then two minor items I just note but will not insist on: - -devel package description mentions static libary files, but none are installed. Perhaps that part can be omitted. - API.md would fit -devel package docs better, since it is interesting only for development. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation say this: > For example API documentation belongs in the -devel subpackage, not the main one.
- This disable default ldflags: LDFLAGS="-fPIE" --disable-static Use LDFLAGS="%build_ldflags -fPIE" --disable-static - In order to avoid unintentional soname bump, we recommend not globbing the major soname vension, be more specific instead: %{_libdir}/%{name}.so.* - Own %{_includedir}/uev/ or remove the glob - explicitly BR make
Since there are two so files (libuev.so.2.2.0 and libuev.so.2), using %{_libdir}/%{name}.so.2* is ok? Thanks.
(In reply to Robert-André Mauchin 🐧 from comment #10) > Use LDFLAGS="%build_ldflags -fPIE" --disable-static Yes. Right. But if I use this, checks fails with: /usr/bin/ld: active.o: relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIE
(In reply to Alessio from comment #12) > (In reply to Robert-André Mauchin 🐧 from comment #10) > > Use LDFLAGS="%build_ldflags -fPIE" --disable-static > > Yes. Right. > But if I use this, checks fails with: > > /usr/bin/ld: active.o: relocation R_X86_64_32 against `.rodata' can not be > used when making a PIE object; recompile with -fPIE Ah. This is because in tests/Makefile the CFLAGS variable is "hard coded". tests/Makefile.am contains CFLAGS = -W -Wall -Wextra -Wno-unused-result -Wno-unused-parameter So, after running autogen, tests/Makefile.in will contain such stuff, instead of "CFLAGS = @CFLAGS@"
Thank you for your patience and your advises. These are the new files: Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec SRPM URL: https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm
(In reply to Alessio from comment #11) > Since there are two so files (libuev.so.2.2.0 and libuev.so.2), using > %{_libdir}/%{name}.so.2* is ok? > Thanks. Yes it is ok.
(In reply to Alessio from comment #14) > Thank you for your patience and your advises. > > These are the new files: > Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec > SRPM URL: > https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm Looks good. Only two points brought up by Robert-André Mauchin 🐧 are still unresolved: - Own %{_includedir}/uev/ or remove the glob. So you could do in %files simply this and get both directory ownership and the contents installed (reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership): %{_includedir}/uev - explicitly BR make: BuildRequires: make
Ok. ... BuildRequires: make ... %files devel ... %{_includedir}/uev These are the new files: Spec URL: https://alciregi.fedorapeople.org/uredir/libuev.spec SRPM URL: https://alciregi.fedorapeople.org/uredir/libuev-2.3.2-1.fc34.src.rpm
Great! Everything seems to be in order now, review passed. You may now request a repository for this package as explained at https://fedoraproject.org/wiki/Package_Review_Process#Contributor Thank you for contributing to Fedora!
Yay! Thank you for your assistance.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libuev
FEDORA-2021-6d7cab8d70 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-6d7cab8d70
FEDORA-2021-6d7cab8d70 has been pushed to the Fedora 34 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-6d7cab8d70 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-6d7cab8d70 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
Should the Fedora 34 update be pushed to stable so that libuev would be available in Fedora 34 or has the plan been changed to only include this in Fedora 35? I am asking because a) a build for Fedora 34 has been submitted, but not pushed to stable, and b) bug 1954212, which, as far as I can tell, would be closed by pushing this.
FEDORA-2021-6d7cab8d70 has been pushed to the Fedora 34 stable repository. If problem still persists, please make note of it in this bug report.