https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Bug ID: 1936772 Summary: Review Request: sixel - Encoder and decoder for DEC Sixel Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: dank@qemfd.net QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: http://dank.qemfd.net/sixel.spec SRPM URL: http://dank.qemfd.net/sixel-1.8.6-1.fc35.src.rpm Description: Encoder and decoder for DEC Sixel terminal pixel graphics Fedora Account System Username: nickblack
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Aleksei Bavshin alebastr89@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |alebastr89@gmail.com Assignee|nobody@fedoraproject.org |alebastr89@gmail.com Doc Type|--- |If docs needed, set a value Flags| |fedora-review?
--- Comment #1 from Aleksei Bavshin alebastr89@gmail.com ---
Name: sixel
Upstream name is libsixel and all other distributions are unanimously using it[1]. Let's avoid renaming the package without a good reason.
Source0: https://github.com/saitoha/lib%%7Bname%7D/releases/download/v%%7Bversion%7D/...
You can shorten Source to %{url}/releases/download/v%{version}/%{name}-%{version}.tar.gz. But the release archive lacks license files[2] so you'd really want to use %{url}/archive/v%{version}/%{name}-%{version}.tar.gz
BuildRequires: git
Unnecessary. Noting in the build process requires git.
BuildRequires: gcc BuildRequires: pkgconfig(libjpeg) BuildRequires: pkgconfig(libpng)
Missing `BuildRequires: make` [3].
There's an optional dependency on libcurl, but given that there are at least 2 known CVEs[4][5] in the file loaders it's better to keep network support disabled.
%package devel Summary: Development files for %{name} Provides: %{name}%{?_isa} = %{?epoch:%{epoch}:}%{version}-%{release}
Uh... what? You are trying to tell that the main package with libraries is not necessary if -devel is installed. I believe you meant `Requires: %{name}%{?_isa} = %{version}-%{release}`. Rpmlint agrees with me: sixel-devel.x86_64: W: no-dependency-on sixel/sixel-libs/libsixel
%package utils Summary: Binaries from the libsixel project
How about `SIXEL decoder and encoder utilities`?
License: MIT
You don't need to repeat the license for subpackage if it doesn't differ from the main one.
%description utils Binaries from libsixel.
sixel-utils.x86_64: W: description-shorter-than-summary Let's make that at least `%{summary}.`
make %{?_smp_mflags}
%make_build
make install DESTDIR=$RPM_BUILD_ROOT
%make_install
Source archive contains unit tests. Consider running them at build time with %check %make_build test
%files %doc ChangeLog NEWS
Please, add license files. For example: %license LICENSE LICENSE.{pnmcolormap,sdump,sixel,stb}
Some of those are MIT and it's important to distribute these along with the binaries.
%{_mandir}/man5/*.5*
The man file could be more suitable for devel or utils subpackage. It's just a generic description of a SIXEL format.
%files utils # we don't want libsixel-config
We really want it in -devel package. Some applications may call libsixel-config instead of using pkg-config at build time.
%{_datadir}/bash-completion/* %{_datadir}/zsh/*
Please, be more explicit. Also, you need to own the zsh completion directories (bash-completion is already owned by filesystem):
%{_datadir}/bash-completion/completions/* %dir %{_datadir}/zsh %dir %{_datadir}/zsh/site-functions %{_datadir}/zsh/site-functions/_*
--- [1] https://repology.org/project/libsixel/versions [2] https://github.com/saitoha/libsixel/pull/129 [3] https://fedoraproject.org/wiki/Changes/Remove_make_from_BuildRoot [4] https://github.com/saitoha/libsixel/issues/134 (CVE-2020-11721) [5] https://github.com/saitoha/libsixel/issues/136 (CVE-2020-19668)
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Aleksei Bavshin alebastr89@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |182235 (FE-Legal)
--- Comment #2 from Aleksei Bavshin alebastr89@gmail.com --- Adding FE-Legal blocker.
https://github.com/saitoha/libsixel/blob/master/LICENSE.sixel looks benign and the source files in question mention that the code was relicensed under MIT[1][2]. I can't read the original license text though and not qualified to make decisions on the custom licenses, however permissive these are.
Several sources are also originally distributed under public domain. In my (limited) understanding that means it's fine to distribute the result under MIT and keep the License tag as simple `MIT`.
Ben, can you please look into that and confirm that it's safe to consider the combined work licensed as MIT? I hope we have someone who can read Japanese flavor of legalese :)
[1] https://github.com/saitoha/libsixel/blob/master/src/fromsixel.c [2] https://github.com/saitoha/libsixel/blob/master/src/tosixel.c
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Ben Cotton bcotton@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bcotton@redhat.com
--- Comment #3 from Ben Cotton bcotton@redhat.com --- Ack. I'll look into it.
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Ben Cotton bcotton@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|182235 (FE-Legal) |
--- Comment #4 from Ben Cotton bcotton@redhat.com --- The original license meets Fedora's standards and does not prohibit re-licensing. You may consider the combined work as MIT-licensed.
Removing the FE-LEGAL block.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
--- Comment #5 from Nick Black dank@qemfd.net --- Thanks Ben, and thanks for the review @alebastr89@gmail.com. I'll get on your feedback before the end of the week.
Product: Fedora Version: rawhide Component: Package Review
Nick Black dank@qemfd.net has canceled Package Review package-review@lists.fedoraproject.org's request for Aleksei Bavshin alebastr89@gmail.com's needinfo: Bug 1936772: Review Request: sixel - Encoder and decoder for DEC Sixel https://bugzilla.redhat.com/show_bug.cgi?id=1936772
--- Comment #7 from Nick Black dank@qemfd.net --- I don't see these problems as easily surmounted, and I'm no longer involved with libsixel. Closing!
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Nick Black dank@qemfd.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Flags|needinfo?(alebastr89@gmail. | |com) | Resolution|--- |WONTFIX Last Closed| |2022-03-19 08:48:03
--- Comment #7 from Nick Black dank@qemfd.net --- I don't see these problems as easily surmounted, and I'm no longer involved with libsixel. Closing!
https://bugzilla.redhat.com/show_bug.cgi?id=1936772
Petr Menšík pemensik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2227397
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2227397 [Bug 2227397] Review Request: libsixel - SIXEL encoding and decoding
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com