Bug 600243 - Review Request: libjpeg-turbo - MMX/SSE accelerated libjpeg
Summary: Review Request: libjpeg-turbo - MMX/SSE accelerated libjpeg
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chen Lei
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-06-04 09:17 UTC by Adam Tkac
Modified: 2013-04-30 23:46 UTC (History)
12 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-06-28 12:57:02 UTC
Type: ---
Embargoed:
supercyper1: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Adam Tkac 2010-06-04 09:17:13 UTC
Spec URL: http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
SRPM URL: http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-1.fc14.src.rpm
Description: libjpeg-turbo library contains MMX/SSE accelerated functions for manipulating JPEG images

Comment 1 Adam Tkac 2010-06-04 09:21:01 UTC
Note for the reviewer - it might look scary that libjpeg-turbo Obsoletes & Provides libjpeg but it is actually a proposed Fedora 14 feature - http://fedoraproject.org/wiki/Features/libjpeg-turbo.

Comment 2 Chen Lei 2010-06-09 07:36:39 UTC
Some trival comment:
1.Source0:	http://downloads.sourceforge.net/project/libjpeg-turbo/%{version}/libjpeg-turbo-%{version}.tar.gz

->
Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz

See http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net

2.
Obsoletes:	libjpeg < 6b-47.fc12
Provides:	libjpeg = 6b-47.fc12

Provides is not needed here.

Comment 4 Paul Wise (Debian) 2010-06-09 12:44:55 UTC
Wouldn't it be better to improve libjpeg?

Comment 5 Adam Tkac 2010-06-09 13:03:03 UTC
(In reply to comment #4)
> Wouldn't it be better to improve libjpeg?    

This question was properly discussed on devel list, check this thread:
http://lists.fedoraproject.org/pipermail/devel/2010-May/136556.html

Particularly those messages:
http://lists.fedoraproject.org/pipermail/devel/2010-June/137052.html
http://lists.fedoraproject.org/pipermail/devel/2010-June/137060.html (some parts of libjpeg-turbo are licensed under less restrictive license than libjpeg's license)

Comment 6 Chen Lei 2010-06-09 14:49:46 UTC
> > 2.
> > Obsoletes: libjpeg < 6b-47.fc12
> > Provides: libjpeg = 6b-47.fc12
> > 
> > Provides is not needed here.    
> If I read
> http://fedoraproject.org/wiki/PackageNamingGuidelines#Renaming.2Freplacing_existing_packages
> correctly it is needed, isn't it?
> New spec + SRPM:
> http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
> http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-2.fc14.src.rpm    
If a package supersedes/replaces an existing package without being a compatible enough replacement as defined in above, use only the Obsoletes from above. 

For packages that are not usually pulled in by using the package name as the dependency such as library only packages (which are pulled in through library soname depenencies), there's usually no need to add the Provides. Note however that the -devel subpackages of lib packages are pulled in as build dependencies using the package name, so adding the Provides is often appropriate there. 

So for libjpeg-turbo and libjpeg-turbo-tools provides are not needed. Upgrade path is not related to provides at all.

I'm not sure if obsoletes is needed for libjpeg-turbo since the -tools subpackage already depends on soname, you may need check it using yum.

yum install libjpeg from official repo.

Then koji-build libjpeg-turbo, download them to one directory.

Type: createrepo <directory>

Use a repo file such as this one:
[tmp]
name=tmp
baseurl=file:///tmp/rpm
enabled=1
gpgcheck=0

Type: yum upgrade

Comment 7 Adam Tkac 2010-06-10 09:02:35 UTC
(In reply to comment #6)
> For packages that are not usually pulled in by using the package name as the
> dependency such as library only packages (which are pulled in through library
> soname depenencies), there's usually no need to add the Provides. Note however
> that the -devel subpackages of lib packages are pulled in as build dependencies
> using the package name, so adding the Provides is often appropriate there. 
> 
> So for libjpeg-turbo and libjpeg-turbo-tools provides are not needed. Upgrade
> path is not related to provides at all.

Your analysis seems valid for me, I modified the obsoletes/provides as you suggested.

New spec + SRPM:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-3.fc14.src.rpm

Comment 8 Chen Lei 2010-06-10 09:48:33 UTC
Mock building for libjpeg-turbo fails on ppc/ppc64, since libjpeg is a critical path package in fedora, I think it should be fixed and build cleanly on most secondary architectures.


See http://koji.fedoraproject.org/koji/taskinfo?taskID=2242161

Comment 9 Adam Tkac 2010-06-10 10:43:42 UTC
(In reply to comment #8)
> Mock building for libjpeg-turbo fails on ppc/ppc64, since libjpeg is a critical
> path package in fedora, I think it should be fixed and build cleanly on most
> secondary architectures.

Seems I was dazzled by performance improvement on x86 & x64 and I forgot to test SRPM on secondary archs :)

Following SRPM should be fine:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-4.fc14.src.rpm

Comment 10 Michel Lind 2010-06-10 10:53:57 UTC
Chen Lei, will you be doing the package review?

Comment 11 Adam Goode 2010-06-10 16:53:32 UTC
Do you want the disttag in the obsoletes/provides? I don't think "fc12" looks right there.

Comment 12 Adam Tkac 2010-06-11 07:58:33 UTC
(In reply to comment #11)
> Do you want the disttag in the obsoletes/provides? I don't think "fc12" looks
> right there.    

Fixed, fetch new spec+SRPM from:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-5.fc14.src.rpm

Comment 13 Chen Lei 2010-06-11 08:32:12 UTC
An intersting question: If we enable SIMD optimazation in libjpeg-turbo, can it work fine on pure i686 machine without sse2, e.g. OLPC?

Comment 14 Michel Lind 2010-06-11 08:53:05 UTC
(In reply to comment #13)
> An intersting question: If we enable SIMD optimazation in libjpeg-turbo, can it
> work fine on pure i686 machine without sse2, e.g. OLPC?    

After the debacle with the F-13 GCC breakage, best bet would be to compile libjpeg-turbo for F-12 (since F-13 does not boot on OLPC right now) and ask around on their mailing lists for volunteers.

One would hope that the instructions to choose are dynamically picked at runtime, but someone familiar with the code should probably answer this. Adam?

Comment 15 Adam Tkac 2010-06-11 10:14:49 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > An intersting question: If we enable SIMD optimazation in libjpeg-turbo, can it
> > work fine on pure i686 machine without sse2, e.g. OLPC?    
> 
> After the debacle with the F-13 GCC breakage, best bet would be to compile
> libjpeg-turbo for F-12 (since F-13 does not boot on OLPC right now) and ask
> around on their mailing lists for volunteers.
> 
> One would hope that the instructions to choose are dynamically picked at
> runtime, but someone familiar with the code should probably answer this. Adam?    

It works fine on such machines, I tested it on my Pentium III:

[atkac@drtic ~]$ cat /proc/cpuinfo |egrep '(model|flags)'
model           : 8
model name      : Pentium III (Coppermine)
flags           : fpu vme de pse tsc msr pae mce cx8 mtrr pge mca cmov pse36 mmx fxsr sse up

libjpeg-turbo code uses the cpuid instruction and dynamically selects the best routines (SSE2/MMX/standard FPU).

Comment 16 Chen Lei 2010-06-11 14:55:33 UTC
The original libjpeg ship static subpackage, is there any package in fedora  link with libjpeg-static? Should libjpeg-turbo also need to ship static subpackage or simple obsoletes it?

Comment 17 Tom Lane 2010-06-11 15:06:10 UTC
The static library is used by some boot packages (think boot splash screen).  If your ambition is to replace libjpeg 6b altogether, you'd better offer a static library.  Obsoleting it without replacing the functionality would certainly be bogus anyway.

Comment 18 Chen Lei 2010-06-12 09:58:28 UTC
Hi Adam,
Add -static subpackage and more documents to mainpackage and -devel subpackage, I'll start to review this package.

See http://cvs.fedoraproject.org/viewvc/devel/libjpeg/libjpeg.spec?revision=1.29&view=markup

Comment 19 Kalev Lember 2010-06-13 00:20:10 UTC
repoquery --enablerepo=rawhide-source --archlist=src --whatrequires libjpeg-static
returns no matches, so I'd say no package in Fedora actually uses the -static subpackage.

Comment 20 Tom Lane 2010-06-13 04:42:32 UTC
> --whatrequires libjpeg-static returns no matches, 

Well, of course not.  Please review what static linking means.

Comment 21 Kalev Lember 2010-06-13 08:33:10 UTC
(In reply to comment #20)
> > --whatrequires libjpeg-static returns no matches, 
> 
> Well, of course not.  Please review what static linking means.    

If something in package A links against a static library in package B-static, then A also needs BuildRequires: B-static. Otherwise you just can't build against the static library if it isn't present at build time.

My repoquery queried *source* packages for such BuildRequires. Since the result was empty, it is safe to say in current rawhide, no package uses libjpeg-static for building.

Is that clearer now, Tom?

Comment 22 Tom Lane 2010-06-14 04:01:00 UTC
Well, nonetheless, you *are* going to get push-back if you remove the static library.  I know because it was tried once already, and not that long ago.  See bug #186060, bug #215537.  Fedora is used for other things than just building itself.

Comment 23 Adam Tkac 2010-06-14 10:19:37 UTC
(In reply to comment #22)
> Well, nonetheless, you *are* going to get push-back if you remove the static
> library.  I know because it was tried once already, and not that long ago.  See
> bug #186060, bug #215537.  Fedora is used for other things than just building
> itself.    

In my opinion it's long time since F7 (libjpeg-turbo will hit F14) so we can "retest" if someone still needs the static libjpeg. I would rather see no static libjpeg.a for now. It might be easily added in the future if someone requests it, libjpeg-turbo project uses libtool so I will drop the "--disable-static" configure flag in the specfile and add -static subpackage, it's pretty simple. If we package -static libjpeg now we won't be able to figure time when we can say "noone uses the static libjpeg.a".

Comment 24 Adam Tkac 2010-06-14 10:39:17 UTC
(In reply to comment #18)
> Hi Adam,
> Add -static subpackage and more documents to mainpackage and -devel subpackage,
> I'll start to review this package.

As I wrote in the comment #23 I don't think the -static package is needed now.

I included the "example.c" documentation file in the devel subpackage and extended description a little, it should be sufficient.

New spec + SRPM:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-6.fc14.src.rpm

Comment 25 Chen Lei 2010-06-14 10:51:39 UTC
(In reply to comment #24)
> (In reply to comment #18)
> > Hi Adam,
> > Add -static subpackage and more documents to mainpackage and -devel subpackage,
> > I'll start to review this package.
> 
> As I wrote in the comment #23 I don't think the -static package is needed now.
> 
> I included the "example.c" documentation file in the devel subpackage and
> extended description a little, it should be sufficient.
> 
> New spec + SRPM:
> http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
> http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-6.fc14.src.rpm    

How about to ask this on -devel? I found that many core libraries in fedora provides static libs, e.g. glibc openssl zlib libpng. 

If we don't provide -static subpackage, we need to obsolete it in the main package.

Comment 26 Adam Tkac 2010-06-14 11:29:18 UTC
(In reply to comment #25)
> How about to ask this on -devel? I found that many core libraries in fedora
> provides static libs, e.g. glibc openssl zlib libpng. 

Done. Check http://lists.fedoraproject.org/pipermail/devel/2010-June/137604.html.

> If we don't provide -static subpackage, we need to obsolete it in the main
> package.    

Right you are but it should be obsoleted in the -devel subpackage.

New spec + SRPM:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-7.fc14.src.rpm

Comment 27 Chen Lei 2010-06-15 13:09:49 UTC
formal review here:
+:ok, =:needs attention, -:needs fixing

MUST Items:
[+] MUST: rpmlint must be run on every package.
libjpeg-turbo.x86_64: W: shared-lib-calls-exit /usr/lib64/libjpeg.so.62.0.0 exit.5
libjpeg-turbo-devel.x86_64: W: obsolete-not-provided libjpeg-static
libjpeg-turbo-tools.x86_64: W: obsolete-not-provided libjpeg
Those warnings are harmless, and won't fix.
[+] 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>>ac8bb8b00558b077c159a2f35dc196a0
[+] 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.

Issues:
1. BR: nasm should be conditional

%ifarch %{ix86} x86_64
BuildRequires: nasm
%endif


2.

It'll be better to add more files to %doc

%doc README README-turbo.txt change.log ChangeLog.txt LGPL.txt LICENSE.txt 
%file devel
%doc coderules.doc jconfig.doc libjpeg.doc structure.doc wizard.doc example.c
%file tools
usage.doc 

3.

Personally, I suggest you to change -tools subpackage to -utils subpackages for two reasons:

yum list \*-utils and yum list \*-tools show we have much more utils subpackages in repo;

Upstream and wikipedia call those programs as utilities, the word utility will be more appropriate compared to tool.

See http://en.wikipedia.org/wiki/Libjpeg

Utilities
The following utility programs are available with libjpeg:

Comment 28 Chen Lei 2010-06-15 13:28:17 UTC
Packages need fix after review, most of those packages actually don't need depend on libjpeg explicitly:

repoquery --whatrequires  --exactdeps libjpeg
libjpeg-0:6b-46.fc12.x86_64
libjpeg-0:6b-46.fc12.i686
gallery2-jpegtran-0:2.3.1-1.fc13.noarch
java-1.6.0-openjdk-1:1.6.0.0-37.b17.fc13.x86_64
gocr-0:0.48-1.fc13.x86_64
darkplaces-0:20091001-2.fc13.x86_64
libjpeg-devel-0:6b-46.fc12.x86_64
libjpeg-devel-0:6b-46.fc12.i686
renrot-0:1.1-1.fc13.2.noarch
java-1.6.0-openjdk-1:1.6.0.0-39.b18.fc13.x86_64

Comment 29 Kalev Lember 2010-06-16 01:36:13 UTC
(In reply to comment #28)
> Packages need fix after review, most of those packages actually don't need
> depend on libjpeg explicitly:

To fix upgrade path for those packages I'd suggest to add Provides: libjpeg = %{version}-%{release}

Comment 30 Chen Lei 2010-06-16 02:17:01 UTC
(In reply to comment #29)
> (In reply to comment #28)
> > Packages need fix after review, most of those packages actually don't need
> > depend on libjpeg explicitly:
> 
> To fix upgrade path for those packages I'd suggest to add Provides: libjpeg =
> %{version}-%{release}    

It will break some packages silently, some of those packages depend on libjpeg utilities which now packages as a subpackage. Among those packages, only java-1.6.0-openjdk should fix intermediatly.

Comment 31 Kalev Lember 2010-06-16 02:25:29 UTC
(In reply to comment #30)
> (In reply to comment #29)
> > (In reply to comment #28)
> > > Packages need fix after review, most of those packages actually don't need
> > > depend on libjpeg explicitly:
> > 
> > To fix upgrade path for those packages I'd suggest to add Provides: libjpeg =
> > %{version}-%{release}    
> 
> It will break some packages silently, some of those packages depend on libjpeg
> utilities which now packages as a subpackage. Among those packages, only
> java-1.6.0-openjdk should fix intermediatly.    

Right now -tools subpackages has Obsoletes: libjpeg < 6b-47, so it'd make sense to add the Provides also to -tools subpackage.

Comment 32 Chen Lei 2010-06-16 02:45:13 UTC
(In reply to comment #31) 
> Right now -tools subpackages has Obsoletes: libjpeg < 6b-47, so it'd make sense
> to add the Provides also to -tools subpackage.    

Provides what? Original libjpeg don't have a -tools subpackage, also it's easy to fix those broken dependencies, I don't think it's a real issue for all of us. Also provides libjpeg in libjpeg-turbo will violate packaging guideline.

Comment 33 Kalev Lember 2010-06-16 03:28:28 UTC
As soname deps are automatically handled by rpm, I bet most of the packages which currently have manual libjpeg Requires do that because they really need one of the tools. That's also why it's libjpeg-turbo-tools that is Obsoleting libjpeg package, as opposed to libjpeg-turbo having the Obsoletes.

Since the libjpeg-turbo-tools package already contains Obsoletes: libjpeg, it'd make sense to add the Provides: libjpeg also to the same libjpeg-turbo-tools package.

Right now -tools subpackage has:
Obsoletes:	libjpeg < 6b-47

To provide a clean upgrade path it should be:
Provides:	libjpeg = 6b-47
Obsoletes:	libjpeg < 6b-47


(In reply to comment #32)
> Also provides libjpeg in libjpeg-turbo will violate packaging guideline.    

Huh, how so? In fact, packaging guidelines [1] suggest to use the following scheme to replace an existing package:
Provides: oldpackagename = $provEVR
Obsoletes: oldpackagename < $obsEVR

[1] http://fedoraproject.org/wiki/PackageNamingGuidelines#Renaming.2Freplacing_existing_packages

Comment 34 Chen Lei 2010-06-16 03:36:33 UTC
(In reply to comment #33)
> As soname deps are automatically handled by rpm, I bet most of the packages
> which currently have manual libjpeg Requires do that because they really need
> one of the tools. That's also why it's libjpeg-turbo-tools that is Obsoleting
> libjpeg package, as opposed to libjpeg-turbo having the Obsoletes.
> 
> Since the libjpeg-turbo-tools package already contains Obsoletes: libjpeg, it'd
> make sense to add the Provides: libjpeg also to the same libjpeg-turbo-tools
> package.
> 
> Right now -tools subpackage has:
> Obsoletes: libjpeg < 6b-47
> 
> To provide a clean upgrade path it should be:
> Provides: libjpeg = 6b-47
> Obsoletes: libjpeg < 6b-47
> 
> 
> (In reply to comment #32)
> > Also provides libjpeg in libjpeg-turbo will violate packaging guideline.    
> 
> Huh, how so? In fact, packaging guidelines [1] suggest to use the following
> scheme to replace an existing package:
> Provides: oldpackagename = $provEVR
> Obsoletes: oldpackagename < $obsEVR
> 
> [1]
> http://fedoraproject.org/wiki/PackageNamingGuidelines#Renaming.2Freplacing_existing_packages    

I already explained this to Adam.
See comments 6 and https://fedoraproject.org/wiki/Upgrade_paths_—_renaming_or_splitting_packages

Comment 35 Kalev Lember 2010-06-16 10:56:19 UTC
The Provides: libjpeg is needed to provide a clean upgrade path for the list of packages in comment #28 which have Requires: libjpeg.

Comment 36 Chen Lei 2010-06-16 11:22:46 UTC
(In reply to comment #35)
> The Provides: libjpeg is needed to provide a clean upgrade path for the list of
> packages in comment #28 which have Requires: libjpeg.    

Upgrade path only applies to end users, rawhide is a place which allow to have broken denpendecies, one week is enough to treat all those broken dependencies.

Comment 37 Kalev Lember 2010-06-16 11:42:04 UTC
Some people are actually using rawhide distribution. You don't need to deliberately create broken deps if it's easy to avoid that.

First create a compatible package, then fix up everything that depends on the compatibility interface, and only after everything is fixed remove the compatibility interface.

The libjpeg-turbo feature page states that no rebuilds are needed to switch to libjpeg-turbo as it is a drop-in replacement. You are now suggesting to make it non-drop-in, why would that be good? Also, the feature page says there is a contingency plan to revert back to libjpeg if there are serious problems. Chan, do you understand that if you quickly rebuild all packages to depend on libjpeg-turbo directly, then it will take another rebuild to revert back to libjpeg if it turns out libjpeg-turbo isn't working out?

Comment 38 Chen Lei 2010-06-16 11:57:25 UTC
(In reply to comment #37)
> Some people are actually using rawhide distribution. You don't need to
> deliberately create broken deps if it's easy to avoid that.
> First create a compatible package, then fix up everything that depends on the
> compatibility interface, and only after everything is fixed remove the
> compatibility interface.
> The libjpeg-turbo feature page states that no rebuilds are needed to switch to
> libjpeg-turbo as it is a drop-in replacement. You are now suggesting to make it
> non-drop-in, why would that be good? Also, the feature page says there is a
> contingency plan to revert back to libjpeg if there are serious problems. Chan,
> do you understand that if you quickly rebuild all packages to depend on
> libjpeg-turbo directly, then it will take another rebuild to revert back to
> libjpeg if it turns out libjpeg-turbo isn't working out?    

Only 5 packages need fix, among those packages only openjdk need a quick fix.
For openjdk, the fix is simply removing explict depencenies on libjpeg, we don't need another rebuild if we revert back to libjpeg.

"yum install libjpeg" will be silly if we add provides libjpeg to libjpeg-turbo-tools. I don't want to discuss this issue again unless fedora packaging guideline recommends to do so.

Comment 39 Adam Tkac 2010-06-16 16:41:12 UTC
(In reply to comment #27)
> Issues:
> 1. BR: nasm should be conditional
> 2. It'll be better to add more files to %doc
> 3. I suggest you to change -tools subpackage to -utils

I incorporated all your suggestions to the new srpm + spec:
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo-0.0.93-8.fc14.src.rpm
http://atkac.fedorapeople.org/libjpeg-turbo/libjpeg-turbo.spec

Comment 40 Chen Lei 2010-06-17 03:23:38 UTC
Okay, this package is approved.

1.Before importing libjpeg-turbo to cvs, you need to notice java-1.6.0-openjdk maintainer to remove explict libjpeg dependency first.

rpm -qR java-1.6.0-openjdk|grep jpeg
libjpeg = 6b
libjpeg.so.62()(64bit)  



2.Some trival improvement:
It will be better to move wizard.doc from -devel to -utils, 

%package utils
Summary:	Tools for manipulating JPEG images
->
%package utils
Summary:	Utilities for manipulating JPEG images

Comment 41 Adam Tkac 2010-06-24 11:20:26 UTC
(In reply to comment #40)
> Okay, this package is approved.

Thanks for the review.

> 1.Before importing libjpeg-turbo to cvs, you need to notice java-1.6.0-openjdk
> maintainer to remove explict libjpeg dependency first.
> 
> rpm -qR java-1.6.0-openjdk|grep jpeg
> libjpeg = 6b
> libjpeg.so.62()(64bit)  

I just filled bug #607554.

> 2.Some trival improvement:
> It will be better to move wizard.doc from -devel to -utils, 
> 
> %package utils
> Summary: Tools for manipulating JPEG images
> ->
> %package utils
> Summary: Utilities for manipulating JPEG images    

Ok, I will incorporate your suggestions to the first build of libjpeg-turbo.

Comment 42 Adam Tkac 2010-06-24 11:22:20 UTC
New Package CVS Request
=======================
Package Name: libjpeg-turbo
Short Description: A MMX/SSE2 accelerated library for manipulating JPEG image files
Owners: atkac
Branches: devel
InitialCC:

Comment 43 Rex Dieter 2010-06-25 16:48:40 UTC
Comment #40 could be mitigated if libjpeg-turbo also included:
Provides: libjpeg = 6b-47

Comment 44 Rex Dieter 2010-06-25 16:52:20 UTC
oh, and you're doing that, may as well also
Provides: libjpeg%{?_isa} = 6b-47

Comment 45 Jason Tibbitts 2010-06-26 07:56:28 UTC
CVS done (by process-cvs-requests.py).

Comment 46 Adam Tkac 2010-06-28 12:55:29 UTC
(In reply to comment #43)
> Comment #40 could be mitigated if libjpeg-turbo also included:
> Provides: libjpeg = 6b-47    

This is not needed, as explained in comment #6.

Comment 47 Adam Tkac 2010-06-28 12:57:02 UTC
libjpeg-turbo-0.0.93-9.fc14 has been successfully built, thanks for the review. Closing.

Comment 48 Rex Dieter 2010-06-28 14:12:54 UTC
I'll respectfully disagree with comment #46 ( and comment #6 ) and argue that the Provides should be included, at least temporarily while testing is underway.

Else, it will be impossible to truly test swapping between libjpeg/libjpeg-turbo without breaking dependencies.

Comment 49 Chen Lei 2010-06-28 14:35:24 UTC
(In reply to comment #48)
> I'll respectfully disagree with comment #46 ( and comment #6 ) and argue that
> the Provides should be included, at least temporarily while testing is
> underway.
> Else, it will be impossible to truly test swapping between
> libjpeg/libjpeg-turbo without breaking dependencies.    

Hi Rex,

The situation is bit complicated, libjpeg is split into libjpeg-turbo and libjpeg-turbo-utils now. 
5 packages will be broken after building libjpeg-turbo in koji, if we provide libjpeg in libjpeg-turbo, then we'll break some packages silently because they actually depend on libjpeg-turbo-utils instead of linking with libjpeg. Providing libjpeg in libjpeg-turbo-utils can solve all broken dependecies, but the result will be a bit strange when we type "yum install libjpeg". 
5 packages is not a large amount, also we only need to fix openjdk once, rpmbuild already add libjpeg.so.62 dependency to openjdk automatically, we don't need to do it again if fedora 14 revert to use libjpeg.

repoquery --whatrequires  --exactdeps libjpeg
libjpeg-0:6b-46.fc12.x86_64
libjpeg-0:6b-46.fc12.i686
gallery2-jpegtran-0:2.3.1-1.fc13.noarch
java-1.6.0-openjdk-1:1.6.0.0-37.b17.fc13.x86_64
gocr-0:0.48-1.fc13.x86_64
darkplaces-0:20091001-2.fc13.x86_64
libjpeg-devel-0:6b-46.fc12.x86_64
libjpeg-devel-0:6b-46.fc12.i686
renrot-0:1.1-1.fc13.2.noarch
java-1.6.0-openjdk-1:1.6.0.0-39.b18.fc13.x86_64

Comment 50 Rex Dieter 2010-06-28 15:03:10 UTC
Your way is a little harder and could lead to more churn.  If you and the maintainers involved don't mind that... so be it.

Comment 51 Chen Lei 2010-06-28 16:42:53 UTC
(In reply to comment #50)
> Your way is a little harder and could lead to more churn.  If you and the
> maintainers involved don't mind that... so be it.    

Then, which way will be more appropriate? Adding Provides: libjpeg = 6b to libjpeg-turbo or libjpeg-turbo-utils?

Comment 52 Rex Dieter 2010-06-28 17:21:31 UTC
either would be fine for testing purposes, but I suppose adding it to -utils (to match the Obsoletes) would be more appropriate


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