Bug 1980282
| Summary: | Review Request: exaile - music player | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Graham White <graham_alton> |
| Component: | Package Review | Assignee: | Robert-André Mauchin 🐧 <eclipseo> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | code, eclipseo, package-review |
| Target Milestone: | --- | Flags: | eclipseo:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2021-07-22 01:13:44 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Graham White
2021-07-08 09:04:17 UTC
You’re on the right track. Here are the official instructions for what you’re trying to do: https://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers#Claiming_Ownership_of_a_Retired_Package Fantastic, thanks Ben. I'll give that a whirl but leave this bug open while I'm attempting to claim ownership. This review is part of the process anyway… I can’t do the review right now but might take a look at it later if I have some time. OK, so looking around on the devel list, Exaile was orphaned/retired because of Python 2 as I had originally suspected [1]. There's also a bug report for having it obsoleted [2]. I've moved over to following the Claiming Ownership of a Retired Package procedure and have mail the devel list [3]. [1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/SYBEPMFAEZCCHQE6SM2SDXXUV6S73HGI/ [2] https://bugzilla.redhat.com/show_bug.cgi?id=1810253 [3] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/5RV2LRDPW4Y23VG74LQSDBVWEOEUZC7D/ - You need to set Fedora default build flags: %build %set_build_flags %make_build - You could try to keep timestamps by changing install -m to $(INSTALL) -m - and also use the auto rpm script "brp-python-bytecompile" to compile all the pyc files by disabling compile in the Makefile # Keep timestamps while installing # Delegate pyc compilation to brp-python-bytecompile sed -i "s|install -m|\$(INSTALL) -m|;s|all: compile |all: |" Makefile - Why no tests? Steps because it's tricky: Source0: https://github.com/exaile/exaile/archive/%{version}/%{name}-%{version}.tar.gz # Fix for bug https://github.com/exaile/exaile/issues/750 Patch0: https://github.com/exaile/exaile/commit/d8bbcfd174b658babb6605799d1e9e788b578c84.patch […] BuildRequires: cairo-gobject BuildRequires: desktop-file-utils BuildRequires: gettext BuildRequires: gobject-introspection BuildRequires: gstreamer1-plugins-base >= 1.14 BuildRequires: gstreamer1-plugins-good >= 1.14 BuildRequires: gtk3 >= 3.22 BuildRequires: help2man BuildRequires: libappstream-glib BuildRequires: python3-bsddb3 BuildRequires: python3-cairo BuildRequires: python3-dbus BuildRequires: python3-devel BuildRequires: python3-gobject-devel >= 3.22 BuildRequires: python3-gstreamer1 >= 1.14 BuildRequires: python3-mox3 BuildRequires: python3-mutagen >= 1.38 BuildRequires: python3-pytest BuildRequires: python3-setproctitle […] %prep %autosetup -p1 […] %check appstream-util validate-relax --nonet %{buildroot}%{_datadir}/appdata/*.appdata.xml make test - Remove the shebang from these files: exaile.noarch: E: non-executable-script /usr/share/exaile/plugins/bpm/bpmdetect.py 644 /usr/bin/env python3 exaile.noarch: E: non-executable-script /usr/share/exaile/plugins/ipconsole/ipython_view.py 644 /usr/bin/env python3 See https://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries - Patch the old addresses with the new address and send the patch upstream: exaile.noarch: E: incorrect-fsf-address /usr/share/exaile/plugins/lyricsmania/__init__.py exaile.noarch: E: incorrect-fsf-address /usr/share/exaile/plugins/somafm/__init__.py Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review neede ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GNU General Public License, Version 2", "Unknown or generated", "GNU General Public License v2.0 or later", "GNU General Public License v1.0 or later", "*No copyright* GNU General Public License v2.0 or later", "GNU General Public License v3.0 or later", "*No copyright* GNU General Public License v1.0 or later", "BSD 3-clause "New" or "Revised" License", "*No copyright* GNU General Public License v3.0 or later", "GNU General Public License v2.0 or later [obsolete FSF postal address (Mass Ave)]", "*No copyright* GNU General Public License", "GNU Lesser General Public License v2.1 or later". 407 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/exaile/review- exaile/licensecheck.txt [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: The spec file handles locales properly. [!]: 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]: 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: There are rpmlint messages (see attachment). [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [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]: %config files are marked noreplace or the reason is justified. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [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]: No %config files under /usr. [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). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: 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. [!]: %check is present and all tests pass. [!]: 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 all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: exaile-4.1.1-1.fc35.noarch.rpm exaile-4.1.1-1.fc35.src.rpm exaile.noarch: W: spelling-error %description -l en_US playlists -> play lists, play-lists, stylists exaile.noarch: W: spelling-error %description -l en_US fm -> FM, Fm, gm exaile.noarch: W: spelling-error %description -l en_US scrobbling -> scribbling, scrabbling, scrolling exaile.noarch: E: non-executable-script /usr/share/exaile/plugins/bpm/bpmdetect.py 644 /usr/bin/env python3 exaile.noarch: E: non-executable-script /usr/share/exaile/plugins/ipconsole/ipython_view.py 644 /usr/bin/env python3 exaile.noarch: E: incorrect-fsf-address /usr/share/exaile/plugins/lyricsmania/__init__.py exaile.noarch: E: incorrect-fsf-address /usr/share/exaile/plugins/somafm/__init__.py exaile.src: W: spelling-error %description -l en_US playlists -> play lists, play-lists, stylists exaile.src: W: spelling-error %description -l en_US fm -> FM, Fm, gm exaile.src: W: spelling-error %description -l en_US scrobbling -> scribbling, scrabbling, scrolling exaile.src: W: spelling-error %description -l en_US podcasts -> podcast, pod casts, pod-casts exaile.src: W: spelling-error %description -l en_US icecast -> ice cast, ice-cast, icecap 2 packages and 0 specfiles checked; 4 errors, 8 warnings. Thanks for taking the time to do the review and all your extra detail, that was all extremely helpful. I have updated the spec file and built a new set of RPMS, all uploaded to https://grahamwhiteuk.fedorapeople.org/pkgreviews/exaile/ I have also submitted 3 patches upstream: 1) https://github.com/exaile/exaile/pull/759 - Preserve timestamp upon file installation. 2) https://github.com/exaile/exaile/pull/760 - Use the correct address for the FSF 3) https://github.com/exaile/exaile/pull/761 - Remove shebangs from plugins installed without execute perms Changes made are: * Set Fedora default build flags - added %set_build_flags to the %build section * Keep timestamps and defer byte compilation to brp-python-bytecompile - spec file modifies the Makefile to add -p flag to install command (this aspect will be remove if PR 759 is accepted upstream) - spec file modifies the Makefile to remove the compilation step * Re-enabled tests, I've no idea why this was disabled by the previous packagers - patch d8bbcfd174b658babb6605799d1e9e788b578c84 incorporated to facilitate successful tests until next upstream release (where the patch has been merged upstream) * Updated BuildRequires - as indicated in the above package review and to include additional BRs to allow the tests to execute * Correct rpmlint issues - patch submitted upstream to use the correct FSF address - patch submitted upstream to remove unnecessary shebangs @zebob.m - I wonder if you would be kind enough to double check with an updated review? Thanks again. LGTM, package approved. Following the process [1], the next step is a releng ticket [2]. [1] https://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers#Claiming_Ownership_of_a_Retired_Package [2] https://pagure.io/releng/issue/10206 FEDORA-2021-f569edcb75 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-f569edcb75 FEDORA-2021-230656804f has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-230656804f FEDORA-2021-f569edcb75 has been pushed to the Fedora 34 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-f569edcb75 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-f569edcb75 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates. FEDORA-2021-230656804f has been pushed to the Fedora 33 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-230656804f \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-230656804f See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates. FEDORA-2021-f569edcb75 has been pushed to the Fedora 34 stable repository. If problem still persists, please make note of it in this bug report. FEDORA-2021-230656804f has been pushed to the Fedora 33 stable repository. If problem still persists, please make note of it in this bug report. |