Bug 2222844 - Review Request: qoi - The “Quite OK Image Format” for fast, lossless image compression
Summary: Review Request: qoi - The “Quite OK Image Format” for fast, lossless image co...
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/phoboslab/qoi
Whiteboard:
: 2222846 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-07-14 05:44 UTC by Ryan
Modified: 2023-07-16 08:40 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)
The .spec file difference from Copr build 6171803 to 6172039 (1.30 KB, patch)
2023-07-14 08:59 UTC, Fedora Review Service
no flags Details | Diff

Description Ryan 2023-07-14 05:44:11 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/errornointernet/qoi/fedora-rawhide-x86_64/06171789-qoi/qoi.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/errornointernet/qoi/fedora-rawhide-x86_64/06171789-qoi/qoi-20230615git36190eb-1.fc39.src.rpm

Upstream: https://github.com/phoboslab/qoi
Description: Binaries (qoibench and qoiconv) for fast, lossless image compression using the "Quite OK Image Format".
Fedora Account System Username: errornointernet

Sorry for any mistakes made.
This is one of my first packages, and I am in need of a sponsor.

Comment 1 Fedora Review Service 2023-07-14 05:50:19 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6171803
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2222844-qoi/fedora-rawhide-x86_64/06171803-qoi/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 3 Ryan 2023-07-14 08:53:24 UTC
*** Bug 2222846 has been marked as a duplicate of this bug. ***

Comment 4 Fedora Review Service 2023-07-14 08:59:04 UTC
Created attachment 1975737 [details]
The .spec file difference from Copr build 6171803 to 6172039

Comment 5 Fedora Review Service 2023-07-14 08:59:06 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6172039
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2222844-qoi/fedora-rawhide-x86_64/06172039-qoi/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 6 Artur Frenszek-Iwicki 2023-07-15 10:29:02 UTC
> %install
> mkdir -p %{buildroot}/%{_includedir}
> cp qoi.h %{buildroot}/%{_includedir}
1. You should aim to preserve file timestamps. You can use "cp -a" or "cp --preserve=timestamps".
   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_timestamps
2. Using "cp" is fine, but I think that most .spec files use "install" instead.
   "install -d" can also be used to create directories.

> %files devel
> %license LICENSE
> %doc README.md
The -devel package has a hard "Requires:" on the base package,
so these files can be omitted.
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#subpackage-licensing

Also, looking at the copr build made by the review service:
> gcc -std=c99 -O3 -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64   -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer  qoiconv.c -o qoiconv
This does not include Fedora's LDFLAGS. You'll need to either modify the Makefile to include those in the gcc invocation, or call the compiler manually.

Comment 7 Ryan 2023-07-16 08:40:14 UTC
Thanks for your reply :)

> This does not include Fedora's LDFLAGS. You'll need to either modify the Makefile to include those in the gcc invocation, or call the compiler manually.
I have now added a patch (Makefile-ldflags.patch) for the Makefile to include LDFLAGS.

Changes:
- Used `install -d` and `install -p` instead of `cp`
- Added `Requires: %{name}%{?isa} = %{version}-%{release}` to the -devel package
- Added LDFLAGS patch

New .spec URL: https://download.copr.fedorainfracloud.org/results/errornointernet/qoi/fedora-rawhide-x86_64/06176114-qoi/qoi.spec
New .srpm URL: https://download.copr.fedorainfracloud.org/results/errornointernet/qoi/fedora-rawhide-x86_64/06176114-qoi/qoi-20230615git36190eb-3.fc39.src.rpm
Makefile-ldflags.patch: https://github.com/ErrorNoInternet/rpm-specs/raw/main/qoi/Makefile-ldflags.patch


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