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