Bug 225742 - Merge Review: expat
Merge Review: expat
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Joe Orton
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:34 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version: 1.95.8-10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-16 13:37:35 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ruben: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:34:54 EST
Fedora Merge Review: expat

http://cvs.fedora.redhat.com/viewcvs/devel/expat/
Initial Owner: jorton@redhat.com
Comment 1 Ruben Kerkhof 2007-02-03 15:03:20 EST
* RPM name is OK
* Source expat-1.95.8.tar.gz is the same as upstream
* Builds fine in mock
* File list of expat looks OK
* File list of expat-devel looks OK

Needs work:
* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* The %makeinstall macro should not be used
  (wiki: PackagingGuidelines#MakeInstall)
* rpmlint of expat: W: expat summary-ended-with-dot A library for parsing XML.
* rpmlint of expat-devel: W: expat-devel summary-ended-with-dot Libraries and include files to 
develop XML applications with expat.

Minor:
* Duplicate BuildRequires: autoconf (by automake), automake (by libtool)

Notes:
* Please use {?dist} in Release tag
* You should Require(post) and Require(postun) ldconfig
* Is it necessary to include static binaries (see the wiki: PackageGuidelines/Exclusion of Static Libraries
Comment 2 Joe Orton 2007-02-04 08:28:22 EST
Thanks for the review.

Fixed in -10:
- Summary trailing dot removed
- switch to preferred BuildRoot

Won't fix:
- %makeinstall is necessary for this non-DESTDIR-aware Makefile
- dist tag use is not mandatory
- explicit BuildRequires is a good thing
- rpm automatically adds correct Requires for scriptlet interpreters
- yes, static libraries are necessary for the static build of rpm
Comment 3 Ruben Kerkhof 2007-02-04 09:05:10 EST
Hi Joe,

> - %makeinstall is necessary for this non-DESTDIR-aware Makefile

Agreed, from the guidelines:
Fedora's RPM includes a %makeinstall macro but it must NOT be used when make install DESTDIR=%
{buildroot} will work.
So in this case it's not a blocker.

> - dist tag use is not mandatory

That's right

> - explicit BuildRequires is a good thing

True.

> - rpm automatically adds correct Requires for scriptlet interpreters

True as well.

> - yes, static libraries are necessary for the static build of rpm

There's no official policy on the inclusion of static libraries as far as I know, so this is no blocker either.

This package is APPROVED.
Comment 4 Robert Scheck 2007-02-04 09:52:32 EST
%{?dist} is no must? Since when if true or did I misunderstand something?
Comment 5 Ruben Kerkhof 2007-02-04 10:59:50 EST
Uh, look trought the list of MUST items in the Review Guidelines... nothing there.

From http://fedoraproject.org/wiki/Packaging/DistTag:

Do I Have To Use the Dist Tag?
No. It is documented and standardized so that maintainers who wish to use it can do so, but it is not 
mandatory.
Comment 6 Ruben Kerkhof 2007-03-18 06:07:33 EDT
Assigning ticket to myself as per http://fedoraproject.org/wiki/PackageReviewProcess

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