Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1239067
Summary: | Review Request: libaudclient - audacious D-Bus remote control library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sergio Basto <sergio> |
Component: | Package Review | Assignee: | Michael Schwendt <bugs.michael> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | gwync, package-review |
Target Milestone: | --- | Flags: | bugs.michael:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | libaudclient-3.5-0.2.rc2.fc21 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-07-23 08:58:57 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Sergio Basto
2015-07-03 12:14:03 UTC
> Name: libaudclient > Group: Development/Libraries The Group tag for runtime libraries has been "System Environment/Libraries" for many years. Nowadays the Group tag is obsolete: https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag > BuildRequires: autoconf > BuildRequires: automake Not needed. > %description > audacious D-Bus remote control library The player is called "Audacious" with upper-case 'A' - except for the logo and file names. > %package -n libaudclient-devel > Summary: Development files for libaudclient > Group: Development/Libraries/C and C++ Red Hat and Fedora have used group "Development/Libraries" for -devel packages for many years. > Requires: %{name} = %{version}-%{release} https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > %description -n libaudclient-devel > Development files for libaudclient (to communicate with audacious) As above. > ### corrects version mismatch > sed '/^version/cversion=%{version}' -i audclient.pc If the comment explained *why* it does that, it would be a better comment. Now there's a version mismatch in the file, because there are two different versions and two different ways to retrieve those versions: $ grep ^version /usr/lib64/pkgconfig/audclient.pc version=3.5 $ pkg-config --variable=version audclient 3.5 $ grep ^Version /usr/lib64/pkgconfig/audclient.pc Version: 3.5-rc2 $ pkg-config --atleast-version=3.5 audclient && echo "yes" yes $ pkg-config --atleast-version=3.5-rc2 audclient && echo "yes" yes $ pkg-config --atleast-version=3.5-rc3 audclient && echo "yes" $ > %build > %configure > make %{?_smp_mflags} Build output is non-verbose. One cannot see which compiler/linker flags are used. You can insert sed -i '\,^.SILENT:,d' buildsys.mk.in before %configure to fix that. Audacious does that for years, too. ;-) > %files -n libaudclient-devel > %dir %{_includedir}/audacious/ Directory ownership is okay, since package audacious-devel is not needed for installing libaudclient-devel. [...] As for the runtime, last audtty in Fedora rebuilds and seems to run with this. (In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #1) > > Name: libaudclient > > Group: Development/Libraries > > The Group tag for runtime libraries has been "System Environment/Libraries" > for many years. Nowadays the Group tag is obsolete: > https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag I read somewhere rpmlint validates groups from /usr/share/doc/rpm/GROUPS So I choose : Development/Libraries > > BuildRequires: autoconf > > BuildRequires: automake > > Not needed. OK , fixed > > > %description > > audacious D-Bus remote control library > > The player is called "Audacious" with upper-case 'A' - except for the logo > and file names. OK, fixed > > %package -n libaudclient-devel > > Summary: Development files for libaudclient > > Group: Development/Libraries/C and C++ > > Red Hat and Fedora have used group "Development/Libraries" for -devel > packages for many years. OK, so where is right use Group: Development/Libraries like in main package ? or should I remove Group on main package ? > > Requires: %{name} = %{version}-%{release} > > https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package OK, fixed > > %description -n libaudclient-devel > > Development files for libaudclient (to communicate with audacious) > > As above. OK, fixed > > ### corrects version mismatch > > sed '/^version/cversion=%{version}' -i audclient.pc > > If the comment explained *why* it does that, it would be a better comment. > Now there's a version mismatch in the file, because there are two different > versions and two different ways to retrieve those versions: > > $ grep ^version /usr/lib64/pkgconfig/audclient.pc > version=3.5 > $ pkg-config --variable=version audclient > 3.5 > > $ grep ^Version /usr/lib64/pkgconfig/audclient.pc > Version: 3.5-rc2 > $ pkg-config --atleast-version=3.5 audclient && echo "yes" > yes > $ pkg-config --atleast-version=3.5-rc2 audclient && echo "yes" > yes > $ pkg-config --atleast-version=3.5-rc3 audclient && echo "yes" > $ OK , I removed sed script This came in SUSE src.rpm, is not my script. I see now that SUSE Version was 3.5~rc2 , now is just 3.5 so it wasn't right at all . > > %build > > %configure > > make %{?_smp_mflags} > > Build output is non-verbose. One cannot see which compiler/linker flags are > used. You can insert > > sed -i '\,^.SILENT:,d' buildsys.mk.in > > before %configure to fix that. Audacious does that for years, too. ;-) DONE > > %files -n libaudclient-devel > > %dir %{_includedir}/audacious/ > > Directory ownership is okay, since package audacious-devel is not needed for > installing libaudclient-devel. So it is correct ? no modifications ? > [...] > > As for the runtime, last audtty in Fedora rebuilds and seems to run with > this. Cool! Thanks. SPEC: https://sergiomb.fedorapeople.org/libaudclient.spec SRPM: https://sergiomb.fedorapeople.org/libaudclient-3.5-0.2.rc2.fc21.src.rpm > I read somewhere rpmlint validates groups from /usr/share/doc/rpm/GROUPS > So I choose : Development/Libraries $ grep Lib /usr/share/doc/rpm/GROUPS Development/Libraries System Environment/Libraries As you can see, "System Environment/Libraries" is in there, too. > OK, so where is right use > Group: Development/Libraries > like in main package ? or should I remove Group on main package ? The Group tag can be set for each [sub-]package. Unless you want to drop it everywhere: https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag > Requires: %{name}}%{?_isa} = %{version}-%{release} The extra '}' in there causes the package to be not installable. >> Directory ownership is okay, since package audacious-devel >> is not needed for installing libaudclient-devel. > > So it is correct ? no modifications ? Yes, that's what "is okay" means. Reviewing is not only about pointing out mistakes, it's also an opportunity to acknowledge things that are done right. Owning /usr/include/audacious is acceptable according to this: https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function > %install > -%make_install > +make install DESTDIR=%{buildroot} A completely unnecessary change. Using %make_install is entirely acceptable. See: rpm -E %make_install https://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.25makeinstall_macro_should_not_be_used [...] If you fix the Group tag in dist git and the accidental '}', you can get an APPROVED here. (In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #3) > > I read somewhere rpmlint validates groups from /usr/share/doc/rpm/GROUPS > > So I choose : Development/Libraries > > $ grep Lib /usr/share/doc/rpm/GROUPS > Development/Libraries > System Environment/Libraries > > As you can see, "System Environment/Libraries" is in there, too. > > > > OK, so where is right use > > Group: Development/Libraries > > like in main package ? or should I remove Group on main package ? > > The Group tag can be set for each [sub-]package. Unless you want to drop it > everywhere: https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag Sorry, I misunderstood at first time , I'd like keep Group , so : Name: libaudclient Group: System Environment/Libraries (...) %package -n libaudclient-devel Summary: Development files for libaudclient Group: Development/Libraries is correct ? or what remove second group tag or second group tag should be: Group: System Environment/Libraries ? > > Requires: %{name}}%{?_isa} = %{version}-%{release} > > The extra '}' in there causes the package to be not installable. > > > >> Directory ownership is okay, since package audacious-devel > >> is not needed for installing libaudclient-devel. > > > > So it is correct ? no modifications ? > > Yes, that's what "is okay" means. Reviewing is not only about pointing out > mistakes, it's also an opportunity to acknowledge things that are done right. > Owning /usr/include/audacious is acceptable according to this: > https://fedoraproject.org/wiki/Packaging: > Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your > _package_to_function > > > > %install > > -%make_install > > +make install DESTDIR=%{buildroot} > > A completely unnecessary change. Using %make_install is entirely acceptable. > See: rpm -E %make_install > > > https://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_. > 25makeinstall_macro_should_not_be_used Fedora's RPM includes a %makeinstall macro but it must NOT be used when make install DESTDIR=%{buildroot} works. So I change it , is correct ? > [...] > > If you fix the Group tag in dist git and the accidental '}', you can get an > APPROVED here. OK thanks, > Fedora's RPM includes a %makeinstall macro but it must NOT be used > when make install DESTDIR=%{buildroot} works. There are _two_ macros: 1. %makeinstall 2. %make_install The %makeinstall macro should be avoided, because it redefines many path variables that can lead to ugly side-effects. The %make_install macro does exactly what you want. Really do take a look at "rpm -E %make_install" to see what the macro does. It's there to make packaging easier. ;-) > or what remove second group tag or second group tag should be: > Group: System Environment/Libraries ? No, "Development/Libraries" is correct for library -devel packages. Reloaded with last fixes : SPEC: https://sergiomb.fedorapeople.org/libaudclient.spec SRPM: https://sergiomb.fedorapeople.org/libaudclient-3.5-0.2.rc2.fc21.src.rpm Package Change Request ====================== Package Name: libaudclient Owners: sergiomb New Branches: f21 f22 WARNING: Package does not appear to exist in pkgdb currently. Use a New Package Request. New Package SCM Request ====================== Package Name: libaudclient Short Description: Client library for audacious Upstream URL: http://audacious-media-player.org/ Owners: sergiomb New Branches: f21 f22 Git done (by process-git-requests). I just have master branch ! , went to pkgdb [1] I requested branch f21 and f22 : Branch f21 created for user sergiomb Branch f22 created for user sergiomb but after that still not have: branch f22 and f21: fedpkg clone libaudclient cd libaudclient/ [snip] fedpkg push; fedpkg build fedpkg switch-branch f22 Unknown remote branch origin/f22 [1] https://admin.fedoraproject.org/pkgdb/package/libaudclient/ $ fedpkg clone libaudclient Cloning into 'libaudclient'... remote: Counting objects: 8, done. remote: Compressing objects: 100% (6/6), done. remote: Total 8 (delta 0), reused 0 (delta 0) Receiving objects: 100% (8/8), done. Checking connectivity... done. $ cd libaudclient $ fedpkg switch-branch f22 Branch f22 set up to track remote branch f22 from origin. $ fedpkg switch-branch f21 Branch f21 set up to track remote branch f21 from origin. $ fedpkg switch-branch master Switched to branch 'master' $ fedpkg switch-branch f22 Switched to branch 'f22' $ Try again? Today git pull did: From ssh://pkgs.fedoraproject.org/libaudclient * [new branch] f21 -> origin/f21 * [new branch] f22 -> origin/f22 Thanks, libaudclient-3.5-0.2.rc2.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/libaudclient-3.5-0.2.rc2.fc22 libaudclient-3.5-0.2.rc2.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/libaudclient-3.5-0.2.rc2.fc21 Michael Schwendt, Would you like join on Main Contact and Package Administrator of this package ? if yes you may request it in [1]. I will approve it . Thanks, [1] https://admin.fedoraproject.org/pkgdb/package/libaudclient/ libaudclient-3.5-0.2.rc2.fc22 has been pushed to the Fedora 22 testing repository. libaudclient-3.5-0.2.rc2.fc22 has been pushed to the Fedora 22 stable repository. libaudclient-3.5-0.2.rc2.fc21 has been pushed to the Fedora 21 stable repository. |