Bug 226418 - Merge Review: sharutils
Summary: Merge Review: sharutils
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2007-01-31 20:59 UTC by Nobody's working on this, feel free to take it
Modified: 2008-09-04 19:24 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2008-09-04 19:24:09 UTC
Type: ---
j: fedora-review+

Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 20:59:16 UTC
Fedora Merge Review: sharutils

Initial Owner: than@redhat.com

Comment 1 Michael Schwendt 2007-02-06 14:27:51 UTC
There is a newer 4.6.3 release plus sig upstream:

> Prereq: /sbin/install-info

Requires(post): /sbin/install-info
Requires(preun): /sbin/install-info

> %makeinstall

Standard make install with DESTDIR ought to be preferred, provided
that it works:

  make DESTDIR=$RPM_BUILD_ROOT install

The %makeinstall macro overrides many Make variables, which can lead
to %buildroot finding its way into built files.

> mkdir -p ${RPM_BUILD_ROOT}%{_docdir}-%{name}-%{version}

This line should be unnecessary for files installed with %doc.

> BuildRoot: ...

Doesn't match the Fedora standard

  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

and might be rejected as soon as it might become mandatory.

Comment 2 Patrice Dumas 2007-10-05 12:11:29 UTC
Additionally some issues:

.gz in install-info scriptlets should be removed.

You can add 
INSTALL='install -p' or the like to keep the timestamps of the man

There are checks, they should certainly be run in %check with
make check

I don't think that the following line, in the description is useful:

Install sharutils if you send binary files through e-mail.

rpmlint is not silent:

sharutils.src:10: W: prereq-use /sbin/install-info
sharutils.src:158: W: macro-in-%changelog defattr
sharutils.src: W: summary-ended-with-dot The GNU shar utilities for packaging
and unpackaging shell archives.
sharutils.src: W: invalid-license GPL
sharutils.i386: W: file-not-utf8 /usr/share/doc/sharutils-4.6.3/TODO
sharutils.i386: W: file-not-utf8 /usr/share/doc/sharutils-4.6.3/THANKS
sharutils.i386: W: summary-ended-with-dot The GNU shar utilities for packaging
and unpackaging shell archives.
sharutils.i386: W: invalid-license GPL
sharutils-debuginfo.i386: W: invalid-license GPL

A suggestion:
replace %defattr(-,root,root) with %defattr(-,root,root,-)

Comment 3 Patrice Dumas 2007-10-05 12:12:57 UTC
make %{?_smp_mflags}
should be used unless it doesn't work.

Comment 4 Jason Tibbitts 2008-09-04 19:24:09 UTC
After recent checkins, this builds fine and rpmlint is silent.  All of the above suggestions seem to have been addressed.

One minor problem is that Requires(pre): info should be Requires(post): info.  This is trivial, so I have committed a fix.

* source files match upstream:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   sharutils = 4.7-2.fc10
   sharutils(x86-64) = 4.7-2.fc10

* %check is present and all tests pass:
   All 5 tests passed

* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %find_lang used properly to collect locale files.
* scriptlets are OK (info page installation).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.


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