Bug 590387

I suggest to move shlibs to the mainpackage, then create -utils subpackage.

From http://www.littlecms.com, you'll find the main role of lcms2 is shlibs instead of applications.

Some more suggestions:

1. Update to the latest 2.0a

2. BuildRequires:  pkgconfig and Requires:  pkgconfig is not needed now, rpmbuild will add pkgconfig as dependency if .pc files exist.

3.BuildRequires:  swig >= 1.3.12 seems not needed, upstream tarball not include any wrappers currently.

Good review points, thanks.

New packages here:

Spec URL: http://people.freedesktop.org/~hughsient/temp/lcms2.spec
SRPM URL: http://people.freedesktop.org/~hughsient/temp/lcms2-2.0-2.fc13.src.rpm

rpmlint *.rpm
lcms2.src: I: enchant-dictionary-not-found en_US
lcms2.src: W: non-standard-group Unspecified
lcms2.src: W: no-cleaning-of-buildroot %clean
lcms2.src: W: no-buildroot-tag
lcms2.src: W: no-%clean-section
lcms2.x86_64: W: non-standard-group Unspecified
lcms2.x86_64: W: spurious-executable-perm /usr/share/doc/lcms2-2.0/AUTHORS
lcms2.x86_64: W: spurious-executable-perm /usr/share/doc/lcms2-2.0/COPYING
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmswtpnt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/transicc/transicc.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmserr.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsxform.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsplugin.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsgmt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmscnvrt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/include/lcms2_plugin.h
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/common/utils.h
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsio1.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmstypes.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/common/vprf.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsopt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/jpgicc/iccjpeg.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmspcs.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/common/xgetopt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmspack.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsps2.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsio0.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsgamma.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsvirt.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmssamp.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/jpgicc/jpgicc.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/psicc/psicc.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsnamed.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsintrp.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/linkicc/linkicc.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmssm.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmscgats.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsmd5.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmscam02.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/utils/tificc/tificc.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmslut.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/lcms2_internal.h
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/src/cmsmtrx.c
lcms2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/lcms-2.0/include/lcms2.h
lcms2-utils.x86_64: W: non-standard-group Unspecified
lcms2-utils.x86_64: W: spurious-executable-perm /usr/share/doc/lcms2-utils-2.0/AUTHORS
lcms2-utils.x86_64: W: manual-page-warning /usr/share/man/man1/jpgicc2.1.gz 40: warning: `p' not defined
lcms2-utils.x86_64: W: spurious-executable-perm /usr/share/doc/lcms2-utils-2.0/COPYING
lcms2-utils.x86_64: W: manual-page-warning /usr/share/man/man1/tificc2.1.gz 52: warning: `p' not defined
lcms2-utils.x86_64: W: no-manual-page-for-binary psicc2
lcms2-utils.x86_64: W: no-manual-page-for-binary linkicc2
lcms2-utils.x86_64: W: no-manual-page-for-binary transicc2
 
 non-standard-group Unspecified, spurious-executable-perm and no-%clean-section(if you want to push lcms2 to stable branches or EL6) should be fixed.
formal review here:
+:ok, =:needs attention, -:needs fixing

MUST Items:
[+] MUST: rpmlint must be run on every package.
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must meet the Packaging Guidelines. [FIXME?: covers this
list and more]
[+] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL.
<<md5sum checksum>>c4f115462a7a5b306c247d018d7a8982
[+] MUST: The package must successfully compile and build into binary rpms on
at least one supported architecture.
[+] MUST: All build dependencies must be listed in BuildRequires
[+] MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro.
[+] MUST: Every binary RPM package which stores shared library files (not just
symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun.
[+] MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[-] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.
[-] MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines.
[+] MUST: The package must contain code, or permissible content. This is
described in detail in the code vs. content section of Packaging Guidelines.
[+] MUST: If a package includes something as %doc, it must not affect the
runtime of the application.
[+] MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
[+] SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.
[=] SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin consider requiring the package which provides the file
instead of the file itself.


Some trival issue:
1.Version:        2.0 -> Version:        2.0a, in this case non-numeric is permiteed.
2.fix some rpm warings, see above.
3. %doc AUTHORS COPYING in -utils is not needed.
4. I suggest to change Summary:        Color Management System to Summary:        Color Management Engine

I think I've addressed most issues:

lcms2.i686: W: spelling-error %description -l en_US lcms -> lams, LCM, locums
lcms2-debuginfo.i686: W: spelling-error Summary(en_US) lcms -> lams, LCM, locums
lcms2-debuginfo.i686: W: spelling-error %description -l en_US lcms -> lams, LCM, locums
lcms2-utils.i686: W: spelling-error Summary(en_US) lcms -> lams, LCM, locums
lcms2-utils.i686: W: spelling-error %description -l en_US lcms -> lams, LCM, locums
lcms2-utils.i686: W: manual-page-warning /usr/share/man/man1/jpgicc2.1.gz 40: warning: macro `p' not defined
lcms2-utils.i686: W: manual-page-warning /usr/share/man/man1/tificc2.1.gz 52: warning: macro `p' not defined
lcms2-utils.i686: W: no-manual-page-for-binary psicc2
lcms2-utils.i686: W: no-manual-page-for-binary linkicc2
lcms2-utils.i686: W: no-manual-page-for-binary transicc2
lcms2.src: W: spelling-error %description -l en_US lcms -> lams, LCM, locums
5 packages and 0 specfiles checked; 0 errors, 11 warnings.

SPEC:  http://people.freedesktop.org/~hughsient/temp/lcms2.spec
SRPMS: http://people.freedesktop.org/~hughsient/temp/lcms2-2.0a-3.fc13.src.rpm

Some suggestions:
1.Change Group from Applications/Productivity to  System Environment/Librariesfor for main package.
Add Group: Applications/Productivity  for -utils subpackage
2.

# install docs as this is all we've got
install -D -m 644 doc/LittleCMS2.0\ tutorial.pdf $RPM_BUILD_ROOT/usr/share/doc/lcms2-devel-2.0/tutorial.pdf
install -D -m 644 doc/LittleCMS2.0\ API.pdf $RPM_BUILD_ROOT/usr/share/doc/lcms2-devel-2.0/api.pdf
install -D -m 644 doc/LittleCMS2.0\ Plugin\ API.pdf $RPM_BUILD_ROOT/usr/share/doc/lcms2-devel-2.0/plugin-api.pdf

%file devel
%{_datadir}/doc/lcms2-devel-2.0/*.pdf


Why not to simply use 
chmod -x doc/*.pdf 
%doc doc/*.pdf instead?

(In reply to comment #6)
> Some suggestions:
> 1.Change Group from Applications/Productivity to  System
> Environment/Librariesfor for main package.
> Add Group: Applications/Productivity  for -utils subpackage

Done, new spec uploaded, same location.

> 2.
> Why not to simply use 
> chmod -x doc/*.pdf 
> %doc doc/*.pdf instead?    

The docs are not installed by the project, and I'm choosing to install them manually.

Richard.

(In reply to comment #7)
> > 2.
> > Why not to simply use 
> > chmod -x doc/*.pdf 
> > %doc doc/*.pdf instead?    
> The docs are not installed by the project, and I'm choosing to install them
> manually.
> Richard.    

%doc doc/*.pdf = cp -p doc/*.pdf %{buildroot}/%{name}-devel-%{version}/

Using %doc is perfered in packaging guideline, unless you want to install docs to some non-standard place, you can simply use %doc instead.

This package is approved.

(In reply to comment #8)
> Using %doc is perfered in packaging guideline, unless you want to install docs
> to some non-standard place, you can simply use %doc instead.

Cool. I'm talking with the upstream maintainer about generating the content rather than shipping binary files.

> This package is approved.    

Thanks!

New Package CVS Request
=======================
Package Name: lcms2
Short Description: Color Management Engine
Owners: rhughes
Branches: F-12 F-13 EL-5 EL-6
InitialCC: rhughes

(In reply to comment #9)
> (In reply to comment #8)
> > Using %doc is perfered in packaging guideline, unless you want to install docs
> > to some non-standard place, you can simply use %doc instead.
> Cool. I'm talking with the upstream maintainer about generating the content
> rather than shipping binary files.
> > This package is approved.    
> Thanks!    

Distribute binary data files is not forbidden as I known :)

FYI, %file %{_datadir}/doc/* will also add doc flags to all files under this directoty, regardless you don't add %doc before those files.

CVS done (by process-cvs-requests.py).

Thanks guys!

@Richard,
Can you use install -p to prevent timestamp changes at %install step for lcms2 headers. There is also a need to verify  what is done in Makefile.

That may prevent multilib conflicts.

Thx

(In reply to comment #14)
> @Richard,
> Can you use install -p to prevent timestamp changes at %install step for lcms2
> headers. There is also a need to verify  what is done in Makefile.
> That may prevent multilib conflicts.
> Thx    

Agree, docs in -devel subpackage also need such tweak to avoid multilib conflict.

Also, it'll be better to use %{version} instead of hardcoded 2.0 in %install
 and %file.

(In reply to comment #14)
> @Richard,
> Can you use install -p to prevent timestamp changes at %install step for lcms2
> headers. There is also a need to verify  what is done in Makefile.

Sure, if you're a proven packager, just go in and make that change to devel, and I'll sync to F-* on the next release. Otherwise I'll make the change locally and just tag when we get the next upstream release.

Richard.

Richard, you requested branches for EPEL 5 and 6, but the package has not been imported nor, indeed, built. Do you plan to build for EPEL sooner or later ? This is a dependency for java-1.7.0-openjdk.

Xavier, I intended to build these, but got lost along the way. If you want to maintain them in EPEL I would be happy for the help.