Fedora Account System
Red Hat Associate
Red Hat Customer
SPEC URL: https://download.copr.fedorainfracloud.org/results/sfaulken/testing/fedora-rawhide-x86_64/10711298-libfm-extra/libfm-extra.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/sfaulken/testing/fedora-rawhide-x86_64/10711298-libfm-extra/libfm-extra-1.4.1-1.fc45.src.rpm Description: glib/gio-based bootstrap library for libfm and menu-cache. Provides some file management utilities missing in gtk+/glib. FAS Username: sfaulken Reproducible: Always
Currently, in Rawhide, and the Fedora point releases, there exists a circular build dependency, with libfm and menu-cache, this hasn't previously been an issue, due to both packages existing, and there not having been any Version mismatches or ABI changes. This circular dependency was found while attempting to package libfm-qt for EPEL10, where these packages did not already exist. According to menu-cache upstream https://github.com/lxde/menu-cache/issues/14 Installing: Since version 0.7.0, libmenu-cache requires libfm-extra for the menu-cache-gen binary that generates the menu cache. Since libfm depends on libmenu-cache, there are instructions for bootstrappers on how to build these libraries together. First, you need to create libfm-extra. You can easily do this by passing the '--with-extra-only' option to the libfm configure script and by temporarily installing libfm-extra. Then, you can successfully build libmenu-cache, followed by building the full version of libfm. This package satisfies this dependency, and is in line with with how other distributions are handling the issue.
Taking this review.
Initial spec review: > Source0: %{url}/archive/%{version}.tar.gz This should be "%{url}/archive/%{version}/libfm-%{version}.tar.gz" > BuildRequires: gcc-c++ You are missing BRs for make, autoconf, automake, and maybe autoconf-archive (if that's used for this)? > %{?ldconfig_scriptlets} This does nothing across all supported Fedora and RHEL releases. > %{_libdir}/%{name}.so.%{soname} > %{_libdir}/%{name}.so.%{soname}.* This can be simplified to "%{_libdir}/%{name}.so.%{soname}{,.*}"
Updated: SPEC URL: https://download.copr.fedorainfracloud.org/results/sfaulken/testing/fedora-rawhide-x86_64/10714059-libfm-extra/libfm-extra.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/sfaulken/testing/fedora-rawhide-x86_64/10714059-libfm-extra/libfm-extra-1.4.1-1.fc45.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/10714176 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2499516-libfm-extra/fedora-rawhide-x86_64/10714176-libfm-extra/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Created attachment 2148513 [details] The .spec file difference from Copr build 10714176 to 10714205
Copr build: https://copr.fedorainfracloud.org/coprs/build/10714205 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2499516-libfm-extra/fedora-rawhide-x86_64/10714205-libfm-extra/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
This comment was flagged as spam, view the edit history to see the original text if required.
> The code built with `--with-extra-only`, including `fm-xml-file.c` and the installed public headers, is explicitly licensed under `LGPL-2.1-or-later`. :+1: And it's not that that upstream *should* provide the license file, they *must* provide it. We can take the liberty here to add the license file in downstream until upstream accepts the patch to include it. Tip for @uwsy1059, run locally `fedora-review -b 2499516` and you will get the full license-review breakdown (the one mentioned in the review template). It may not catch all license texts and it often misinterprets license, but it can give you a good head start for checking if the project has differently license files. Common things you should manually verify are test data providence and pre-compiled binaries. Other 2 points, yes :+1:. The dir one is not that pressing if nothing else is expected to co-own the directory (don't remember if it would leave `%{_includedir}/libfm-1.0` folder upon uninstall though). Some extra review points: - One thing suspicious is why is `--disable-static` not sufficient and you have to do a `find $RPM_BUILD_ROOT -name '*.la'`? - Can you manually do `autoreconf` instead of the `autogen.sh`?