Bug 631763

My bits:
>$ rpmlint ../SRPMS/zif-0.1.0-1.fc13.src.rpm 
>zif.src: W: non-standard-group Unspecified
I guess you should specify package group. System Environment/Libraries can be
correct.

>zif.src:53: W: macro-in-comment %{_libdir}
>zif.src: W: no-cleaning-of-buildroot %clean
>zif.src: W: no-buildroot-tag
>zif.src: W: no-%clean-section
>1 packages and 0 specfiles checked; 0 errors, 5 warnings.
it is ok

>$ rpmlint ../RPMS/x86_64/zif-*
>zif.x86_64: W: non-standard-group Unspecified
>zif.x86_64: W: shared-lib-calls-exit /usr/lib64/libzif.so.1.0.1 exit.5
This is not good. Should be fixed upstream (but I don't examine it deeply)

>zif.x86_64: W: manual-page-warning /usr/share/man/man1/zif.1.gz 1: warning: `\"' >not defined
I don't know...
>zif-devel.x86_64: W: spelling-error Summary(en_US) GLib -> G Lib, Glib, Lib
>zif-devel.x86_64: W: spelling-error %description -l en_US GLib -> G Lib, Glib, >Lib
>3 packages and 0 specfiles checked; 0 errors, 5 warnings.
ok

> %define alphatag                .20100908git
This macro is not used anywhere. Also, %global is preferred to %define these days.

> %package devel
[...]
> Requires: sqlite-devel

%{?_isa} would be nice here.
http://www.rpm.org/wiki/PackagerDocs/ArchDependencies

> %package devel
[...]
> Requires: %{name} = %{version}-%{release}
[...]
> %files
> %defattr(-,root,root,-)
> %doc README AUTHORS NEWS COPYING
> [...]
> %files devel
> %defattr(-,root,root,-)
> %doc README AUTHORS NEWS COPYING

No need to duplicate these files here, because -devel Requires the main package.

The files in %{_datadir}/gtk-doc/html/zif/ look like developer's documentation to me. Wouldn't they be better placed in the -devel package?

(In reply to comment #1)
> My bits:
> >$ rpmlint ../SRPMS/zif-0.1.0-1.fc13.src.rpm 
> >zif.src: W: non-standard-group Unspecified
> I guess you should specify package group. System Environment/Libraries can be
> correct.

Nahh, this is only for F14+, so the group isn't required at all.

> >$ rpmlint ../RPMS/x86_64/zif-*
> >zif.x86_64: W: non-standard-group Unspecified
> >zif.x86_64: W: shared-lib-calls-exit /usr/lib64/libzif.so.1.0.1 exit.5
> This is not good. Should be fixed upstream (but I don't examine it deeply)

Although there is a call to exit(), it's in egg_error() which is a shared egg file between a number of projects. egg_error() is never called in zif, and certainly not exported, and so it's impossible to call exit.

> >zif.x86_64: W: manual-page-warning /usr/share/man/man1/zif.1.gz 1: warning: `\"' >not defined
> I don't know...

Not sure, I think it's a rpmlint buglet. All my packages seem to have this warning.

(In reply to comment #2)
> > %define alphatag                .20100908git
> This macro is not used anywhere. Also, %global is preferred to %define these
> days.

Eeek, sorry, that got left in from my initial package that was based on the git checkout. Removed now.

> > %package devel
> [...]
> > Requires: sqlite-devel
> 
> %{?_isa} would be nice here.
> http://www.rpm.org/wiki/PackagerDocs/ArchDependencies

Fixed.

> > %package devel
> [...]
> > Requires: %{name} = %{version}-%{release}
> [...]
> > %files
> > %defattr(-,root,root,-)
> > %doc README AUTHORS NEWS COPYING
> > [...]
> > %files devel
> > %defattr(-,root,root,-)
> > %doc README AUTHORS NEWS COPYING
> 
> No need to duplicate these files here, because -devel Requires the main
> package.

Fixed, thanks.

(In reply to comment #3)
> The files in %{_datadir}/gtk-doc/html/zif/ look like developer's documentation
> to me. Wouldn't they be better placed in the -devel package?

Yup, oops, thanks. New spec and SRPMS for review:

http://people.freedesktop.org/~hughsient/temp/zif.spec
http://people.freedesktop.org/~hughsient/temp/zif-0.1.0-2.fc14.src.rpm

Richard.

(In reply to comment #4)
> > >zif.x86_64: W: manual-page-warning /usr/share/man/man1/zif.1.gz 1: warning: `\"' >not defined
> > I don't know...
> 
> Not sure, I think it's a rpmlint buglet. All my packages seem to have this
> warning.

rpmlint merely cites a warning detected by running:
cat man/zif.1 |gtbl |groff -mtty-char -Tutf8 -P-c -mandoc -mandoc -wmac >/dev/null

It does not like the first line:
.\\" auto-generated by docbook2man-spec $Revision$

I believe the correct comment format would be:
.\" auto-generated ...

But that's a bug in docbook2man then.

(In reply to comment #6)
> But that's a bug in docbook2man then.
... filed as bug 639347.

(In reply to comment #4)
> (In reply to comment #1)
> > I guess you should specify package group. System Environment/Libraries can be
> > correct.
> 
> Nahh, this is only for F14+, so the group isn't required at all.

Right, rpm considers the Group tag optional since version 4.6. Though I could swear Fedora Packaging Guidelines still asked for the tag to be specified, I cannot find the requirement anywhere now. Perhaps it's been removed already.

But please be consistent. The -devel subpackage does have a Group tag.


I suggest you extend the %description for the main package a bit. Right now it only repeats what the Summary says and it is not very informative.

(In reply to comment #8)
> Right, rpm considers the Group tag optional since version 4.6. Though I could
> swear Fedora Packaging Guidelines still asked for the tag to be specified, I
> cannot find the requirement anywhere now. Perhaps it's been removed already.

I think so.

> But please be consistent. The -devel subpackage does have a Group tag.

Agreed, apologies.

> I suggest you extend the %description for the main package a bit. Right now it
> only repeats what the Summary says and it is not very informative.

Gotcha. I've added something better.

New spec and SRPMS for review:

http://people.freedesktop.org/~hughsient/temp/zif.spec
http://people.freedesktop.org/~hughsient/temp/zif-0.1.0-3.fc14.src.rpm

Richard.

> $ rpmlint {SRPMS,RPMS/x86_64}/zif*-3.*rpm
> zif.src: I: enchant-dictionary-not-found en_US
> zif.src: W: non-standard-group Unspecified
> zif.src: W: no-cleaning-of-buildroot %clean
> zif.src: W: no-buildroot-tag
> zif.src: W: no-%clean-section
> zif.x86_64: W: non-standard-group Unspecified

All OK with current rpm.

> zif.x86_64: W: shared-lib-calls-exit /usr/lib64/libzif.so.1.0.1 exit.5

OK. exit() is called only from egg_error_real() which is never used in the library.
BTW, a clever use of unsafe linker options might even elimitate the function.

> zif.x86_64: W: manual-page-warning /usr/share/man/man1/zif.1.gz 1: warning: macro `\"' not defined
> zif-devel.x86_64: W: non-standard-group Unspecified
> 4 packages and 0 specfiles checked; 0 errors, 8 warnings.

Can be ignored.

Formal review
-----------------------------
[OK] ... guideline matched
[--] ... irrelevant guideline
[??] ... needs discussion
[!!] ... needs fixing
-----------------------------

[OK] rpmlint posted
[OK] Naming Guidelines
[OK] spec file name matches package name
[..] Packaging Guidelines:
  [OK] naming
  [OK] version and release
  [OK] legal
  [OK] no pre-built binaries
  [OK] spec legibility
  [OK] architecture support
  [OK] fs layout
  [OK] rpmlint
  [OK] changelog
  [OK] tags
  [OK] BuildRoot tag (not needed)
  [OK] %clean (not needed)
  [??] Requires
    I wonder what is special about sqlite-devel that it is required
    explicitly by zif-devel, but libarchive-devel is left to be pulled
    automatically via rpm's pkgconfig dependency extraction.
  [OK] BuildRequires
  [OK] Summary and description
  [OK] encoding
  [OK] documentation
  [OK] compiler flags
  [OK] debuginfo
  [OK] devel package
  [OK] requiring base package
  [OK] shared libraries
  [--] static libraries
  [OK] no duplication of system libraries
  [OK] no rpaths
  [--] config files
  [--] initscripts
  [--] desktop files
  [!!] macros
     %{_mandir} should be used instead of %{_datadir}/man/
  [--] %global preferred over %define
  [--] handling of locale files
  [OK] timestamps
  [OK] parallel make
  [OK] scriptlets
  [--] conditional deps
  [OK] not relocatable
  [OK] code vs content
  [!!] file and dir ownership
    %{_datadir}/gtk-doc/html is owned neither by zif-devel nor any
    Required package. Should Require gtk-doc?
  [OK] no duplicate files
  [OK] file permissions
  [--] users and groups
  [--] web apps
  [OK] no conflicts
  [OK] no kernel modules
  [OK] nothing under /srv
  [OK] no bundling
  [--] patches should have upstream bug link or comment
  [--] use of epochs
  [OK] symlinks
  [OK] man pages
  [--] application-specific guidelines
[OK] approved license
[OK] GPLv2+ is correct
[OK] COPYING included in %doc
[OK] American English
[OK] legible spec
[OK] source matches upstream, sha256sum:
  aefac3cce4310e942bf735ea259efc45ba32fbe1c7cb6461e7f74637fd5df2d5
[OK] builds successfully
[--] ExcludeArch
[OK] BuildRequires
[--] locales
[OK] ldconfig called
[OK] no bundling
[OK] not relocatable
[!!] owning of directories, already noted above
[OK] no listing files more than once
[OK] permissions, %defattr usage
[OK] consistent macro usage
[OK] code or permissible content
[--] large docs
[OK] docs not necessary for runtime
[OK] headers in -devel
[--] static libs in -static
[OK] *.so in -devel
[OK] -devel requires base with a fully versioned dep
[OK] no *.la archives
[--] *.desktop for GUI apps
[OK] no owning of directories already owned by other packages
[OK] valid UTF-8 filenames

(In reply to comment #10)
>   [??] Requires
>     I wonder what is special about sqlite-devel that it is required
>     explicitly by zif-devel, but libarchive-devel is left to be pulled
>     automatically via rpm's pkgconfig dependency extraction.

You're correct. sqlite-devel gets pulled in automatically. I've removed that line.

>   [!!] macros
>      %{_mandir} should be used instead of %{_datadir}/man/

Fixed.

>     %{_datadir}/gtk-doc/html is owned neither by zif-devel nor any
>     Required package. Should Require gtk-doc?

I wasn't sure. Looking into it, I shouldn't just add a Req for gtk-doc, as there's been a big push to not pull it in unless it's really required. I've just taken ownership of all the gtk-doc directory, like has been done to my other packages by other people.

> [!!] owning of directories, already noted above

Fixed.

New spec and SRPMS for review:

http://people.freedesktop.org/~hughsient/temp/zif.spec
http://people.freedesktop.org/~hughsient/temp/zif-0.1.0-4.fc14.src.rpm

Thanks,

Richard.

Everything is good now.

Current scratch build in dist-f15:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2506735

APPROVED

New Package SCM Request
=======================
Package Name: zif
Short Description: Simple wrapper for rpm
Owners: rhughes
Branches: f14
InitialCC: rhughes

Git done (by process-git-requests).

Thanks guys, appreciated.

1) If I look into https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Shared_libraries and considered /etc/rpmdevtools/spectemplate-lib.spec as a reference for library packaging specfile then I don't see following

Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig

2) Following is no longer mandatory by review guidelines since long time

Requires: pkgconfig

So you can remove this safely. Reference see this guidelines update diff https://fedoraproject.org/w/index.php?title=Packaging%3AGuidelines&diff=145230&oldid=144537

(In reply to comment #16)
> 1) If I look into
> https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Shared_libraries and
> considered /etc/rpmdevtools/spectemplate-lib.spec as a reference for library
> packaging specfile then I don't see following
> 
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> 
> 2) Following is no longer mandatory by review guidelines since long time
> 
> Requires: pkgconfig
> 
> So you can remove this safely. Reference see this guidelines update diff
> https://fedoraproject.org/w/index.php?title=Packaging%3AGuidelines&diff=145230&oldid=144537

Parag, please can you remove them from zif in fedora git master. You know what you are doing :-) Thanks.

Thanks. I have committed required changes and built new package in F15.