Bug 467376 - (mingw32-pixman) Review Request: mingw32-pixman - MinGW Windows Pixman library
Review Request: mingw32-pixman - MinGW Windows Pixman library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Robinson
Fedora Extras Quality Assurance
:
Depends On: mingw32-gcc mingw32-dlfcn
Blocks: mingw32-cairo
  Show dependency treegraph
 
Reported: 2008-10-17 04:21 EDT by Richard W.M. Jones
Modified: 2009-02-09 04:00 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-02 11:09:20 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pbrobinson: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 2 Richard W.M. Jones 2009-01-13 07:57:44 EST
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@redhat.com> - 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 02:34:23 EST
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 08:33:18 EST
(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@redhat.com> - 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 08:35:01 EST
Bug link should be: http://bugs.freedesktop.org/show_bug.cgi?id=19582
Comment 6 Peter Robinson 2009-01-20 10:00:51 EST
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 09:33:58 EST
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 07:04:01 EST
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 11:06:41 EST
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 11:23:27 EST
Thanks Peter.

CVS request coming up ...
Comment 11 Richard W.M. Jones 2009-01-28 11:25:05 EST
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-28 19:22:20 EST
cvs done.
Comment 13 Peter Robinson 2009-02-02 11:09:20 EST
Closing as now in rawhide
Comment 14 Fedora Update System 2009-02-04 21:07:47 EST
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 04:00:23 EST
Just to note that a COPYING file was added to pixman:
http://bugs.freedesktop.org/show_bug.cgi?id=19582

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