https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Bug ID: 2177855 Summary: Review Request: libXISF - Library to load and write XISF format Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mattia.verga@proton.me QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/mattia/Astronomy/fedora-r... SRPM URL: https://download.copr.fedorainfracloud.org/results/mattia/Astronomy/fedora-r... Description: LibXISF is C++ library to load and save images in XISF format that is native format PixInsight astronomical image processing program. It implements XISF 1.0 specifications. Fedora Account System Username: mattia
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://gitea.nouspiro.spac | |e/nou/libXISF
--- Comment #1 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5633131 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value Assignee|nobody@fedoraproject.org |ppisar@redhat.com Status|NEW |ASSIGNED Flags| |fedora-review? CC| |ppisar@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #2 from Petr Pisar ppisar@redhat.com --- URL address is fine. But Source0 address https://gitea.nouspiro.space/nou/libXISF/archive/libXISF-v0.2.0.tar.gz returns 404 HTTP error. Upstream https://gitea.nouspiro.space/nou/libXISF/releases does not list 0.2.0 version. Can you elaborate where can I retrieve the source archive to verify its origin?
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #3 from Petr Pisar ppisar@redhat.com --- TODO: Upstream uses lowercase libxisf name in the release archives. I suggest changing a name of this package to the lowercase variant https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_general_naming. It's similar to OpenSSH software which is packages as openssh RPM.
FIX: If the source archive is not publicly available, move the source address into a comment and only keep the file name in Source0 value. FIX: If the 0.2.0 version has not yet been released and this archive is only a prerelease, then follow https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_prerelease_versions to change release value accordingly.
Licenses found in the sources archive:
debian/copyright: GPL-3.0-or-later libxisf.cpp: GPL-3.0-or-later libxisf.h: GPL-3.0-or-later libXISF_global.h: GPL-3.0-or-later LICENSE: GPL-3.0 text lz4/: BSD-2-Clause pugixml/: MIT README.md: GPL-3.0-only (! this looks like an oversight by upstream) test/main.cpp: GPL-3.0-or-later utils.cpp: GPL-3.0-or-later variant.cpp: GPL-3.0-or-later zlib/: Zlib AND BSL-1.0
FIX: Add "GPL-3.0-only" to the license tag and ask upstream to clarify the license declaration found in REAMDE.md. "Licensed under GPLv3" means "GPL-3.0-only". Not "GPL-3.0-or-later" as it was probably intended.
TODO: Constrain "cmake" build-dependency with ">= 3.14" (CMakeLists.txt:1). TODO: Explicitly enable building shared libraries with "-DBUILD_SHARED_LIBS=ON" (CMakeLists.txt:11).
All tests pass. Ok.
$ rpmlint libXISF.spec ../SRPMS/libXISF-0.2.0-1.fc39.src.rpm ../RPMS/x86_64/libXISF-* ======================================== 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 checks: 31, packages: 6
========= 5 packages and 1 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.5 s ======== rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/x86_64/libXISF-0.2.0-1.fc39.x86_64.rpm drwxr-xr-x 2 root root 0 Mar 13 01:00 /usr/lib/.build-id drwxr-xr-x 2 root root 0 Mar 13 01:00 /usr/lib/.build-id/65 lrwxrwxrwx 1 root root 38 Mar 13 01:00 /usr/lib/.build-id/65/622df4e43fabc84df20d8766d5fb975d8f4ebf -> ../../../../usr/lib64/libXISF.so.0.2.0 lrwxrwxrwx 1 root root 16 Mar 13 01:00 /usr/lib64/libXISF.so.0 -> libXISF.so.0.2.0 -rwxr-xr-x 1 root root 392408 Mar 13 01:00 /usr/lib64/libXISF.so.0.2.0 drwxr-xr-x 2 root root 0 Mar 13 01:00 /usr/share/doc/libXISF -rw-r--r-- 1 root root 302 Mar 11 08:34 /usr/share/doc/libXISF/README.md drwxr-xr-x 2 root root 0 Mar 13 01:00 /usr/share/licenses/libXISF -rw-r--r-- 1 root root 35121 Mar 11 08:34 /usr/share/licenses/libXISF/LICENSE $ rpm -q -lv -p ../RPMS/x86_64/libXISF-devel-0.2.0-1.fc39.x86_64.rpm -rw-r--r-- 1 root root 1766 Mar 11 08:34 /usr/include/libXISF_global.h -rw-r--r-- 1 root root 13060 Mar 11 08:34 /usr/include/libxisf.h lrwxrwxrwx 1 root root 12 Mar 13 01:00 /usr/lib64/libXISF.so -> libXISF.so.0 File layout and permissions are Ok.
$ rpm -q --requires -p ../RPMS/x86_64/libXISF-0.2.0-1.fc39.x86_64.rpm | sort -f | uniq -c 1 glibc >= 2.37.9000-3 1 libc.so.6()(64bit) 1 libc.so.6(GLIBC_2.14)(64bit) 1 libc.so.6(GLIBC_2.2.5)(64bit) 1 libc.so.6(GLIBC_2.3.4)(64bit) 1 libc.so.6(GLIBC_2.32)(64bit) 1 libc.so.6(GLIBC_2.38)(64bit) 1 libc.so.6(GLIBC_2.4)(64bit) 1 libgcc_s.so.1()(64bit) 1 libgcc_s.so.1(GCC_3.0)(64bit) 1 libgcc_s.so.1(GCC_3.3.1)(64bit) 1 liblz4.so.1()(64bit) 1 libpugixml.so.1()(64bit) 1 libstdc++.so.6()(64bit) 1 libstdc++.so.6(CXXABI_1.3)(64bit) 1 libstdc++.so.6(CXXABI_1.3.2)(64bit) 1 libstdc++.so.6(CXXABI_1.3.5)(64bit) 1 libstdc++.so.6(CXXABI_1.3.9)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.11)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.14)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.15)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.18)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.20)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.21)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.26)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.29)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.30)(64bit) 1 libstdc++.so.6(GLIBCXX_3.4.9)(64bit) 1 libz.so.1()(64bit) 1 libz.so.1(ZLIB_1.2.0)(64bit) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsZstd) <= 5.4.18-1 1 rtld(GNU_HASH) $ rpm -q --requires -p ../RPMS/x86_64/libXISF-devel-0.2.0-1.fc39.x86_64.rpm | sort -f | uniq -c 1 libXISF(x86-64) = 0.2.0-1.fc39 1 libXISF.so.0()(64bit) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsZstd) <= 5.4.18-1 Binary requires are Ok.
$ rpm -q --provides -p ../RPMS/x86_64/libXISF-0.2.0-1.fc39.x86_64.rpm | sort -f | uniq -c 1 libXISF = 0.2.0-1.fc39 1 libXISF(x86-64) = 0.2.0-1.fc39 1 libXISF.so.0()(64bit) $ rpm -q --provides -p ../RPMS/x86_64/libXISF-devel-0.2.0-1.fc39.x86_64.rpm | sort -f | uniq -c 1 libXISF-devel = 0.2.0-1.fc39 1 libXISF-devel(x86-64) = 0.2.0-1.fc39 Binary provides are Ok.
$ resolvedeps rawhide ../RPMS/x86_64/libXISF{,-devel}-0.2.0-1.fc39.x86_64.rpm Binary dependencies are resolvable. Ok.
The package builds in F39 (https://koji.fedoraproject.org/koji/taskinfo?taskID=98812566). Ok.
Otherwise, this package is in line with Fedora and CMake packaging guidelines.
Please correct the FIX items, consider fixing TODO items, and provide updated spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #4 from Mattia Verga mattia.verga@proton.me --- (In reply to Petr Pisar from comment #3)
TODO: Upstream uses lowercase libxisf name in the release archives. I suggest changing a name of this package to the lowercase variant https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/ #_general_naming. It's similar to OpenSSH software which is packages as openssh RPM.
FIX: If the source archive is not publicly available, move the source address into a comment and only keep the file name in Source0 value. FIX: If the 0.2.0 version has not yet been released and this archive is only a prerelease, then follow https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/ #_prerelease_versions to change release value accordingly.
I've fixed the source URL as requested, gitea uses to rename the file on download, so I put the link URL as comment and just the file name in Source0. Version 0.2.0 is not listed in the "released" section, but is present in the tags list. I don't have a strong preference between uppercase and lowercase name, but considering the source file and mostly of the produced file are using the uppercase variant and considering that packaging guidelines now changed to use SHOULD instead of MUST be lowercase, I'd like to stay with the uppercase variant name.
Licenses found in the sources archive:
debian/copyright: GPL-3.0-or-later libxisf.cpp: GPL-3.0-or-later libxisf.h: GPL-3.0-or-later libXISF_global.h: GPL-3.0-or-later LICENSE: GPL-3.0 text lz4/: BSD-2-Clause pugixml/: MIT README.md: GPL-3.0-only (! this looks like an oversight by upstream) test/main.cpp: GPL-3.0-or-later utils.cpp: GPL-3.0-or-later variant.cpp: GPL-3.0-or-later zlib/: Zlib AND BSL-1.0
FIX: Add "GPL-3.0-only" to the license tag and ask upstream to clarify the license declaration found in REAMDE.md. "Licensed under GPLv3" means "GPL-3.0-only". Not "GPL-3.0-or-later" as it was probably intended.
I've contacted upstream maintainer and they already updated the README.md file to use "GPLv3 or later". Let me know if you you think it is needed to backport the modified README.md.
TODO: Constrain "cmake" build-dependency with ">= 3.14" (CMakeLists.txt:1). TODO: Explicitly enable building shared libraries with "-DBUILD_SHARED_LIBS=ON" (CMakeLists.txt:11).
Done.
Spec URL: https://mattia.fedorapeople.org/libXISF/libXISF.spec SRPM URL: https://mattia.fedorapeople.org/libXISF/libXISF-0.2.0-1.fc39.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #5 from Jakub Kadlčík jkadlcik@redhat.com --- Created attachment 1951611 --> https://bugzilla.redhat.com/attachment.cgi?id=1951611&action=edit The .spec file difference from Copr build 5633131 to 5676975
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #6 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5676975 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #7 from Petr Pisar ppisar@redhat.com --- If you use an archive from https://gitea.nouspiro.space/nou/libXISF/archive/v0.2.0.tar.gz, then use that URL in Source0. There is no point in hiding it in comments. It would be great to apply a 3b3d472149ca6651239c78b6af9762e91b9f46d1 upstream commit which amends README. Alternatively, update the package to 0.2.1 which already contains the change. I will approve this package as it is because a change of license in upstream is enough.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #8 from Mattia Verga mattia.verga@proton.me --- But if I use https://gitea.nouspiro.space/nou/libXISF/archive/v0.2.0.tar.gz RPM will try to unpack a source file named <v0.2.0.tar.gz> which doesn't exists because that URL downloads <libXISF-v0.2.0.tar.gz>. Am I missing something?
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #9 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/libXISF
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-9154501712 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-9154501712
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2023-03-20 17:35:59
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-9154501712 has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #12 from Petr Pisar ppisar@redhat.com --- I see. The server sends a preferred file name with "Content-Disposition: attachment; filename="libXISF-v0.2.0.tar.gz"; filename*=UTF-8''libXISF-v0.2.0.tar.gz" HTTP header. It's up to an HTTP client whether it will do the rename or not. E.g. wget, I used for examining the URL, does not do that (by default). "spectool -g libXISF.spec", a tool native to RPM, also does not do that. rpmbuild does not download any sources. I cannot see a problem. However, if you use an HTTP client which does the rename, you can instruct rpmbuild and spectool to rename with:
Source0: %{url}/archive/v%{version}.tar.gz#/%{name}-v%{version}.tar.gz
That should align rpmbuild with a downloader of your choice.
https://bugzilla.redhat.com/show_bug.cgi?id=2177855
--- Comment #13 from Mattia Verga mattia.verga@proton.me --- Thanks, I've downloaded the source with 'curl -O' and used <v%{version}.tar.gz>. Nice to know the trick to rename the sourcefile, though.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com