Bug 1671571 - Review Request: playerctl - MPRIS command-line controller and library for spotify, vlc, audacious, bmp, cmus, and others
Summary: Review Request: playerctl - MPRIS command-line controller and library for spo...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
low
Target Milestone: ---
Assignee: Dridi Boukelmoune
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-01-31 23:01 UTC by Justin W. Flory (Fedora)
Modified: 2019-04-27 21:24 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-04-21 00:40:55 UTC
Type: Bug
Embargoed:
dridi.boukelmoune: fedora-review+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1266594 0 medium CLOSED Review Request: playerctl - Command-line MPRIS-compatible Media Player Controller 2021-02-22 00:41:40 UTC

Description Justin W. Flory (Fedora) 2019-01-31 23:01:21 UTC
This is my third package and I am seeking a sponsor.

* Spec URL: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SPECS/playerctl.spec
* SRPM URL: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-1.fc29.src.rpm
* Description: Playerctl is a command-line utility and library for controlling media players that implement the MPRIS D-Bus Interface Specification. Playerctl makes it easy to bind player actions, such as play and pause, to media keys. You can also get metadata about the playing track such as the artist and title for integration into statusline generators or other command-line tools. (https://github.com/acrisci/playerctl).
* Fedora Account System Username: jflory7

This package is also hosted in COPR and was built in Koji on F28, F29, and Rawhide.

* https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/
* F30: https://koji.fedoraproject.org/koji/taskinfo?taskID=32392575
* F29: https://koji.fedoraproject.org/koji/taskinfo?taskID=32392582
* F28: https://koji.fedoraproject.org/koji/taskinfo?taskID=32392592

Comment 1 Dridi Boukelmoune 2019-02-03 12:19:18 UTC
I can't sponsor you, but I will do the formal review once you have a sponsor. And as per bug 1266594 I would like to co-maintain this package since I'm currently using my own build of the package and have a vetted interest here.

Before the formal review, I have a few nits after a quick read of the spec:

For the summary, please stick to "Command-line MPRIS-compatible Media Player Controller" instead of mentioning services like spotify or software like VLC that aren't fit for inclusion in Fedora.

You shouldn't need to "Requires: glib", I didn't for my local build.

For the Source URL, please use upstream's official release archive instead of deriving one from the github API:

    https://github.com/acrisci/playerctl/releases/download/v2.0.1/playerctl-2.0.1.tar.xz

They also provide a SHA-256 checksum that you should use before you upload the archive to the look-aside cache.

The license should be LGPLv3+ because upstream clearly uses "or (at your option) any later version" in its source files. I didn't check whether other licenses apply.

Comment 2 Dridi Boukelmoune 2019-02-03 12:28:42 UTC
Also don't bother with %epoch unless one day upstream breaks the upgrade path.

See the guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package

Comment 3 Justin W. Flory (Fedora) 2019-02-03 18:24:14 UTC
Hi Dridi, thanks for the feedback. I pushed a new commit [1] to my RPM spec based on your feedback.

(In reply to Dridi Boukelmoune from comment #1)
> I can't sponsor you, but I will do the formal review once you have a
> sponsor. And as per bug 1266594 I would like to co-maintain this package
> since I'm currently using my own build of the package and have a vetted
> interest here.
> 

I'm happy to co-maintain the package with you!

> For the Source URL, please use upstream's official release archive instead
> of deriving one from the github API:
> 
>    
> https://github.com/acrisci/playerctl/releases/download/v2.0.1/playerctl-2.0.
> 1.tar.xz
> 

I wasn't able to do this. Upstream's archives are xz archives and Koji/COPR doesn't recognize the xz compression format, so the build fails [2]. I'm using the tar.gz archive via the GitHub API similar to other RPM packages available in Fedora.


--
[1]  https://pagure.io/jflory7-rpm-specs/c/370f06098ee6adc2da025c02116b572e3dc949c6?branch=master
[2]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853607/

Comment 4 Dridi Boukelmoune 2019-02-04 08:45:30 UTC
More changes for playerctl:


    --- playerctl.spec	2019-02-04 09:21:17.102143219 +0100
    +++ playerctl.spec	2019-02-04 09:24:00.415760142 +0100
    @@ -5,7 +5,7 @@
     
     License:        LGPLv3+
     URL:            https://github.com/acrisci/playerctl
    -Source0:        %{url}/archive/v%{version}/%{name}-%{version}.tar.gz
    +Source:         %{url}/releases/download/v%{version}/%{name}-%{version}.tar.xz
     
     BuildRequires:  gcc
     BuildRequires:  meson
    @@ -62,13 +62,15 @@
     %{_bindir}/%{name}
     %{_datadir}/gir-1.0/Playerctl-2.0.gir
     %{_libdir}/girepository-1.0/Playerctl-2.0.typelib
    -%{_libdir}/lib%{name}.*
    -%doc %{_datadir}/gtk-doc/html/%{name}/*
    -%doc %{_datadir}/man/man1/%{name}.*
    +%{_libdir}/lib%{name}.so.2*
    +%{_datadir}/gtk-doc/html/%{name}
    +%{_datadir}/man/man1/%{name}.*
     
     
     %files devel
     %{_includedir}/%{name}/*.h
    +%{_libdir}/lib%{name}.a
    +%{_libdir}/lib%{name}.so
     %{_libdir}/pkgconfig/%{name}.pc

The xz archive is handled just fine, and you should file a bug for COPR if it's not working there. We are targeting Fedora proper here so I'd rather stick to the upstream release archive.

I also moved the static library and un-versioned shared object to the devel packages, and added the full soname in the main package so we don't miss soname bumps in the future. I built this with mock locally and I'm currently using the package on my system with no regression.

Finally, I removed the %doc indicators because those are well-known locations in Fedora so they already get flagged as such, and I made sure the %{_datadir}/gtk-doc/html/%{name} is owned by the package, not just its contents.

When you update the spec, make sure to add a new comment with corresponding Spec and SRPM URLs so that during the formal review fedora-review picks up your latest update (especially since you updated the release tag, that changes the SRPM URL).

Good luck with your sponsorship, I will patiently wait :)

Comment 5 Fabio Valentini 2019-02-04 11:42:42 UTC
Just some small comments from a Packaging Committee member:

- Please don't include static libraries, unless *absolutely necessary*.
In this case, I think it can be safely removed (probably by rm-ing it from the buildroot at the end of %install):
  rm %{buildroot}/%{_libdir}/lib%{name}.a

See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries

- "gobject-introspection-devel" could be replaced by "pkgconfig(gobject-introspection-1.0)"

Mainly for consistency, since you're already using pkgconfig(foo) BuildRequires for other things.

- Depending on the size of the html docs, they should be moved to a separate, (probably noarch) -doc sub-package.

For example, fedora-review recommends to move documentation to a separate sub-package if it's bigger than 1 MB.

Comment 6 Justin W. Flory (Fedora) 2019-02-04 12:59:17 UTC
I pushed a new commit [1] to address the above feedback. The build completed successfully in Koji [2][3] and COPR [4].

(In reply to Dridi Boukelmoune from comment #4)
> The xz archive is handled just fine, and you should file a bug for COPR if
> it's not working there. We are targeting Fedora proper here so I'd rather
> stick to the upstream release archive.
> 

Odd. Not sure why the build failed before because xz was an unrecognized format. I made this change again and the package built successfully. Maybe the archive was corrupt originally. In either case, this is done now.

> I also moved the static library and un-versioned shared object to the devel
> packages, and added the full soname in the main package so we don't miss
> soname bumps in the future. I built this with mock locally and I'm currently
> using the package on my system with no regression.
> 
> Finally, I removed the %doc indicators because those are well-known
> locations in Fedora so they already get flagged as such, and I made sure the
> %{_datadir}/gtk-doc/html/%{name} is owned by the package, not just its
> contents.
> 

Thanks for these tips. About the static library / shared objects, could you link me to documentation or explain why this change is helpful? I'm new to building C packages. Understanding why this change is helpful will help me build better packages later too.

> When you update the spec, make sure to add a new comment with corresponding
> Spec and SRPM URLs so that during the formal review fedora-review picks up
> your latest update (especially since you updated the release tag, that
> changes the SRPM URL).
>

Spec: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SPECS/playerctl.spec
SRPM: https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-3.fc29.src.rpm


(In reply to Fabio Valentini from comment #5)
> Just some small comments from a Packaging Committee member:
> 
> - Please don't include static libraries, unless *absolutely necessary*.
> In this case, I think it can be safely removed (probably by rm-ing it from
> the buildroot at the end of %install):
>   rm %{buildroot}/%{_libdir}/lib%{name}.a
> 

It seems the static library is necessary? My Koji build [5] and COPR build [6] failed without it.

> - Depending on the size of the html docs, they should be moved to a
> separate, (probably noarch) -doc sub-package.
> 
> For example, fedora-review recommends to move documentation to a separate
> sub-package if it's bigger than 1 MB.

The HTML docs were only 500KB or so, but I went ahead and split them out into their own package anyways.

--
[1]  https://pagure.io/jflory7-rpm-specs/c/476d05cbafe8913a8d0af4c29bb439a3d1264af5?branch=master
[2]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32534256
[3]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32534242
[4]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853863/
[5]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32533723
[6]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853848/

Comment 7 Fabio Valentini 2019-02-04 13:43:51 UTC
> (In reply to Fabio Valentini from comment #5)
> > Just some small comments from a Packaging Committee member:
> > 
> > - Please don't include static libraries, unless *absolutely necessary*.
> > In this case, I think it can be safely removed (probably by rm-ing it from
> > the buildroot at the end of %install):
> >   rm %{buildroot}/%{_libdir}/lib%{name}.a
> > 
> 
> It seems the static library is necessary? My Koji build [5] and COPR build
> [6] failed without it.

It's not. You just forgot to remove the corresponding entry from the %files section - and now it can't find the file that you removed - that's all.

> > - Depending on the size of the html docs, they should be moved to a
> > separate, (probably noarch) -doc sub-package.
> > 
> > For example, fedora-review recommends to move documentation to a separate
> > sub-package if it's bigger than 1 MB.
> 
> The HTML docs were only 500KB or so, but I went ahead and split them out
> into their own package anyways.

Nice, thanks!

> --
> [1] 
> https://pagure.io/jflory7-rpm-specs/c/
> 476d05cbafe8913a8d0af4c29bb439a3d1264af5?branch=master
> [2]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32534256
> [3]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32534242
> [4]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853863/
> [5]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32533723
> [6]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/853848/

Comment 8 Dridi Boukelmoune 2019-02-04 18:35:14 UTC
Justin, a static library is not code you run, but code you (statically) link to as part of a build process, that's why I moved it to the -devel sub-package. It could have also lived in a -static sub-packages too, you can find such occurrences in the repositories.

Fabio, I know that our guidelines strongly advise against static linking (and bundling in general) but I find it sad to prevent end-users to statically build software on top of Fedora if that's what they want and upstream provides the means. But if that's our guidelines, I can live without the static library and I personally don't care since I integrate %{_bindir}/playerctl in my local setup.

Thanks for your help with the review, I'll wait for an update until the sponsorship is solved to formally review the package.

Comment 9 Justin W. Flory (Fedora) 2019-02-05 04:11:19 UTC
(In reply to Fabio Valentini from comment #7)
> It's not. You just forgot to remove the corresponding entry from the %files
> section - and now it can't find the file that you removed - that's all.

(In reply to Dridi Boukelmoune from comment #8)
> Fabio, I know that our guidelines strongly advise against static linking
> (and bundling in general) but I find it sad to prevent end-users to
> statically build software on top of Fedora if that's what they want and
> upstream provides the means. But if that's our guidelines, I can live
> without the static library and I personally don't care since I integrate
> %{_bindir}/playerctl in my local setup.

I took the middle-ground approach and built a separate, optional package with the static libraries. The updated commit [1] passed successfully Koji [2][3] and COPR [4]. An updated SRPM is also available [5].


(In reply to Dridi Boukelmoune from comment #8)
> Justin, a static library is not code you run, but code you (statically) link
> to as part of a build process, that's why I moved it to the -devel
> sub-package. It could have also lived in a -static sub-packages too, you can
> find such occurrences in the repositories.

Thanks for the explanation.


--
[1]  https://pagure.io/jflory7-rpm-specs/c/ec957e11ed6307b08e5031f5a35451a4761cfb3c?branch=master
[2]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32545025
[3]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32545018
[4]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/854366/
[5]  https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-4.fc29.src.rpm

Comment 10 Neal Gompa 2019-02-06 14:54:48 UTC
(In reply to Justin W. Flory from comment #9)
> (In reply to Fabio Valentini from comment #7)
> > It's not. You just forgot to remove the corresponding entry from the %files
> > section - and now it can't find the file that you removed - that's all.
> 
> (In reply to Dridi Boukelmoune from comment #8)
> > Fabio, I know that our guidelines strongly advise against static linking
> > (and bundling in general) but I find it sad to prevent end-users to
> > statically build software on top of Fedora if that's what they want and
> > upstream provides the means. But if that's our guidelines, I can live
> > without the static library and I personally don't care since I integrate
> > %{_bindir}/playerctl in my local setup.
> 
> I took the middle-ground approach and built a separate, optional package
> with the static libraries. The updated commit [1] passed successfully Koji
> [2][3] and COPR [4]. An updated SRPM is also available [5].
> 

This is obviously allowed, but in Fedora, we just use "-static" instead of "-devel-static". The latter is openSUSE's (and long-ago Fedora) naming for static libraries...

Comment 11 Neal Gompa 2019-02-06 14:55:29 UTC
Also, to point out: static libraries do not require the main package, they require the devel package to be useful.

Comment 12 Justin W. Flory (Fedora) 2019-02-06 23:44:56 UTC
(In reply to Neal Gompa from comment #11)
> Also, to point out: static libraries do not require the main package, they
> require the devel package to be useful.

Thanks Neal. I pushed a new commit [1] with these changes. An SRPM is available [2]. It passed in Koji [3][4] and COPR [5].


---
[1]  https://pagure.io/jflory7-rpm-specs/c/18b94a485fc7a9de47d645c465d739014b24a1e0?branch=master
[2]  https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-5.fc29.src.rpm
[3]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32590171
[4]  https://koji.fedoraproject.org/koji/taskinfo?taskID=32590164
[5]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/855693/

Comment 13 Neal Gompa 2019-02-07 04:07:47 UTC
> %{_datadir}/gir-1.0/Playerctl-2.0.gir

This belongs in the devel subpackage, since this is used by gobject-introspection to generate bindings.

> %{_libdir}/girepository-1.0/Playerctl-2.0.typelib
> %{_libdir}/lib%{name}.so.2*

These two belong in a libs subpackage, and the main binary package should require the libs subpackage.

The reason to do this is to allow the libraries to be multi-libbed in repos for 64-bit arches with their matching 32-bit arches (such as x86_64 and i686).

Comment 15 Justin W. Flory (Fedora) 2019-02-17 16:19:57 UTC
I misunderstood the packaging guidelines. I do not need a sponsor since I already submitted a package to Fedora. If there is no additional feedback, could someone set the fedora-review flag?

Comment 16 Justin W. Flory (Fedora) 2019-02-21 02:10:08 UTC
(In reply to Dridi Boukelmoune from comment #8)
> Thanks for your help with the review, I'll wait for an update until the
> sponsorship is solved to formally review the package.

Hi Dridi, are you able to complete this package review? I incorrectly assumed I needed sponsorship. I did not realize sponsorship is granted after submitting a first package. This is my second package.

Comment 17 Dominik 'Rathann' Mierzejewski 2019-02-21 08:08:59 UTC
Is there a particular reason why you're building and including static libraries? Fedora defaults to shared ones only.

Comment 18 Dridi Boukelmoune 2019-02-24 11:30:30 UTC
Dominik, as I said, even though we package things dynamically, I don't like the idea of removing the possibility from end-users. But I can live without it so really, I don't care.

Justin, the link to the spec file should link to a spec file, not an HTML document. For your next update, please use this URL for the spec:

    https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec

Now there are a few things I'm not too happy with in this spec at a single glance:

The main package should not need to require the -libs sub-package, it should be detected automatically via linker information in binaries.

If you reuse the summary for the description, at least put a dot at the end:

    %description foo
    %{summary}.

Install the %license file in all packages.

After that I'll be happy to move on to a formal review, but my availability will be limited for a couple weeks.

Comment 19 Neal Gompa 2019-02-24 12:43:40 UTC
(In reply to Dridi Boukelmoune from comment #18)
> 
> Now there are a few things I'm not too happy with in this spec at a single
> glance:
> 
> The main package should not need to require the -libs sub-package, it should
> be detected automatically via linker information in binaries.
> 

This is actually a matter of policy for interdependencies like this. This prevents weird things from happening because often private symbols are used from -libs subpackage from other parts of the same source package. That, of course, is not true for external consumers.

> 
> After that I'll be happy to move on to a formal review, but my availability
> will be limited for a couple weeks.

I can take over the review if you want?

Comment 20 Justin W. Flory (Fedora) 2019-03-03 23:33:16 UTC
Hi, I pushed a new commit [1] with this spec [2] and this SRPM [3] to address remaining feedback. Is there anything left to address in the review?


---
[1]  https://pagure.io/jflory7-rpm-specs/c/de0e33bd1c8a992633e79b93aada740f33fdf68c?branch=master
[2]  https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
[3]  https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS

Comment 21 Justin W. Flory (Fedora) 2019-03-12 16:10:35 UTC
Hi, any updates? I am hoping to write a Fedora Magazine article this month about playerctl and another package recently made available in Fedora, so it will help to move this review along.

Comment 22 Dridi Boukelmoune 2019-03-12 21:06:54 UTC
Hi,

I actually tried to run fedora-review (and its python 3 port to compare) this weekend after being unavailable for more than a week. Unfortunately in your last update you didn't fully follow the review process [1] for that last update:

> After you have addressed them, please post the URLs to the updated SPEC and
> SRPM file and remove it from the Whiteboard.

This is even more unfortunate since for previous updates you did post both SPEC and SRPM URLs.

Please do that, and I will pick it up. I will try to be as responsive as possible but this is a volunteer job for me, and I'm also volunteering to try out the python 3 port of fedora-review so it may delay this review a tiny bit more on my end.

[1] https://fedoraproject.org/wiki/Package_Review_Process

Comment 23 Justin W. Flory (Fedora) 2019-03-13 16:16:12 UTC
(In reply to Dridi Boukelmoune from comment #22)
> Please do that, and I will pick it up. I will try to be as responsive as
> possible but this is a volunteer job for me, and I'm also volunteering to
> try out the python 3 port of fedora-review so it may delay this review a
> tiny bit more on my end.
> 

Hi Dridi, the links are here:

https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
https://pagure.io/jflory7-rpm-specs/blob/master/f/rpmbuild/SRPMS/playerctl-2.0.1-7.fc29.src.rpm

If you do not have time for this package review, Neal mentioned he could finish out the package review for you.

Comment 24 Dridi Boukelmoune 2019-03-13 17:47:41 UTC
Unfortunately your URL for the SRPM is still leading to a web page that does the redirection and fedora-review doesn't like that...

Now I'm curious to see whether it will pick up the correct URLs if it's me that posts them:

https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/playerctl-2.0.1-7.fc29.src.rpm

I'll do my best, no need to bother Neal, he already does so many reviews (but thanks to him anyway for stepping up) and he can still keep an eye on this review.

Comment 25 Fabio Valentini 2019-03-15 09:46:45 UTC
I think for fedora-review to pick up the links correctly, you need to adhere to this format (as given in the fedora-review bugzilla template).

SPEC URL: ...
SRPM URL: ...

Comment 26 Dridi Boukelmoune 2019-03-15 11:18:03 UTC
Yes and no. The guidelines ask to do this in the review request description, and then they only say to post a comment with the new URLs if you update the spec as per the review.

I also confirmed that it doesn't have to be the submitter that posts those URLs and was able to run fedora-review and its Python 3 port, but I'm not done going through the report.

Comment 27 Neal Gompa 2019-03-15 12:53:25 UTC
> %package devel
> Summary:        Development libraries and header files for %{name}
> Requires:       %{name}%{?_isa} = %{version}-%{release}
> 
> 
> %package docs
> Summary:        Documentation related to %{name}
> BuildArch:      noarch
> Requires:       %{name}%{?_isa} = %{version}-%{release}
> 
> 
> %package libs
> Summary:        Libraries and shared code for %{name}
> Requires:       %{name}%{?_isa} = %{version}-%{release}
> 
> 
> %package static
> Summary:        Static libraries for %{name} development
> Requires:       %{name}-devel%{?_isa} = %{version}-%{release}
> 
> 
> %description devel
> %{summary}.
> 
> 
> %description docs
> %{summary}.
> 
> 
> %description libs
> %{summary}.
> 
> 
> %description static
> %{summary}.

This layout is actually broken. It's not obvious, but the %summary variable is populated by the _last_ definition of the 'Summary:' field at the time the %description section was evaluated.

So, this leads to quirks like this:
> $ rpm -qip /var/lib/mock/fedora-29-x86_64/result/playerctl-devel-2.0.1-7.fc29.x86_64.rpm
> Name        : playerctl-devel
> Version     : 2.0.1
> Release     : 7.fc29
> Architecture: x86_64
> Install Date: (not installed)
> Group       : Unspecified
> Size        : 68295
> License     : LGPLv3+
> Signature   : (none)
> Source RPM  : playerctl-2.0.1-7.fc29.src.rpm
> Build Date  : Fri 15 Mar 2019 08:24:37 AM EDT
> Build Host  : localhost
> Relocations : (not relocatable)
> URL         : https://github.com/acrisci/playerctl
> Summary     : Development libraries and header files for playerctl
> Description :
> Static libraries for playerctl development.

Make sure the %description section is directly underneath each %package section, unless you want to write the descriptions individually.

Comment 28 Justin W. Flory (Fedora) 2019-03-19 16:14:22 UTC
I updated the RPM SPEC from the above feedback and this is ready for review again.

SPEC URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
SRPM URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/playerctl-2.0.1-8.fc29.src.rpm

Comment 29 Dridi Boukelmoune 2019-03-23 23:16:49 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[!]: Package contains no static executables.

You need an exception to ship the static library. Either get one or remove
the static library in the first place.

(Yes, I remember my previous comments on the topic=)

[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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: "Unknown or generated", "GNU Lesser General Public License (v3
     or later)". 21 files have unknown license. Detailed output of
     licensecheck in
     /home/dridi/fedora/FedoraReview/1671571-playerctl/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/include/playerctl, /usr/share/gtk-
     doc/html/playerctl
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/gir-1.0,
     /usr/share/gtk-doc/html, /usr/lib64/girepository-1.0,
     /usr/include/playerctl, /usr/share/gtk-doc, /usr/share/gtk-
     doc/html/playerctl

Use the following patterns instead:

%{_includedir}/%{name}/
%{_datadir}/gtk-doc/

Add the following to the devel sub-package:

equires:  pkgconfig(gobject-introspection-1.0)

[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.
[x]: 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.

The main package may conflict with itself on a multilib system and this is
fine.

[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary.

The noarch docs package depends on the archful main package, which is
debatable in the first place. At the very least, which variant of the main
package should not be dictated by the build system's architecture.

[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.
[ ]: 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).

playerctl-devel.x86_64: W: no-documentation

The devel sub-package could "Recommends:" the docs sub-package.

On the other hand the documentation is quite small and could live in
the devel package directly.

The perl locale warnings needs more investigation. We either need to set
the environment straight or bring the en-us langpack.

[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 does not own files or directories owned by other packages.

It should however own a couple already mentioned above.

[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 must not depend on deprecated() packages.
[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]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: playerctl-static.

As noted above, an exception is needed to ship the static sub-package.

[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.

Not quite true, but the docs sub-package (note: not "doc") is small enough
to be folded

[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.
[-]: Final provides and requires are sane (see attachments).

See the note above regarding the noarch devel requiring an archful main
package.

[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     playerctl , playerctl-static , playerctl-debuginfo , playerctl-
     debugsource

False-positives.

[x]: 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.

https://koji.fedoraproject.org/koji/taskinfo?taskID=33719966

[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.

Handled by meson.

[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]: The placement of pkgconfig(.pc) files are correct.
[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: playerctl-2.0.1-8.fc31.x86_64.rpm
          playerctl-devel-2.0.1-8.fc31.x86_64.rpm
          playerctl-docs-2.0.1-8.fc31.noarch.rpm
          playerctl-libs-2.0.1-8.fc31.x86_64.rpm
          playerctl-static-2.0.1-8.fc31.x86_64.rpm
          playerctl-debuginfo-2.0.1-8.fc31.x86_64.rpm
          playerctl-debugsource-2.0.1-8.fc31.x86_64.rpm
          playerctl-2.0.1-8.fc31.src.rpm
playerctl.x86_64: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.x86_64: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.x86_64: W: spelling-error %description -l en_US spotify -> spottily
playerctl.x86_64: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.x86_64: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.x86_64: W: spelling-error %description -l en_US cmus -> cums, mus, emus
playerctl-devel.x86_64: W: no-documentation
playerctl-libs.x86_64: W: no-documentation
playerctl-static.x86_64: W: no-documentation
playerctl.src: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.src: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.src: W: spelling-error %description -l en_US spotify -> spottily
playerctl.src: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.src: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.src: W: spelling-error %description -l en_US cmus -> cums, mus, emus
8 packages and 0 specfiles checked; 0 errors, 15 warnings.




Rpmlint (debuginfo)
-------------------
Checking: playerctl-debuginfo-2.0.1-8.fc31.x86_64.rpm
          playerctl-libs-debuginfo-2.0.1-8.fc31.x86_64.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.utf8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.utf8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.utf8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
sh: /usr/bin/python: No such file or directory
playerctl-docs.noarch: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl.x86_64: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.x86_64: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.x86_64: W: spelling-error %description -l en_US spotify -> spottily
playerctl.x86_64: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.x86_64: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.x86_64: W: spelling-error %description -l en_US cmus -> cums, mus, emus
playerctl.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-libs.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-libs.x86_64: W: no-documentation
playerctl-libs-debuginfo.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-devel.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-devel.x86_64: W: no-documentation
playerctl-debugsource.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-static.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
playerctl-static.x86_64: W: no-documentation
playerctl-debuginfo.x86_64: W: invalid-url URL: https://github.com/acrisci/playerctl <urlopen error [Errno -2] Name or service not known>
8 packages and 0 specfiles checked; 0 errors, 17 warnings.



Source checksums
----------------
https://github.com/acrisci/playerctl/releases/download/v2.0.1/playerctl-2.0.1.tar.xz :
  CHECKSUM(SHA256) this package     : c3ae6d74ddd951767da4e7852a76a88a2920ed02f126dab6e3b6f0c77549c74f
  CHECKSUM(SHA256) upstream package : c3ae6d74ddd951767da4e7852a76a88a2920ed02f126dab6e3b6f0c77549c74f


Requires
--------
playerctl (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libplayerctl.so.2()(64bit)
    playerctl-libs(x86-64)
    rtld(GNU_HASH)

playerctl-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libplayerctl.so.2()(64bit)
    pkgconfig(gio-2.0)
    pkgconfig(gio-unix-2.0)
    pkgconfig(gobject-2.0)
    playerctl(x86-64)

playerctl-docs (rpmlib, GLIBC filtered):
    playerctl(x86-64)

playerctl-libs (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    playerctl(x86-64)
    rtld(GNU_HASH)

playerctl-static (rpmlib, GLIBC filtered):
    playerctl-devel(x86-64)

playerctl-debuginfo (rpmlib, GLIBC filtered):

playerctl-debugsource (rpmlib, GLIBC filtered):



Provides
--------
playerctl:
    playerctl
    playerctl(x86-64)

playerctl-devel:
    pkgconfig(playerctl)
    playerctl-devel
    playerctl-devel(x86-64)

playerctl-docs:
    playerctl-docs

playerctl-libs:
    libplayerctl.so.2()(64bit)
    playerctl-libs
    playerctl-libs(x86-64)

playerctl-static:
    playerctl-static
    playerctl-static(x86-64)

playerctl-debuginfo:
    debuginfo(build-id)
    playerctl-debuginfo
    playerctl-debuginfo(x86-64)

playerctl-debugsource:
    playerctl-debugsource
    playerctl-debugsource(x86-64)



Generated by fedora-review 0.6.1 (37b2325) last change: 2019-03-10
Command line :try-fedora-review -b 1671571 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, C/C++, Shell-api
Disabled plugins: fonts, Java, SugarActivity, R, PHP, Perl, Haskell, Python, Ruby, Ocaml
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 30 Justin W. Flory (Fedora) 2019-04-09 22:04:08 UTC
Hi, a few changes:

1. Updated package to latest upstream release, v2.0.2
2. Implemented feedback from fedora-review tool
3. Updated package successfully built in COPR [1] and Koji [2][3][4]

The updated RPM SPEC and SRPM are below and ready for review:

SPEC URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
SRPM URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/playerctl-2.0.2-1.fc29.src.rpm

As for packaging the static library, I do not know how to work around not including it. I reviewed the packaging guidelines and they do not mention a requirement to get an exception to ship the static library, only that the static library should be packaged separately:

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_packaging_static_libraries

I ran this through fedora-review this time before I created this SRPM. Let me know if anything is left for this package review.

---
[1]  https://copr.fedorainfracloud.org/coprs/jflory7/playerctl/build/881174/
[2]  https://koji.fedoraproject.org/koji/taskinfo?taskID=34085414
[3]  https://koji.fedoraproject.org/koji/taskinfo?taskID=34085407
[4]  https://koji.fedoraproject.org/koji/taskinfo?taskID=34085421

Comment 31 Dridi Boukelmoune 2019-04-11 10:21:33 UTC
Unfortunately I still found a few things and ran a few scratch builds myself.

I suspect this will be the last iteration!

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- If your application is a C or C++ application you must list a
  BuildRequires against gcc, gcc-c++ or clang.
  Note: No gcc, gcc-c++ or clang found in BuildRequires
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

You get gcc in your build root because in your build dependency tree you
eventually drag libtool and it requires gcc. You should still explicitly add
one:

    Requires:  gcc

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.

As you pointed out, static libraries are not explicitly prohibited so let's
treat this one as a false-positive.

[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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: "Unknown or generated", "GNU Lesser General Public License (v3
     or later)". 21 files have unknown license. Detailed output of
     licensecheck in
     /home/dridi/fedora/FedoraReview/1671571-playerctl/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib64/girepository-1.0

Change the following line like this:
-%{_libdir}/girepository-1.0/Playerctl-2.0.typelib
+%{_libdir}/girepository-1.0/

[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/gtk-doc(
     [...])

This is compliant as per the guidelines.

[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.

Despite an update to version 2.0.2 the last changelog entry still refers to
2.0.1 and I'm also not sure about the indentation of changelog entries.

[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: 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.

I think the following from the devel sub-package is superfluous:

    Requires: pkgconfig(gobject-introspection-1.0)

I realized that BuildRequires are properly set thanks to the pkg-config file
shipped in the devel package and I shouldn't have asked this to be added in
the first place.

[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.
[ ]: 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]: 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 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 must not depend on deprecated() packages.
[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]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: playerctl-static.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[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.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     playerctl , playerctl-docs , playerctl-static

False-positives.

[x]: 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.

https://koji.fedoraproject.org/koji/taskinfo?taskID=34107470

[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]: The placement of pkgconfig(.pc) files are correct.
[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: playerctl-2.0.2-1.fc31.x86_64.rpm
          playerctl-devel-2.0.2-1.fc31.x86_64.rpm
          playerctl-docs-2.0.2-1.fc31.noarch.rpm
          playerctl-libs-2.0.2-1.fc31.x86_64.rpm
          playerctl-static-2.0.2-1.fc31.x86_64.rpm
          playerctl-debuginfo-2.0.2-1.fc31.x86_64.rpm
          playerctl-debugsource-2.0.2-1.fc31.x86_64.rpm
          playerctl-2.0.2-1.fc31.src.rpm
playerctl.x86_64: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.x86_64: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.x86_64: W: spelling-error %description -l en_US spotify -> spottily
playerctl.x86_64: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.x86_64: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.x86_64: W: spelling-error %description -l en_US cmus -> cums, mus, emus
playerctl.x86_64: W: incoherent-version-in-changelog 2.0.1-1 ['2.0.2-1.fc31', '2.0.2-1']
playerctl-devel.x86_64: W: no-documentation
playerctl-libs.x86_64: W: no-documentation
playerctl-static.x86_64: W: no-documentation
playerctl.src: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.src: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.src: W: spelling-error %description -l en_US spotify -> spottily
playerctl.src: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.src: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.src: W: spelling-error %description -l en_US cmus -> cums, mus, emus
8 packages and 0 specfiles checked; 0 errors, 16 warnings.




Rpmlint (debuginfo)
-------------------
Checking: playerctl-libs-debuginfo-2.0.2-1.fc31.x86_64.rpm
          playerctl-debuginfo-2.0.2-1.fc31.x86_64.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
playerctl-static.x86_64: W: no-documentation
playerctl.x86_64: W: spelling-error %description -l en_US statusline -> status line, status-line, stateliness
playerctl.x86_64: W: spelling-error %description -l en_US introspectable -> introspect able, introspect-able, introspective
playerctl.x86_64: W: spelling-error %description -l en_US spotify -> spottily
playerctl.x86_64: W: spelling-error %description -l en_US vlc -> vac, VLF
playerctl.x86_64: W: spelling-error %description -l en_US bmp -> mp, bump, imp
playerctl.x86_64: W: spelling-error %description -l en_US cmus -> cums, mus, emus
playerctl.x86_64: W: incoherent-version-in-changelog 2.0.1-1 ['2.0.2-1.fc31', '2.0.2-1']
playerctl-libs.x86_64: W: no-documentation
playerctl-devel.x86_64: W: no-documentation
8 packages and 0 specfiles checked; 0 errors, 10 warnings.



Source checksums
----------------
https://github.com/acrisci/playerctl/releases/download/v2.0.2/playerctl-2.0.2.tar.xz :
  CHECKSUM(SHA256) this package     : c84637893b3abc52eb7acb196259780af7cf99197aada7f0eab532b61c2d6751
  CHECKSUM(SHA256) upstream package : c84637893b3abc52eb7acb196259780af7cf99197aada7f0eab532b61c2d6751


Requires
--------
playerctl (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libplayerctl.so.2()(64bit)
    playerctl-libs(x86-64)
    rtld(GNU_HASH)

playerctl-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libplayerctl.so.2()(64bit)
    pkgconfig(gio-2.0)
    pkgconfig(gio-unix-2.0)
    pkgconfig(gobject-2.0)
    pkgconfig(gobject-introspection-1.0)
    playerctl(x86-64)

playerctl-docs (rpmlib, GLIBC filtered):
    playerctl

playerctl-libs (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    playerctl(x86-64)
    rtld(GNU_HASH)

playerctl-static (rpmlib, GLIBC filtered):
    playerctl-devel(x86-64)

playerctl-debuginfo (rpmlib, GLIBC filtered):

playerctl-debugsource (rpmlib, GLIBC filtered):



Provides
--------
playerctl:
    playerctl
    playerctl(x86-64)

playerctl-devel:
    pkgconfig(playerctl)
    playerctl-devel
    playerctl-devel(x86-64)

playerctl-docs:
    playerctl-docs

playerctl-libs:
    libplayerctl.so.2()(64bit)
    playerctl-libs
    playerctl-libs(x86-64)

playerctl-static:
    playerctl-static
    playerctl-static(x86-64)

playerctl-debuginfo:
    debuginfo(build-id)
    playerctl-debuginfo
    playerctl-debuginfo(x86-64)

playerctl-debugsource:
    playerctl-debugsource
    playerctl-debugsource(x86-64)



Generated by fedora-review 0.6.1 (37b2325) last change: 2019-03-10
Command line :try-fedora-review -b 1671571 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: C/C++, Generic, Shell-api
Disabled plugins: R, Java, PHP, Ruby, Perl, Ocaml, Haskell, SugarActivity, Python, fonts
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 32 Justin W. Flory (Fedora) 2019-04-11 14:25:40 UTC
(In reply to Dridi Boukelmoune from comment #31)
> Unfortunately I still found a few things and ran a few scratch builds myself.
> 
> I suspect this will be the last iteration!
> 

Great! Thanks for this round of feedback. The updated RPM SPEC and SRPM are below and ready for review:

SPEC URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SPECS/playerctl.spec
SRPM URL: https://pagure.io/jflory7-rpm-specs/raw/master/f/rpmbuild/SRPMS/playerctl-2.0.2-2.fc29.src.rpm

Let me know if anything else is remaining.

Comment 33 Dridi Boukelmoune 2019-04-12 18:51:02 UTC
Approved.

Don't forget to add me as a co-maintainer, I will stay around to help if you get any bug reports or if I see an update lingering.

Comment 34 Tony Crisci 2019-04-12 18:56:21 UTC
Thanks for helping me get my software out there. It's a privilege to be in the repo. If you have problems with upstream down the line, send me an email or open an issue on github.

Comment 35 Gwyn Ciesla 2019-04-12 19:16:57 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/playerctl

Comment 36 Fedora Update System 2019-04-12 20:33:56 UTC
playerctl-2.0.2-2.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-9453c711eb

Comment 37 Fedora Update System 2019-04-12 20:35:32 UTC
playerctl-2.0.2-2.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-1bac0790b1

Comment 38 Fedora Update System 2019-04-12 20:36:07 UTC
playerctl-2.0.2-2.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2019-4249741e9d

Comment 39 Fedora Update System 2019-04-13 02:00:22 UTC
playerctl-2.0.2-2.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-9453c711eb

Comment 40 Fedora Update System 2019-04-13 03:22:20 UTC
playerctl-2.0.2-2.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-4249741e9d

Comment 41 Fedora Update System 2019-04-13 12:10:49 UTC
playerctl-2.0.2-2.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-1bac0790b1

Comment 42 Fedora Update System 2019-04-21 00:40:55 UTC
playerctl-2.0.2-2.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.

Comment 43 Fedora Update System 2019-04-21 05:04:28 UTC
playerctl-2.0.2-2.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 44 Fedora Update System 2019-04-27 21:24:13 UTC
playerctl-2.0.2-2.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.


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