Spec URL: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06822023-libkdcraw/libkdcraw.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06822023-libkdcraw/libkdcraw-24.01.85-1.fc40.src.rpm Description: Libkdcraw is a C++ interface around LibRaw library used to decode RAW picture files. More information about LibRaw can be found at http://www.libraw.org. Fedora Account System Username:loise
This reopens the libkcdraw rpms that were closed due to renaming to kf5-libkdcraw, now with Qt6 and KF6 a Qt6/KF6 version is needed while keeping kf5-libkdcraw around until all qt5 consuming rpms are migrated.al
Copr build: https://copr.fedorainfracloud.org/coprs/build/6822043 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2255982-libkdcraw/fedora-rawhide-x86_64/06822043-libkdcraw/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libkdcraw Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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.
Taking this review.
A couple things I spotted: 1- The following can probably be removed: %if 0%{?rhel} == 8 ExclusiveArch: x86_64 ppc64le %{arm} %endif (As this is a kf6 package, and plasma 6 wont go to rhel 8) 2- Remove the following: %global revision %(echo %{version} | cut -d. -f3) %if %{revision} >= 50 %global stable unstable %else %global stable stable %endif 3- Replace %{stable} in the Source0 url by %{stable_kf6} (The macro you used in #2 has been added to our default macros so no need to declare it anymore)
%ldconfig_scriptlets can also be removed.
Hi Steve, 1. yes, I guess that can be removed. I took over the spec file from kf5-libkdcraw.spec and adapted it and thought I'd leave it there for the first try 2. I'm objecting to remove this part (together with #3) - the stable_kf6 macro doesn't do what it should do, delivering the url to the file to build the srpm file, because it is part of the kf6-rpm-macros rpm that gets loaded only at building the rpms from the srpm. So, 2 is needed as it is (and should be in all files back where it is necessary) to allow building the srpm outside of the fedora cache if it isn't already there or within a local SOURCES cache. %ldconfig_scriptlets - fine with removal if it can go away ;-)
Also, please add gcc-c++ and cmake as build requires.
1. SPEC: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06822117-libkdcraw/libkdcraw.spec 2. SRPM: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06822117-libkdcraw/libkdcraw-24.01.85-1.fc40.src.rpm Fixes: 1- The following can probably be removed: %if 0%{?rhel} == 8 ExclusiveArch: x86_64 ppc64le %{arm} %endif (As this is a kf6 package, and plasma 6 wont go to rhel 8) - %ldconfig_scriptlets removed - gcc-c++ + cmake added as BR - history log shortened
Created attachment 2006107 [details] The .spec file difference from Copr build 6822043 to 6822119
Copr build: https://copr.fedorainfracloud.org/coprs/build/6822119 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2255982-libkdcraw/fedora-rawhide-x86_64/06822119-libkdcraw/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libkdcraw Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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.
Please remove this: ``` %global revision %(echo %{version} | cut -d. -f3) %if %{revision} >= 50 %global stable unstable %else %global stable stable %endif ``` Alternatively you can do ``` %global stable %stable_kf6 ``` Are we sure `-DQT_MAJOR_VERSION=6` is needed? Isn't that the default?
(In reply to marcdeop from comment #11) > Please remove this: > > ``` > %global revision %(echo %{version} | cut -d. -f3) > %if %{revision} >= 50 > %global stable unstable > %else > %global stable stable > %endif > ``` > > Alternatively you can do > > ``` > %global stable %stable_kf6 Maybe you missed the discussion on IRC yesterday and the days before. I very much appreciate making things simpler and cleaner, but in this case, it breaks generating the URL if kf6-rpm-macros are not installed and availabe on a system (which is regularly the case using copr, where I noticed the stable_kf6 macro doesn't work when it should provide the URL for the download for building the srpm. I'm happy for more discussions finding out what is best to do and then change that, and if everyone insists on changing something proven functional into something that is not functional under all circumstances, then so be it, I will do that, but from a technical viewpoint it is not doing anymore what it should technically do :) Mind, I'm not discussing just because of it, I'm trying to find an equivalent solution that still guarantees the URL can be resolved (I made an example where it doesn't yesterday for those on IRC, I can re-do that again for you if you like) > ``` > > Are we sure `-DQT_MAJOR_VERSION=6` is needed? Isn't that the default? Yes, CMakeLists.txt does use that and defaults to 5, so if you want a version using Qt6 you have to set this (see the according parallel rpm for Qt5/KF5, https://src.fedoraproject.org/rpms/kf5-libkdcraw where I took the spec file and modified it)
@marcdeop: I made this change as the copr build works due to someone having uploaded the source tarball already to the fedora file cache, so in that case it works: SPEC: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06823402-libkdcraw/libkdcraw.spec SRPM: https://download.copr.fedorainfracloud.org/results/loise/libkdcraw/fedora-rawhide-x86_64/06823402-libkdcraw/libkdcraw-24.01.85-1.fc40.src.rpm If you do that with something that is not (yet) in the filecache, the copr builds fail building the srpm at the very beginning because the URL that needs to be used can't be resolved (containing the macro instead of the path) so you get a 404 error. That't the reason I'd like to discuss (any) working solution for this particular problem which is not at all specific to this rpm alone but a general issue.
Created attachment 2006253 [details] The .spec file difference from Copr build 6822119 to 6823512
Copr build: https://copr.fedorainfracloud.org/coprs/build/6823512 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2255982-libkdcraw/fedora-rawhide-x86_64/06823512-libkdcraw/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.
(In reply to loise from comment #12) > Maybe you missed the discussion on IRC yesterday and the days before. I very > much appreciate making things simpler and cleaner, but in this case, it > breaks generating the URL if kf6-rpm-macros are not installed and availabe > on a system (which is regularly the case using copr, where I noticed the > stable_kf6 macro doesn't work when it should provide the URL for the > download for building the srpm. I'm happy for more discussions finding out > what is best to do and then change that, and if everyone insists on changing > something proven functional into something that is not functional under all > circumstances, then so be it, I will do that, but from a technical viewpoint > it is not doing anymore what it should technically do :) > > Mind, I'm not discussing just because of it, I'm trying to find an > equivalent solution that still guarantees the URL can be resolved (I made an > example where it doesn't yesterday for those on IRC, I can re-do that again > for you if you like) Oh, I didn't miss anything. Your usage of COPR is the _exception_ here. you can "workaround" your particular problem (from the top of my head): - install the macros thing in the copr you are using first - upload a full srpm (which is what you should do anyway as internet downloads are normally forbidden on rpm creation) - upload the sources to the side-cache once you are a packager. The goal here is to make it build in Fedora's infrastructure, not in COPR. While I appreciate being flexible and try to accommodate other options (%autorelease and %autochangelog come to mind), to me having 6 lines *duplicated* in *hundreds* of packages is an absolute _no go_ (specially when there are ultra easy alternative solutions)
> While I appreciate being flexible and try to accommodate other options > (%autorelease and %autochangelog come to mind), to me having 6 lines > *duplicated* in *hundreds* of packages is an absolute _no go_ (specially > when there are ultra easy alternative solutions) Sure, that's why I changed it according to your wishes. Please check my latest SPEC, I replaced the 6 lines with the kf6 macro used elsewhere already which you referenced.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== C/C++: [-]: Provides: bundled(gnulib) in place as required. Note: Sources not installed [x]: Package does not contain kernel modules. [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]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [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. Note: Checking patched sources after %prep for licenses. No licenses found. Please check the source files for licenses manually. [x]: License file installed when any subpackage combination is installed. [?]: 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. [-]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [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]: The License field must be a valid SPDX expression. [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 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 8659 bytes in 1 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [x]: Reviewer should test that the package builds in mock. [-]: 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). [x]: 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. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [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]: 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: No rpmlint messages. [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: libkdcraw-24.01.85-1.fc40.x86_64.rpm libkdcraw-devel-24.01.85-1.fc40.x86_64.rpm libkdcraw-debuginfo-24.01.85-1.fc40.x86_64.rpm libkdcraw-debugsource-24.01.85-1.fc40.x86_64.rpm libkdcraw-24.01.85-1.fc40.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpxjabuna4')] checks: 31, packages: 5 libkdcraw.spec:48: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 48) libkdcraw.x86_64: W: incoherent-version-in-changelog 24.01.85-1 ['24.01.85-1.fc40', '24.01.85-1'] 5 packages and 0 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 0.3 s Rpmlint (debuginfo) ------------------- Checking: libkdcraw-debuginfo-24.01.85-1.fc40.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpekqjiij0')] checks: 31, packages: 1 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.1 s Rpmlint (installed packages) ---------------------------- (none): E: there is no installed rpm "libkdcraw". (none): E: there is no installed rpm "libkdcraw-debuginfo". (none): E: there is no installed rpm "libkdcraw-devel". (none): E: there is no installed rpm "libkdcraw-debugsource". There are no files to process nor additional arguments. Nothing to do, aborting. ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 4 0 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 filtered, 0 badness; has taken 0.0 s Source checksums ---------------- https://download.kde.org/unstable/release-service/24.01.85/src/libkdcraw-24.01.85.tar.xz : CHECKSUM(SHA256) this package : f2b77dbf3eb363653a16dc3646201b2fc300dd0af511a89000512dd3e8942248 CHECKSUM(SHA256) upstream package : f2b77dbf3eb363653a16dc3646201b2fc300dd0af511a89000512dd3e8942248 Requires -------- libkdcraw (rpmlib, GLIBC filtered): kf6-filesystem libQt6Core.so.6()(64bit) libQt6Core.so.6(Qt_6)(64bit) libQt6Core.so.6(Qt_6.6)(64bit) libQt6Gui.so.6()(64bit) libQt6Gui.so.6(Qt_6)(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libm.so.6()(64bit) libraw.so.23()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) rtld(GNU_HASH) libkdcraw-devel (rpmlib, GLIBC filtered): cmake(Qt6Gui) cmake-filesystem(x86-64) libKDcrawQt6.so.5()(64bit) libkdcraw(x86-64) libkdcraw-debuginfo (rpmlib, GLIBC filtered): libkdcraw-debugsource (rpmlib, GLIBC filtered): Provides -------- libkdcraw: libKDcrawQt6.so.5()(64bit) libkdcraw libkdcraw(x86-64) libkdcraw-devel: cmake(KDcrawQt6) cmake(kdcrawqt6) libkdcraw-devel libkdcraw-devel(x86-64) libkdcraw-debuginfo: debuginfo(build-id) libKDcrawQt6.so.5.0.0-24.01.85-1.fc40.x86_64.debug()(64bit) libkdcraw-debuginfo libkdcraw-debuginfo(x86-64) libkdcraw-debugsource: libkdcraw-debugsource libkdcraw-debugsource(x86-64) Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/bin/fedora-review --no-colors --prebuilt --rpm-spec --name libkdcraw --mock-config /var/lib/copr-rpmbuild/results/configs/child.cfg Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Generic, Shell-api Disabled plugins: Java, Python, R, Haskell, fonts, PHP, Perl, Ocaml, SugarActivity Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
This generally looks fine to me, so... PACKAGE APPROVED.
Hello @loise, since this is your first Fedora package, you need to get sponsored by a package sponsor before it can be accepted. A sponsor is an experienced package maintainer who will guide you through the processes that you will follow and the tools that you will use as a future maintainer. A sponsor will also be there to answer your questions related to packaging. You can find all active sponsors here: https://docs.pagure.org/fedora-sponsors/ I created a sponsorship request for you: https://pagure.io/packager-sponsors/issue/612 Please take a look and make sure the information is correct. Thank you, and best of luck on your packaging journey. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
I've sponsored you as a packager, congratulations and welcome! :)