Bug 226075 - Merge Review: libXinerama
Merge Review: libXinerama
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Package Reviews List
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:30 EST by Nobody's working on this, feel free to take it
Modified: 2010-09-02 02:49 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-09-02 02:49:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:30:58 EST
Fedora Merge Review: libXinerama

http://cvs.fedora.redhat.com/viewcvs/devel/libXinerama/
Initial Owner: sandmann@redhat.com
Comment 1 Orcan Ogetbil 2009-01-17 18:05:35 EST
The full review arrived!

* Summary and especially the description are bizarre. Can you update them. You can find these on the manpage:
    Xinerama - API for Xinerama extension to X11 Protocol   

    Xinerama is a simple library designed to interface the Xinerama Extension for 
    retrieving information about physical output devices which may be combined 
    into a single logical X screen.

* rpmlint says
   libXinerama.src:18: W: unversioned-explicit-obsoletes XFree86-libs
   libXinerama.src:18: W: unversioned-explicit-obsoletes xorg-x11-libs
   libXinerama.src:32: W: unversioned-explicit-obsoletes XFree86-devel
   libXinerama.src:32: W: unversioned-explicit-obsoletes xorg-x11-devel
   libXinerama.x86_64: E: zero-length /usr/share/doc/libXinerama-1.0.3/AUTHORS
   libXinerama.x86_64: E: zero-length /usr/share/doc/libXinerama-1.0.3/README
   libXinerama.x86_64: W: obsolete-not-provided XFree86-libs
   libXinerama.x86_64: W: obsolete-not-provided xorg-x11-libs
   libXinerama-devel.x86_64: W: obsolete-not-provided XFree86-devel
   libXinerama-devel.x86_64: W: obsolete-not-provided xorg-x11-devel
The zero-length files are obviously not needed so they should be removed. The obsoletes look very problematic. Can you fix those (or alternatively explain them in the SPEC file as comments)?

* BR: libXau-devel is not needed. Afaict it is not used.

* BRs: libX11-devel pkgconfig and xorg-x11-proto-devel are not needed. They will be picked up by libXext-devel.

* Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). This applies to the devel package.

! Try to make use of the %{name} macro (e.g. files sections).

* Parallel make must be supported whenever possible. If it is not supported, this should be noted in the SPEC file as a comment.


Added Adam and Matthias to CC since they are the last two known maintainers.
Sorry if this was not desired.
Comment 2 Adam Jackson 2009-02-21 16:03:13 EST
Mostly fixed in 1.0.3-3, except for the %description bit

Note that xorg-x11-proto-devel pulls in pkgconfig for free.
Comment 3 Orcan Ogetbil 2009-02-21 21:03:08 EST
Thanks, there are two more things.

? Could you at least define xinerama in the description as Jason did in libdmx? This is just a request from me.

* You forgot this one: BR: libXau-devel is not needed. Afaict it is not used.
Comment 4 Orcan Ogetbil 2009-06-09 15:03:04 EDT
ping?
Comment 5 Orcan Ogetbil 2010-02-19 02:51:58 EST
ping again?
Comment 6 Adam Jackson 2010-08-30 15:50:50 EDT
done.
Comment 7 Parag AN(पराग) 2010-08-30 21:38:13 EDT
where is the official review?
Comment 8 Orcan Ogetbil 2010-08-31 00:12:05 EDT
see comment 1, and then comment 3. It's all good.

The packager should have informed me about the changes and waited for me to set the fedora-review flag to + though.
Comment 9 Orcan Ogetbil 2010-08-31 18:32:30 EDT
Parag, can we close this now since a full review has been done?
Comment 10 Parag AN(पराग) 2010-09-01 06:36:12 EDT
Yes please. Thanks for this review.

But, I generally suggest

Package change happened in F15 so,
1) buildroot should be removed
2) %clean not needed anymore
3) cleaning of buildroot at start of %install also not needed

I wonder if maintainer can find some time to clean his package then why can't he apply above cosmetic changes also.
Comment 11 Orcan Ogetbil 2010-09-01 10:16:26 EDT
(In reply to comment #10)
> 1) buildroot should be removed

Any references for this? I don't see a guideline that says buildroot should be removed.

> 2) %clean not needed anymore
> 3) cleaning of buildroot at start of %install also not needed
> 

'Not needed' doesn't mean 'Must be removed'. It is the packager's choice to keep them. We have to respect that.
Comment 12 Parag AN(पराग) 2010-09-01 10:31:18 EDT
(In reply to comment #11)
> (In reply to comment #10)
> > 1) buildroot should be removed
> 
> Any references for this? I don't see a guideline that says buildroot should be
> removed.
 
 My understanding to reference http://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag is that if buildroot tag is ignored then why to unnecessarily write them in spec files.

> > 2) %clean not needed anymore

 reference to http://fedoraproject.org/wiki/PackagingGuidelines#.25clean

> > 3) cleaning of buildroot at start of %install also not needed

  Again reference to http://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag this cleaning of buildroot is now implicit.

> > 
> 
> 'Not needed' doesn't mean 'Must be removed'. It is the packager's choice to
> keep them. We have to respect that.

Sure that is why I suggest them. So its upto maintainer whether to accept above 3 changes or not.
Comment 13 Orcan Ogetbil 2010-09-02 02:49:00 EDT
Thank you. I am sure the maintainer will consider your suggestions.

Closing the bug for good this time.

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