Bug 467376 (mingw32-pixman)

Summary: Review Request: mingw32-pixman - MinGW Windows Pixman library
Product: [Fedora] Fedora Reporter: Richard W.M. Jones <rjones>
Component: Package ReviewAssignee: Peter Robinson <pbrobinson>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: berrange, fedora-package-review, notting, pbrobinson
Target Milestone: ---Flags: pbrobinson: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-02-02 16:09:20 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 454410, 478640    
Bug Blocks: 467416    

Comment 2 Richard W.M. Jones 2009-01-13 12:57:44 UTC
Spec URL: http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/file/tip/pixman/mingw32-pixman.spec
SRPM URL: http://www.annexia.org/tmp/mingw/fedora-10/src/SRPMS/mingw32-pixman-0.13.2-1.fc10.src.rpm
* Tue Jan 13 2009 Richard W.M. Jones <rjones> - 0.13.2-1
- Resynch with Fedora package (0.13.2).
- Disable static library for speed.
- Use _smp_mflags.
- Requires pkgconfig.
- Depends on dlfcn.

Here's a Koji scratch-build without the additional dlfcn
dependency:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1049362

Comment 3 Peter Robinson 2009-01-15 07:34:23 UTC
Mostly OK. Needs a license file. I'm not quite sure what the make-pixman-snapshot.sh script is used for. Its not packaged nor used during the build process that I can see. I presume it must be for creating a git/svn/cvs snapshot, which AFAICT we're not using a snapshot so does it need to be included. It also doesn't currently build in koji but that's expected due to the lack of mingw32-dlfcn. It does build without it though. I'm not sure whether mingw32-dlfcn is currently being reviewed. Its got input on the ticket but nobody currently has ownership of it.

+ rpmlint output

$ rpmlint -i mingw32-pixman-0.13.2-1.fc10.src.rpm 
mingw32-pixman.src: W: strange-permission make-pixman-snapshot.sh 0775
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

- %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  837df4a02c61a60a880644393b57faed  pixman-0.13.2.tar.gz
- package successfully builds on at least one architecture
  tested using koji scratch build
  http://koji.fedoraproject.org/koji/taskinfo?taskID=1054744
+ BuildRequires list all build dependencies
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr
n/a package owns all directories it creates
n/a no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
+ packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
- reviewer should build the package in mock/koji
n/a the package should build into binary RPMs on all supported architectures
n/a review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

Comment 4 Richard W.M. Jones 2009-01-15 13:33:18 UTC
(In reply to comment #3)
> Mostly OK. Needs a license file. I'm not quite sure what the
> make-pixman-snapshot.sh script is used for. Its not packaged nor used during
> the build process that I can see. I presume it must be for creating a
> git/svn/cvs snapshot, which AFAICT we're not using a snapshot so does it need
> to be included.

That file is from the base Fedora pixman package.  I just included
it so that we keep as close to the base package as possible.  It's
not used currently in the base package either.

> It also doesn't currently build in koji but that's expected due
> to the lack of mingw32-dlfcn. It does build without it though. I'm not sure
> whether mingw32-dlfcn is currently being reviewed. Its got input on the ticket
> but nobody currently has ownership of it.

I'm hoping that Adel Gadllah will have a look at mingw32-dlfcn
(bug 478640) soon.

> + rpmlint output
> 
> $ rpmlint -i mingw32-pixman-0.13.2-1.fc10.src.rpm 
> mingw32-pixman.src: W: strange-permission make-pixman-snapshot.sh 0775
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.
> 
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Not sure what rpmlint is complaining about here :-)
It's a shell script, in the SRPM, (potentially) used to
build the package, so I don't think this is incorrect.

> - %doc includes license file

This is annoying isn't it.  Upstream don't provide a license
file and the base Fedora package doesn't include one either.

I've raised the following bug upstream:
http://bugs.freedesktop.org/show_bug.cgi?id=19582

Below is a new build which preemptively includes this
patch.

Spec URL: http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/file/tip/pixman/mingw32-pixman.spec
SRPM URL: http://www.annexia.org/tmp/mingw/fedora-10/src/SRPMS/mingw32-pixman-0.13.2-2.fc10.src.rpm

* Thu Jan 15 2009 Richard W.M. Jones <rjones> - 0.13.2-2
- Include LICENSE file (freedesktop bug 19582).

$ rpm -qlp /home/rjones/rpmbuild/RPMS/noarch/mingw32-pixman-0.13.2-2.fc11.noarch.rpm
/usr/i686-pc-mingw32/sys-root/mingw/bin/libpixman-1-0.dll
/usr/i686-pc-mingw32/sys-root/mingw/include/pixman-1
/usr/i686-pc-mingw32/sys-root/mingw/include/pixman-1/pixman-version.h
/usr/i686-pc-mingw32/sys-root/mingw/include/pixman-1/pixman.h
/usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.dll.a
/usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.la
/usr/i686-pc-mingw32/sys-root/mingw/lib/pkgconfig/pixman-1.pc
/usr/share/doc/mingw32-pixman-0.13.2
/usr/share/doc/mingw32-pixman-0.13.2/LICENSE

Comment 5 Richard W.M. Jones 2009-01-15 13:35:01 UTC
Bug link should be: http://bugs.freedesktop.org/show_bug.cgi?id=19582

Comment 6 Peter Robinson 2009-01-20 15:00:51 UTC
Compiles fine now it has the dependencies in rawhide. Not sure if any of these are applicable for mingw binaries.

[perobinson@neo download]$ rpmlint -i mingw32-pixman-0.13.2-2.fc11.noarch.rpm 
mingw32-pixman.noarch: E: script-without-shebang /usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.la
This text file has executable bits set or is located in a path dedicated for
executables, but lacks a shebang and cannot thus be executed.  If the file is
meant to be an executable script, add the shebang, otherwise remove the
executable bits or move the file elsewhere.

mingw32-pixman.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/lib/pkgconfig/pixman-1.pc
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

mingw32-pixman.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/include/pixman-1/pixman.h
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

mingw32-pixman.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.dll.a
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

mingw32-pixman.noarch: W: spurious-executable-perm /usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.dll.a
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

mingw32-pixman.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/include/pixman-1/pixman-version.h
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

mingw32-pixman.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libpixman-1.dll.a
The package contains a binary or object file but is tagged noarch.

mingw32-pixman.noarch: W: non-standard-dir-in-usr i686-pc-mingw32
Your package is creating a non-standard subdirectory in /usr. The standard
directories are: X11R6, X386, bin, games, include, lib, lib64, local, sbin,
share, src, spool, tmp.

1 packages and 0 specfiles checked; 2 errors, 6 warnings.

Comment 7 Richard W.M. Jones 2009-01-21 14:33:58 UTC
These are all rpmlint warnings/errors that we'd normally ignore.
I'll explain here why:

script-without-shebang [...] libpixman-1.la
 - We use *.la files when linking.  This is explicitly allowed in
   the MinGW guidelines that were passed.
   http://fedoraproject.org/wiki/Packaging/MinGW#Libraries_.28DLLs.29

devel-file-in-non-devel-package
 - All MinGW packages are for development only, so when we passed
   the guidelines we didn't opt to have a separate -devel subpackage.
   See bug 468987 for why this warning isn't suppressed.

spurious-executable-perm
 - See https://bugzilla.redhat.com/show_bug.cgi?id=467397#c4

arch-independent-package-contains-binary-or-object
 - See bug 468989.

non-standard-dir-in-usr i686-pc-mingw32
 - This breaks the FHS, but is permitted by the MinGW guidelines.

/me should put this information in a wiki page ...

/me starts https://fedoraproject.org/wiki/MinGW/Rpmlint ....

Comment 8 Richard W.M. Jones 2009-01-28 12:04:01 UTC
Peter, ping?

If you have rawhide & mingw32-filesystem >= 44 installed, then
rpmlint errors are reduced to this single warning[1]:

mingw32-pixman.src: W: strange-permission make-pixman-snapshot.sh 0775

I don't think this is a valid warning because it's perfectly
normal for shell scripts to have executable permission.

[1] However this is cheating slightly, because all we are doing is
dropping in an rpmlint configuration file which tells rpmlint to
suppress the warnings listed in https://fedoraproject.org/wiki/MinGW/Rpmlint
for MinGW packages.

Comment 9 Peter Robinson 2009-01-28 16:06:41 UTC
Sorry, been away. It all seems fine to me. Overall with the mingw* packages I think there generally need to be updates to the packaging specs to cover the mingw requirements (and rpmlint as appropriate) and also to include the license file (to be fair we'd roast a commercial company for this!) and most of the mingw reviews would be a breeze!

APPROVED!

Comment 10 Richard W.M. Jones 2009-01-28 16:23:27 UTC
Thanks Peter.

CVS request coming up ...

Comment 11 Richard W.M. Jones 2009-01-28 16:25:05 UTC
New Package CVS Request
=======================
Package Name: mingw32-pixman
Short Description: MinGW Windows Pixman library
Owners: rjones berrange lfarkas
Branches: F-10 EL-5
InitialCC:

Comment 12 Kevin Fenzi 2009-01-29 00:22:20 UTC
cvs done.

Comment 13 Peter Robinson 2009-02-02 16:09:20 UTC
Closing as now in rawhide

Comment 14 Fedora Update System 2009-02-05 02:07:47 UTC
mingw32-pixman-0.13.2-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Richard W.M. Jones 2009-02-09 09:00:23 UTC
Just to note that a COPYING file was added to pixman:
http://bugs.freedesktop.org/show_bug.cgi?id=19582