Bug 226418 - Merge Review: sharutils
Merge Review: sharutils
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 15:59 EST by Nobody's working on this, feel free to take it
Modified: 2008-09-04 15:24 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-04 15:24:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:59:16 EST
Fedora Merge Review: sharutils

http://cvs.fedora.redhat.com/viewcvs/devel/sharutils/
Initial Owner: than@redhat.com
Comment 1 Michael Schwendt 2007-02-06 09:27:51 EST
There is a newer 4.6.3 release plus sig upstream:
ftp://ftp.gnu.org/gnu/sharutils/REL-4.6.3/


> 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 08:11:29 EDT
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
pages.

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 08:12:57 EDT
Also 
make %{?_smp_mflags}
should be used unless it doesn't work.
Comment 4 Jason Tibbitts 2008-09-04 15:24:09 EDT
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:
   2f29604c9bc4471fb35975c10074bb3585dea66ebc52b5560989a370f2e3f00e  
   sharutils-4.7.tar.bz2
* 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
  =
   /bin/bash
   /bin/sh
   /usr/bin/perl
   info
   perl(File::Temp)

* %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.

APPROVED

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