Hide Forgot
Spec URL: https://fedorapeople.org/~jpokorny/pkgs/slurp/slurp.spec SRPM URL: https://fedorapeople.org/~jpokorny/pkgs/slurp/slurp-0.0.1-1.20181024git0dbd039.fc30.src.rpm Fedora Account System Username: jpokorny Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=30634998 Note that this is mostly helpful in combination with grim [bug 1645764] and uses the very same style of packaging (reviewer would ideally tackle both at once).
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - The file protocol/wlr-layer-shell-unstable-v1.xml is licensed with "NTP (legal disclaimer)". It's not on the list of good licenses. You should check with Fedora Legal whether this is a good license. - Upstream files are not properly licensed, most files are missing license headers. This should at least be reported to upstream. ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [?]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "MIT/X11 (BSD like)", "NTP (legal disclaimer)", "Unknown or generated". 12 files have unknown license. Detailed output of licensecheck in /home/thofmann/fedora/reviews/review- slurp/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [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. [-]: 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. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: No rpmlint messages. [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]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [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 use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [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]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: 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]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [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]: 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: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: slurp-0.0.1-1.20181024git0dbd039.fc30.x86_64.rpm slurp-debuginfo-0.0.1-1.20181024git0dbd039.fc30.x86_64.rpm slurp-debugsource-0.0.1-1.20181024git0dbd039.fc30.x86_64.rpm slurp-0.0.1-1.20181024git0dbd039.fc30.src.rpm 4 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (debuginfo) ------------------- Checking: slurp-debuginfo-0.0.1-1.20181024git0dbd039.fc30.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory slurp.x86_64: W: invalid-url URL: https://github.com/emersion/slurp <urlopen error [Errno -2] Name or service not known> slurp-debugsource.x86_64: W: invalid-url URL: https://github.com/emersion/slurp <urlopen error [Errno -2] Name or service not known> slurp-debuginfo.x86_64: W: invalid-url URL: https://github.com/emersion/slurp <urlopen error [Errno -2] Name or service not known> 3 packages and 0 specfiles checked; 0 errors, 3 warnings. Requires -------- slurp (rpmlib, GLIBC filtered): libc.so.6()(64bit) libcairo.so.2()(64bit) libwayland-client.so.0()(64bit) libwayland-cursor.so.0()(64bit) rtld(GNU_HASH) slurp-debugsource (rpmlib, GLIBC filtered): slurp-debuginfo (rpmlib, GLIBC filtered): Provides -------- slurp: slurp slurp(x86-64) slurp-debugsource: slurp-debugsource slurp-debugsource(x86-64) slurp-debuginfo: debuginfo(build-id) slurp-debuginfo slurp-debuginfo(x86-64) Source checksums ---------------- https://github.com/emersion/slurp/archive/0dbd03991462397eb92bb40af712c837c898ebf1.tar.gz#/slurp-0.0.1-20181024git0dbd039.tar.gz : CHECKSUM(SHA256) this package : cf2cdc909ec9a479f6b8a895ce9cbcc587f2795f93be9b9fb8be8785fbecc47e CHECKSUM(SHA256) upstream package : cf2cdc909ec9a479f6b8a895ce9cbcc587f2795f93be9b9fb8be8785fbecc47e Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -u https://bugzilla.redhat.com/show_bug.cgi?id=1645765 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Spec URL: https://fedorapeople.org/~jpokorny/pkgs/slurp/slurp.spec SRPM URL: https://fedorapeople.org/~jpokorny/pkgs/slurp/slurp-0.0.1-2.20181024git0dbd039.fc30.src.rpm Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=30774201 - Clarify non-MIT license notice - Add -grim subpackage carrying smooth slurp-grim integration
Sorry for my blindness to "issues" section, was concentrating merely on the checkboxes since my brain was learnt that there's usually a lot of ballast coming from fedora-review (meaning it likely needs some love to catch up the evolution): > - The file protocol/wlr-layer-shell-unstable-v1.xml is licensed with > "NTP (legal disclaimer)". It's not on the list of good licenses. You > should check with Fedora Legal whether this is a good license. Indeed it is a good license, and that's just unfortunate than licensecheck provides a misleading verdict vs. how Fedora classifies the license -- this is the matching text: https://fedoraproject.org/wiki/Licensing:MIT#Old_Style_with_legal_disclaimer but it contains "and sell" addition to the very first sentences, but this matches with, e.g., https://fedoraproject.org/wiki/Licensing:MIT#Old_Style (see also [bug 1475962 comment 12]). Do you see any conflict here? I shall fix the wording in the spec to this very interpretation since otherwise I am carrying this cargo cult confusion (originating in wlroots since [bug 1529352 comment 5]), though. And you should likely do the same with sway, since the protocol file in question was borrowed there :-) > - Upstream files are not properly licensed, most files are missing > license headers. This should at least be reported to upstream. Cannot find anyting to that effect in https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines Can you point out the particular part to me that asks for this, please? (Again, sway is in the same boat here.) My TODO is to to get the slurp-grim.desktop more into shape (especially Categories).
(In reply to Jan Pokorný from comment #3) > Sorry for my blindness to "issues" section, was concentrating merely > on the checkboxes since my brain was learnt that there's usually a lot > of ballast coming from fedora-review (meaning it likely needs some > love to catch up the evolution): Funny, I always do it the other way around: focus on issues, post the rest for completeness' sake. > > > - The file protocol/wlr-layer-shell-unstable-v1.xml is licensed with > > "NTP (legal disclaimer)". It's not on the list of good licenses. You > > should check with Fedora Legal whether this is a good license. > > Indeed it is a good license, and that's just unfortunate than > licensecheck provides a misleading verdict vs. how Fedora classifies > the license -- this is the matching text: > > https://fedoraproject.org/wiki/Licensing:MIT#Old_Style_with_legal_disclaimer > > but it contains "and sell" addition to the very first sentences, but > this matches with, e.g., > > https://fedoraproject.org/wiki/Licensing:MIT#Old_Style > > (see also [bug 1475962 comment 12]). > > Do you see any conflict here? I shall fix the wording in the spec to > this very interpretation since otherwise I am carrying this cargo cult > confusion (originating in wlroots since [bug 1529352 comment 5]), > though. OK, that makes sense. No conflict here, it just wasn't clear to me. > > And you should likely do the same with sway, since the protocol file > in question was borrowed there :-) Yes! > > > - Upstream files are not properly licensed, most files are missing > > license headers. This should at least be reported to upstream. > > Cannot find anyting to that effect in > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/ > LicensingGuidelines > Can you point out the particular part to me that asks for this, please? It's not explicitly stated in the guidelines, but if the code is not properly licensed, then it cannot be shipped in Fedora. I think it's clear enough what upstream intends, but legally it's not 100% clear. Afaik, just putting a LICENSE file in your repository doesn't license all the files with that license. Since the upstream intention is clear, this is not a blocker, but it should definitely be pointed out to them so they can fix it, which should be in their interest. > > (Again, sway is in the same boat here.) Good point, I never checked that when I started maintaining sway (I did not create the initial package). > > My TODO is to to get the slurp-grim.desktop more into shape > (especially Categories).
Can we get the package wrapped up? I reconsidered my comment about the licensing, apparently this is handled differently for MIT-licensed projects, MIT doesn't have the requirement that the license is mentioned in every source file. So this is not a blocker. If you can update the package to the latest release, I can continue (or rather re-do) the review.
Jan, are you still interested in this package?
*** This bug has been marked as a duplicate of bug 1789980 ***