Bug 2177855 - Review Request: libXISF - Library to load and write XISF format
Summary: Review Request: libXISF - Library to load and write XISF format
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL: https://gitea.nouspiro.space/nou/libXISF
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-03-13 17:14 UTC by Mattia Verga
Modified: 2023-03-20 17:43 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-03-20 17:35:59 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 5633131 to 5676975 (1.19 KB, patch)
2023-03-18 09:17 UTC, Jakub Kadlčík
no flags Details | Diff

Description Mattia Verga 2023-03-13 17:14:27 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/mattia/Astronomy/fedora-rawhide-x86_64/05633065-libXISF/libXISF.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/mattia/Astronomy/fedora-rawhide-x86_64/05633065-libXISF/libXISF-0.2.0-1.fc39.src.rpm
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

Comment 1 Jakub Kadlčík 2023-03-13 17:25:44 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5633131
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2177855-libXISF/fedora-rawhide-x86_64/05633131-libXISF/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.

Comment 2 Petr Pisar 2023-03-17 14:33:33 UTC
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?

Comment 3 Petr Pisar 2023-03-17 15:34:03 UTC
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.

Comment 4 Mattia Verga 2023-03-18 09:08:19 UTC
(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

Comment 5 Jakub Kadlčík 2023-03-18 09:17:27 UTC
Created attachment 1951611 [details]
The .spec file difference from Copr build 5633131 to 5676975

Comment 6 Jakub Kadlčík 2023-03-18 09:17:29 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5676975
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2177855-libXISF/fedora-rawhide-x86_64/05676975-libXISF/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.

Comment 7 Petr Pisar 2023-03-20 12:34:15 UTC
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.

Comment 8 Mattia Verga 2023-03-20 17:13:57 UTC
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?

Comment 9 Fedora Admin user for bugzilla script actions 2023-03-20 17:20:50 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/libXISF

Comment 10 Fedora Update System 2023-03-20 17:35:35 UTC
FEDORA-2023-9154501712 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-9154501712

Comment 11 Fedora Update System 2023-03-20 17:35:59 UTC
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.

Comment 12 Petr Pisar 2023-03-20 17:38:54 UTC
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.

Comment 13 Mattia Verga 2023-03-20 17:43:49 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.