Bug 226641 - Merge Review: xorg-x11-proto-devel
Summary: Merge Review: xorg-x11-proto-devel
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bill Nottingham
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:33 UTC by Nobody's working on this, feel free to take it
Modified: 2014-03-17 03:05 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-13 18:12:29 UTC
Type: ---
Embargoed:
notting: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 21:33:09 UTC
Fedora Merge Review: xorg-x11-proto-devel

http://cvs.fedora.redhat.com/viewcvs/devel/xorg-x11-proto-devel/
Initial Owner: ajackson

Comment 1 Bill Nottingham 2007-02-28 04:47:04 UTC
MUST Items:

 - Package meets naming and packaging guidelines ***

Package is an agglomeration of 29 source tarballs. I'm not going to tell you to
split it; so it's OK with me, even if the name doesn't match a particular
upstream tarball.

 - Spec file matches base package name. - OK
 - Spec has consistant macro usage. - OK
 - Meets Packaging Guidelines. - OK
 - License - MIT/X11 (albeit with many copyright holders )
 - License field in spec matches - ***

Should be changed to 'MIT/X11' (to match other X packages) or 'MIT' (to pacify
rpmlint).

 - License file included in package ***

Please include the various module copying. Yes, this is a mess. I
suggest in the build loop adding a:

 mv COPYING COPYING-${dir%%-*}

and adding a %doc */COPYING* directive.

 - Spec in American English - OK
 - Spec is legible. - OK
 - Sources match upstream md5sum: - OK (that was fun)

 - BuildRequires correct - OK
 - Package has %defattr and permissions on files is good. - OK
 - Package has a correct %clean section. - OK
 - Package has correct buildroot - OK
 - Package is code or permissible content. - OK
 - Packages %doc files don't affect runtime. - OK

 - .pc files in -devel subpackage/requires pkgconfig - ***

Should require pkgconfig.

 - Package compiles and builds on at least one arch. - OK
 - Package has no duplicate files in %files. - OK
 - Package doesn't own any directories other packages own. - OK
 - Package owns all the directories it creates. - ***

A Requires on pkgconfig should handle %{_libdir}/pkgconfig

 - No rpmlint output. - ***

Source rpmlint:
W: xorg-x11-proto-devel invalid-license The Open Group License

See above.

W: xorg-x11-proto-devel unversioned-explicit-obsoletes XFree86-devel
W: xorg-x11-proto-devel unversioned-explicit-obsoletes xorg-x11-devel

These aren't coming back, so it's OK with me. However, it's safer to add
a version for the last version of each.

W: xorg-x11-proto-devel mixed-use-of-spaces-and-tabs (spaces: line 89, tab: line 73)

Feel free to fix if you want.

Binary rpmlint:

W: xorg-x11-proto-devel invalid-license The Open Group License

See above.

E: xorg-x11-proto-devel obsolete-not-provided XFree86-devel
E: xorg-x11-proto-devel obsolete-not-provided xorg-x11-devel

Since this was a package split, this package alone does not provide the
functionality of xorg-x11-devel, etc. (no libX11, libXext, etc.) So this should
be OK.

E: xorg-x11-proto-devel no-binary

Not a bug.

SHOULD Items:

 - Should build in mock. - OK
 - Should have sane scriptlets. - OK
 - Should have dist tag - OK
 - Should package latest version - didn't check
 - check for outstanding bugs on package. (For core merge reviews) ***

Bug 229336 should be handled. Adding a simple:

%doc randrproto-*/randrproto.txt damageproto-*/damageproto.txt

along with a "rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc" in %install should handle it.


Comment 2 Adam Jackson 2007-02-28 20:38:10 UTC
> W: xorg-x11-proto-devel unversioned-explicit-obsoletes XFree86-devel
> W: xorg-x11-proto-devel unversioned-explicit-obsoletes xorg-x11-devel
> 
> These aren't coming back, so it's OK with me. However, it's safer to add
> a version for the last version of each.

So like:

Obsoletes: XFree86-devel <= 4.3.0

?

Did the rest in 7.2-5.

Comment 3 Bill Nottingham 2007-02-28 20:42:25 UTC
Yes, along those lines. Might need to add an epoch if
XFree86-devel/xorg-x11-devel had one. Admittedly, all it's doing is helping the
case if we ever switch back to XFree86, which is obviously going to happen any
day now.

Comment 4 Adam Jackson 2007-02-28 23:22:35 UTC
Mmm, inclined to just not bother really.

Comment 5 Bill Nottingham 2007-03-01 02:01:50 UTC
Normally the %doc macro is used (to put it in the rpm-defined docdir of
/usr/share/doc/%{name}-%{version} - the way you've done it makes it not actually
use the package name.

Comment 6 Adam Jackson 2007-03-01 20:36:30 UTC
Imagine that I don't understand how %doc works (I don't) and that the RPM manual
doesn't document it very well (it doesn't).

Comment 7 Bill Nottingham 2007-03-01 20:50:35 UTC
%doc is a magic macro interpreted by RPM during the build process. If it is
present, rpm, after the %install, %brp-compress, etc. processes, does:

cd <build dir>
mkdir -p $RPM_BUILD_ROOT/%{_datadir}/doc/%{name}-%{version}
cp -p <all things listed in %doc> $RPM_BUILD_ROOT/%{_datadir}/doc/%{name}-%{version}

It then marks all these files as 'documentation' in the package header.

So, all paths in %doc are relative to the rpm build directory (exploded source
tree).



Comment 8 Adam Jackson 2007-03-12 19:41:33 UTC
Does 7.2-6 look better?

Comment 9 Bill Nottingham 2007-03-12 19:47:32 UTC
Looks good. Feel free to close.

Comment 10 Adam Jackson 2007-03-13 18:12:29 UTC
Hooray!


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