Spec URL: https://git.sr.ht/~fnux/hikari-rpm/blob/12bafe7020151cdd30c6fba6e6a2bb01ce9a4cbd/libucl/libucl.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/fnux/hikari/fedora-rawhide-x86_64/01654056-libucl/libucl-0.8.1-2.fc34.src.rpm Description: Universal configuration library parser. Fedora Account System Username: fnux
Some important items for this library package: * https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package * https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries * https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites > %build > make %{?_smp_mflags} Compiler/linker messages don't tell much, so look for options to make them verbose, which will make build.log output much more useful. V=1 make %{?_smp_mflags} > rw-r--r-- /usr/lib64/pkgconfig/libucl.pc $ pkg-config --cflags libucl -I/usr/include/ Since /usr/include is a standard search path, explicitly adding it via -I can troublesome in projects that want to customize the include directories.
Spec URL: https://git.sr.ht/~fnux/hikari-rpm/blob/4038784238db53f19d5f40753b918ad1e07165fa/libucl/libucl.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/fnux/hikari/fedora-rawhide-x86_64/01849059-libucl/libucl-0.8.1-3.fc34.src.rpm Thanks for the tips, Micheal! I made the devel subpackage require arch-dependent base package, and run the test suite in the check section. I don't think the 'packaging static library' guidelines are relevant here, since the package ships a dynamically linked library...? > Since /usr/include is a standard search path, explicitly adding it via -I can troublesome > in projects that want to customize the include directories. Regarding the above topic: I suppose I can remove `Cflags: -I${includedir}/` from /usr/lib64/pkgconfig/libucl.pc, althought such a thing seems fairly common (if not present everywhere) from what I can see on my system. There is, as far as I know, no guideline on this: I think I would let the upstream file untouched, but am not strongly opinionated.
> %{_libdir}/libucl.a That _is_ a static library. > %{_libdir}/libucl.la That is a socalled libtool archive, which is also covered by the linked packaging guidelines, because .la files are an extra source of trouble (not limited to introducing a complex and growing set of inter-library dependencies at build-time). > %check It is worth noting that the %check section is executed _after_ %install, which also makes it possible to run tests on files installed into %buildroot, so within the spec file, usually %check is placed after %install as to be explicit. > `Cflags: -I${includedir}/` Well, you could remove the trailing slash character, and pkg-config will be smart enough to recognize the standard header path. $ pkg-config --cflags libucl -I/usr/include/ $ sudo sed -i 's!}/$!}!g' /usr/lib64/pkgconfig/libucl.pc $ pkg-config --cflags libucl $
> That _is_ a static library. Oops, true. It sneaked in the -devel package alongside the .so. We have no need for the statically compiled output here, I will make sure it is not build/shipped. Thanks! > It is worth noting that the %check section is executed _after_ %install, which also makes it possible to run tests on files installed into %buildroot, so within the spec file, usually %check is placed after %install as to be explicit. I am aware. It's a typo-class issue on my side. > Well, you could remove the trailing slash character, and pkg-config will be smart enough to recognize the standard header path. Didn't know that. Thanks again!
Spec URL: https://git.sr.ht/~fnux/hikari-rpm/blob/4db5b86af162088189d7f247d1fc105e114c2162/libucl/libucl.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/fnux/hikari/fedora-33-x86_64/01849088-libucl/libucl-0.8.1-4.fc33.src.rpm
I’m not doing a full review at this time, but here are a few things I noticed: The upstream package contains bindings for Python, Lua, and Haskell. All of these would be useful in Fedora. I think the package is incomplete without these language bindings. “V=1 make %{?_smp_mflags}” would be better expressed as “%make_build”, and “V=1 make %{?_smp_mflags} check” would be better expressed as “%make_build check”. An accepted change for Fedora 34 is that packages using make must BR it explicitly (“BuildRequires: make”). The guidelines have not yet been updated. In python/setup.py, the metadata claims an MIT license; however, the overall license of the source tarball is BSD. None of the Pythong binding sources have a license/copyright comment. Since MIT is by far the most common license in Python land, this is a possible copy-paste error. Upstream should be queried to clarify the license of the Python bindings. If MIT is intended, upstream should be asked to include a file in the python/ subdirectory with the license text. Note that the MIT license would *require* the license text to accompany any distribution. See https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/. Currently, the python bindings are not packaged, but I think they should be. If upstream states they are MIT-licensed and adds a license file, then the python3-ucl subpackage will have license “MIT”. The contents of the klib subdirectory are MIT-licensed and have the same issue. Additionally, they comprise a bundled static (header-only) partial copy of the klib library (https://attractivechaos.github.io/klib/). See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_packaging_header_only_libraries and also https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling for options. These sections of the guidelines have changed quite a bit recently. Similarly, src/tree.h is MIT-licensed and is a bundled static (header-only) copy of https://piumarta.com/software/tree/. Similarly, src/mum.h is MIT-licensed and is a bundled static (header-only) copy of https://github.com/vnmakarov/mum-hash. The overall license of the main package will be “BSD and MIT” unless *all* MIT-licensed bundled libraries are unbundled and removed in prep. You should be able to just pass --disable-static to the configure script (“%configure --disable-static”) instead of patching the call to LT_INIT in configure.ac. The patch to remove the trailing slash from libucl.pc.in should be sent upstream.
Oh, and the contents of uthash/ are another bundled header-only library—this time, one that actually exists in Fedora.
SPEC URL: https://git.sr.ht/~fnux/hikari-rpm/blob/3fbdef61709ccb7102cffe3ccc6f39b78ed84f37/libucl/libucl.spec SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/7366/63207366/libucl-0.8.1-2.fc33.src.rpm > The upstream package contains bindings for Python, Lua, and Haskell. All of these would be useful in Fedora. I think the package is incomplete without these language bindings. I built the python bindings since it was a low-hanging fruit. I'll build the lua and haskell ones later on / on demand, if needed. > “V=1 make %{?_smp_mflags}” would be better expressed as “%make_build”, and “V=1 make %{?_smp_mflags} check” would be better expressed as “%make_build check”. Fixed. > An accepted change for Fedora 34 is that packages using make must BR it explicitly (“BuildRequires: make”). The guidelines have not yet been updated. Fixed. > Bundled libraries. The build system is a pain and does not seem to eat includes properly. I kept the partially-bundled libraries in but marked themo so.
The linked SRPM is built with an old version of the spec file; it does not match the spec URL. ----- It’s not hard to unbundle at least the header-only libraries. For example, for uthash, which was already in Fedora: 1. Per the guidelines for depending on header-only libraries, you need to BR uthash-static in addition to uthash-devel. 2. In %prep, “rm -rf uthash/*” That’s all! Now it uses the system copy. Now there are some problems with uthash 2.x, vs. the bundled 1.x: * libucl uses removed macros utstring_append_len() and utstring_append_c(); this could be easily patched by defining them, if missing, in src/ucl_internal.h * ucl_parser.c uses strtoimax without directly including inttypes.h, which was previously indirectly included from uthash.h; this is also easily patched * ucl_emitter_utils.c uses uthash internals, accessing the pd member directly, but this member no longer exists in 2.x Okay, now it’s getting hard. At this point we could decide that this is more than we are willing to try to patch, and leave the library bundled—not because of the build system, but because of the uthash 2.x incompatibility. In that case, you need to: 1. Drop the BR’s on uthash-devel and uthash-static 2. Write the virtual Provides for the bundled library correctly: instead of Provides: bundled(uthash-devel) it should be # See UTHASH_VERSION in uthash/uthash.h Provides: bundled(uthash) = 1.9.8 3. File an upstream bug report about the incompatibility and link it from the spec file! ----- You really need to try to unbundle at least the other header-only libraries, since it is as easy as removing the bundled copies in %prep (which you are required to do for bundled libraries anyway). That’s pretty much upstream support. They only thing upstream could do to make it easier would be to provide a configure argument. I think the guidelines don’t really give you any wiggle room here. Note that for anything you do not unbundle because upstream does not support using an external/system copy, you are required to publicly contact upstream (e.g. via a GitHub issue) requesting that support. ----- The Python bindings still have a license problem, as they claim to be MIT-licensed but lack a license/copyright statement. I see there is now an upstream bug report about this: https://github.com/vstakhov/libucl/issues/252.
Spec URL: https://paste.sr.ht/blob/4cc7741a92b9e6c153e22c907dee186348be1530 SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/5128/64485128/libucl-0.8.1-6.fc34.src.rpm I unbundled both mum-hash and libtree (both in Rawhide by now) but klib was fairly patched after bundling: https://github.com/vstakhov/libucl/commits/master/klib I'll open an issue upstream regarding uthash later - I have to go. Thanks for your patience / detailed explanations, much appreciated :-)
Spec URL: https://paste.sr.ht/blob/4cc7741a92b9e6c153e22c907dee186348be1530 SRPM URL: https://paste.gnugen.ch/paste/54wq Here's an updated SRPM URL (the previous one expired) as mentionned on the sway-sig mailing list. Thanks!
I have trouble building the package in mock: 1. You are missing a `BuildRequires: python3-devel`; without it, the %py3_build macro is not expanded (!) and the build fails with "no job control" error (a bit cryptic, but that error usually means an unexpanded macro). 2. After correcting that, the python build step fails with `ucl.h: No such file or directory` (`#include <ucl.h>`). I'll leave this to you to figure out. -- I also had problems with `fedora-review -b 1910504` picking the latest links (it still picked up the previous ones) – my guess is that it looks for `*.spec`/`*.src.rpm` in the URL and the pastes do not match. My suggestion would be making a COPR build and linking to the resulting artifacts from rawhide chroot – that way we can be reasonably certain that the package will build even on my machine and the links would have the expected format.
Spec URL: https://download.copr.fedorainfracloud.org/results/fnux/hikari/fedora-rawhide-x86_64/02128850-libucl/libucl.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/fnux/hikari/fedora-rawhide-x86_64/02128850-libucl/libucl-0.8.1-7.fc35.src.rpm The python subpackages was indeed looking for the library which was not yet installed. It now builds against rawhide (note that some dependencies are only available in F35+).
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: libucl-0.8.1-7.fc35.x86_64.rpm libucl-devel-0.8.1-7.fc35.x86_64.rpm python3-libucl-0.8.1-7.fc35.x86_64.rpm libucl-debuginfo-0.8.1-7.fc35.x86_64.rpm libucl-debugsource-0.8.1-7.fc35.x86_64.rpm libucl-0.8.1-7.fc35.src.rpm libucl.x86_64: W: shared-lib-calls-exit /usr/lib64/libucl.so.5.1.0 exit.5 libucl-devel.x86_64: W: summary-not-capitalized C libucl development files libucl-devel.x86_64: W: no-documentation python3-libucl.x86_64: W: no-documentation libucl.src:24: W: unversioned-explicit-provides bundled(klib) 6 packages and 0 specfiles checked; 0 errors, 5 warnings. Rpmlint (debuginfo) ------------------- Checking: libucl-debuginfo-0.8.1-7.fc35.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- python3-libucl.x86_64: W: no-documentation libucl-devel.x86_64: W: summary-not-capitalized C libucl development files libucl-devel.x86_64: W: no-documentation libucl.x86_64: W: shared-lib-calls-exit /usr/lib64/libucl.so.5.1.0 exit.5 5 packages and 0 specfiles checked; 0 errors, 4 warnings. Unversioned so-files -------------------- python3-libucl: /usr/lib64/python3.9/site-packages/ucl.cpython-39-x86_64-linux-gnu.so Source checksums ---------------- https://github.com/vstakhov/libucl/archive/0.8.1.tar.gz#/libucl-0.8.1.tar.gz : CHECKSUM(SHA256) this package : a6397e179672f0e8171a0f9a2cfc37e01432b357fd748b13f4394436689d24ef CHECKSUM(SHA256) upstream package : a6397e179672f0e8171a0f9a2cfc37e01432b357fd748b13f4394436689d24ef Requires -------- libucl (rpmlib, GLIBC filtered): libc.so.6()(64bit) libm.so.6()(64bit) rtld(GNU_HASH) libucl-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config libucl(x86-64) libucl.so.5()(64bit) python3-libucl (rpmlib, GLIBC filtered): libc.so.6()(64bit) libpthread.so.0()(64bit) libucl(x86-64) libucl.so.5()(64bit) python(abi) rtld(GNU_HASH) libucl-debuginfo (rpmlib, GLIBC filtered): libucl-debugsource (rpmlib, GLIBC filtered): Provides -------- libucl: bundled(klib) bundled(uthash) libucl libucl(x86-64) libucl.so.5()(64bit) libucl-devel: libucl-devel libucl-devel(x86-64) pkgconfig(libucl) python3-libucl: python-libucl python3-libucl python3-libucl(x86-64) python3.9-libucl python3.9dist(ucl) python3dist(ucl) libucl-debuginfo: debuginfo(build-id) libucl-debuginfo libucl-debuginfo(x86-64) libucl-debugsource: libucl-debugsource libucl-debugsource(x86-64) Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1910504 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic, C/C++ Disabled plugins: Perl, PHP, Haskell, SugarActivity, fonts, Java, Ocaml, Python, R Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Package approved.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libucl
Thanks!