Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 226418

Summary: Merge Review: sharutils
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Jason Tibbitts <tibbs>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: pertusus, redhat-bugzilla, than
Target Milestone: ---Flags: tibbs: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-09-04 19:24:09 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

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

http://cvs.fedora.redhat.com/viewcvs/devel/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:
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 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
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 12:12:57 UTC
Also 
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:
   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