Bug 225852 - Merge Review: gok
Merge Review: gok
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Debarshi Ray
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:57 EST by Nobody's working on this, feel free to take it
Modified: 2009-01-30 10:42 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-30 10:42:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
debarshir: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:57:11 EST
Fedora Merge Review: gok

http://cvs.fedora.redhat.com/viewcvs/devel/gok/
Initial Owner: davidz@redhat.com
Comment 1 Debarshi Ray 2008-12-28 12:05:45 EST
MUST Items: 

xx - rpmlint is unclean on RPM
    + [rishi@freebook devel]$ rpmlint gok
      gok.x86_64: W: file-not-utf8 /usr/share/doc/gok-1.3.7/NEWS
      gok.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/gok.schemas
      gok.x86_64: W: non-standard-group Desktop/Accessibility
      1 packages and 0 specfiles checked; 0 errors, 3 warnings.
      [rishi@freebook devel]$ 
    + rpmlint is unclean on SRPM
      [rishi@freebook SPECS]$ rpmlint /devel/rpmbuild/SRPMS/gok-2.25.2-1.fc9.src.rpm
      gok.src:251: W: macro-in-%changelog _lib
      gok.src: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 19)
      gok.src: W: non-standard-group Desktop/Accessibility
      1 packages and 0 specfiles checked; 0 errors, 3 warnings.
      [rishi@freebook SPECS]$

OK - follows Naming Guidelines
OK - spec file is named as %{name}.spec

xx - package does not meet Packaging Guidelines
    + Please consider fixing the mixed use of tabs and spaces in the
      BuildRequires and Requires tags.
    + Even though Fedora does not use the Group tag, please consider using one
      from /usr/share/doc/rpm-4.4.2.3/GROUPS. "User Interface/Desktops" is a
      probable choice. Orca also uses it.
    + 'Requires: htmlview' is needed because of Patch0.
    + Can gok-1.2.1-fixlogin.patch be removed from CVS?
    + Please convert /usr/share/doc/gok-1.3.7/NEWS from ISO-8859 to UTF-8. See:
      https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#file-not-utf8
    + According to https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make you should use 'make %{?_smp_mflags}' whenever possible. In this case since upstream supports parallel builds you should use it.
    + To preserve timestamps you could consider using:
      make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT
    + 'rm -rf $RPM_BUILD_ROOT/var/scrollkeeper' is not needed on recent
      distributions.
    + Replace %{_lib} with %%{_lib} in the %changelog.

OK - license meets Licensing Guidelines

OK - license field meets actual license
    + gok/keysym2ucs.c is in the public domain.

OK - upstream license file included in %doc
OK - spec file uses American English
OK - spec file is legible
OK - sources match upstream sources

xx - package does not build successfully
    + Fails to build due to missing BuildRequires:
      http://koji.fedoraproject.org/koji/taskinfo?taskID=1023579

OK - ExcludeArch not needed

xx - build dependencies incorrectly listed
    + 'BuildRequires: esound-devel' is missing.
    + 'BuildRequires: m4 perl(XML::Parser)' is redundant.

OK - locales handled properly
OK - no shared libraries
OK - package is not relocatable

xx - file and directory ownership
    + Since an icon has been installed under %{_datadir}/icons/hicolor there
      should be a runtime dependency on hicolor-icon-theme. However, is this
      icon really needed as there is no .desktop file?
    + The -devel sub-package should have 'Requires: gtk-doc' as it needs
      /usr/share/gtk-doc.

OK - no duplicates in %file
OK - file permissions set properly
OK - %clean present

OK - macros used consistently
    + Both gok and %{name} used.

OK - contains code and permissable content
OK - -doc is not needed
OK - contents of %doc does not affect the runtime
OK - no header files
OK - no static libraries
OK - -devel has *.pc file and requires pkgconfig
OK - no library files
OK - -devel requires base package
OK - no libtool archives

OK - %{name}.desktop file not provided
    + Changelog contains rationale for excluding it.

OK - does not own files or directories owned by other packages
OK - buildroot correctly prepped
OK - all file names valid UTF-8

SHOULD Items:

OK - upstream provides license text
xx - no translations for description and summary

xx - package fails to build in mock
    + 'BuildRequires: esound-devel' is missing.

OK - package builds on all supported architectures

OK - package functions as expected

xx - scriptlets are not sane
    + There does not seem to be any .omf files provided by the package, yet
      the scrollkeeper scriptlets are being used. Why? It looks like
      gok-1.3.7-3.fc9.x86_64 had them.

OK - subpackages other than -devel are not needed
OK - pkgconfig files in -devel
OK - no file dependencies
Comment 2 Debarshi Ray 2008-12-28 22:22:32 EST
(In reply to comment #1)
> xx - file and directory ownership
>     + Since an icon has been installed under %{_datadir}/icons/hicolor there
>       should be a runtime dependency on hicolor-icon-theme. However, is this
>       icon really needed as there is no .desktop file?

Ofcourse it is needed for the icons in the window titles. So please add the dependency on hicolor-icon-theme.
Comment 3 Debarshi Ray 2009-01-21 04:29:49 EST
Ping?
Comment 4 Matthias Clasen 2009-01-24 00:54:30 EST
I've fixed most of the fixworthy things you listed.

    + Since an icon has been installed under %{_datadir}/icons/hicolor there
      should be a runtime dependency on hicolor-icon-theme.

gok -> gtk2 -> hicolor-icon-theme

 + There does not seem to be any .omf files provided by the package, yet
      the scrollkeeper scriptlets are being used. W

[mclasen@matthiasc devel]$ rpm -ql gok | grep omf
/usr/share/omf/gok
/usr/share/omf/gok/gok-C.omf
Comment 5 Debarshi Ray 2009-01-28 14:35:31 EST
> + According to
>   https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make you should
>   use 'make %{?_smp_mflags}' whenever possible. In this case since upstream
>   supports parallel builds you should use it.

You have missed this one.

Sorry for not mentioning this earlier, but thanks to the new RPM in Rawhide the following Requires in the -devel sub-package are redundant:
    Requires: libgnomeui-devel
    Requires: atk-devel
    Requires: libbonobo-devel
    Requires: gtk2-devel
    Requires: gail-devel
    Requires: libwnck-devel
    Requires: esound-devel
    Requires: at-spi-devel
    Requires: pkgconfig

[rishi@freebook tmp]$ rpm --requires -qp gok-devel-2.25.3-3.fc11.x86_64.rpm
/usr/bin/pkg-config
[...]
pkgconfig(atk)
pkgconfig(cspi-1.0)
pkgconfig(esound)
pkgconfig(gail)
pkgconfig(gtk+-2.0)
pkgconfig(libbonobo-2.0)
pkgconfig(libgnomeui-2.0)
pkgconfig(libspi-1.0)
pkgconfig(libwnck-1.0)

Even though https://fedoraproject.org/wiki/Packaging:Guidelines#Pkgconfig_Files still mandates the use of 'Requires: pkgconfig', Spot confirmed that they are not needed from Fedora 11 onwards.
Comment 6 Matthias Clasen 2009-01-28 14:50:41 EST
> You have missed this one.

No, I haven't. I didn't feel it was worth it. There's only so much time I can justify spending on irrelevant details of a spec file that make no difference whatsoever to the resulting package.
Comment 7 Debarshi Ray 2009-01-29 08:51:04 EST
(In reply to comment #6)

>> You have missed this one.
 
> No, I haven't. I didn't feel it was worth it. There's only so much time I can
> justify spending on irrelevant details of a spec file that make no difference
> whatsoever to the resulting package.

The guidelines recommend the use of parallel builds and there is a rationale behind it: http://www.redhat.com/archives/fedora-packaging/2007-July/msg00004.html Can I make the change on your behalf?
Comment 8 Matthias Clasen 2009-01-29 09:52:53 EST
Thats not a rationale. 
Thats just spot saying "it should work". 
Do you know that it works in this case ?
Comment 9 Mamoru TASAKA 2009-01-29 10:31:23 EST
(In reply to comment #8)
> Do you know that it works in this case ?

http://koji.fedoraproject.org/koji/taskinfo?taskID=1090498
Comment 10 Matthias Clasen 2009-01-29 10:39:21 EST
So, if it makes you happy, commit it. 
Obsessing over trivia like this is what makes merge reviews so resented...
Comment 11 Parag AN(पराग) 2009-01-29 10:59:03 EST
Merge-reviews are always topic of debate on fedora lists. If we have approved spec template as given in /etc/rpmdevtools/spectemplate-minimal.spec then why not megre-review packages simply follow them?
Merge-review packages should not be considered a special case and exempted from what FESCo approved Packaging Guidelines say.
Comment 12 Matthias Clasen 2009-01-29 12:14:02 EST
No need to jump on this.
Comment 13 Debarshi Ray 2009-01-30 03:46:51 EST
I added '%{?_smp_mflags}' and took the liberty to remove the mixed tabs and spaces warnings from RPMLint.

* Fri Jan 30 2009 Debarshi Ray <rishi@fedoraproject.org> - 2.25.3-4
- Removed mixed use of tabs and spaces.
- Use parallel make.

+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+

Please close this review as NEXTRELEASE.

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