Remove misleading support for (use_system_libjpeg == 1 and use_libjpeg_turbo == 1)

Abandonned

[Steve Block, 2012-07-02 18:27:32]

Hi Hironori,

It looks to me like (use_system_libjpeg == 1 and use_libjpeg_turbo == 1) is
never used, so I was wondering what you think about removing it for clarity?

If it is needed, I think it would be good to add a comment to make clear exactly
what this combination of flags does. Also, the use_system_libjpeg settings are
currently duplicated by the libjpeg and libjpeg-turbo gyp files, so it would be
good to consolidate them to a central location.

[Steve Block, 2012-07-02 18:55:20]

http://codereview.chromium.org/10699059/diff/1/third_party/libjpeg_turbo/libj...
File third_party/libjpeg_turbo/libjpeg.gyp (right):

http://codereview.chromium.org/10699059/diff/1/third_party/libjpeg_turbo/libj...
third_party/libjpeg_turbo/libjpeg.gyp:17: {
This looks horrendous, but is just an indentation change!

[Hironori Bono, 2012-07-03 00:38:06]

Greetings Steve,

Even though this change looks good for me, I would like to hear thoughts of
Pawel, who requested this variable.

Regards,

Hironori Bono

[Steve Block, 2012-07-10 10:07:34]

+evan

Hi Evan, sorry to bug you now you're no longer on Chrome. It looks like Pawel is
on vacation for some time and Hironori mentioned that you might have some
context on whether it's OK to remove this combination of flags.

For background, http://src.chromium.org/viewvc/chrome?view=rev&revision=71326
changed the configuration to statically link libjpeg_turbo on Linux after
http://code.google.com/p/chromium/issues/detail?id=31427.
http://src.chromium.org/viewvc/chrome?view=rev&revision=139754 then changed it
so that all builds statically link libjpeg_turbo.

This means that as far as I can tell, nobody is using use_system_libjpeg with
libjpeg_turbo.

[Evan Martin, 2012-07-10 16:26:55]

Pawel just mailed me yesterday so maybe he'll chime in, ping me again if he
doesn't.


On Tue, Jul 10, 2012 at 3:07 AM, <steveblock@chromium.org> wrote:

> +evan
>
> Hi Evan, sorry to bug you now you're no longer on Chrome. It looks like
> Pawel is
> on vacation for some time and Hironori mentioned that you might have some
> context on whether it's OK to remove this combination of flags.
>
> For background, http://src.chromium.org/**viewvc/chrome?view=rev&**
> revision=71326<http://src.chromium.org/viewvc/chrome?view=rev&revision=71326>
> changed the configuration to statically link libjpeg_turbo on Linux after
>
http://code.google.com/p/**chromium/issues/detail?id=**31427<http://code.goog...
> .
>
http://src.chromium.org/**viewvc/chrome?view=rev&**revision=139754<http://src...
changed it
> so that all builds statically link libjpeg_turbo.
>
> This means that as far as I can tell, nobody is using use_system_libjpeg
> with
> libjpeg_turbo.
>
>
http://codereview.chromium.**org/10699059/<http://codereview.chromium.org/106...
>

[Steve Block, 2012-07-12 17:57:12]

Still no reply from Pawel, so if you could take a look, or recommend a reviewer,
that would be great. Thanks!

[Evan Martin, 2012-07-12 18:09:56]

Your CL description mentions use_libjpeg_turbo but I don't see it anywhere in
the diff.  What am I missing?

[Steve Block, 2012-07-12 18:18:04]

Thanks Evan

This gyp file is specifically for libjpeg_turbo. Logic in build/common.gypi sets
libjpeg_gyp_path based on the value of use_libjpeg_turbo to pull in either this
gyp file, or the gyp file for libjpeg (third_party/libjpeg/libjpeg.gyp).

[Evan Martin, 2012-07-12 18:24:54]

On 2012/07/12 18:18:04, Steve Block wrote:
> Thanks Evan
> 
> This gyp file is specifically for libjpeg_turbo. Logic in build/common.gypi
sets
> libjpeg_gyp_path based on the value of use_libjpeg_turbo to pull in either
this
> gyp file, or the gyp file for libjpeg (third_party/libjpeg/libjpeg.gyp).

I think I follow.  Isn't libjpeg-turbo intended to be a drop-in replacement for
libjpeg?  It would seem that using the system libjpeg, whatever it is, is the
right thing to do if use_system_libjpeg is specified.

(I am pretty ignorant of this code, so I'm trying to channel Pawel here.)

[Steve Block, 2012-07-12 18:41:32]

> Isn't libjpeg-turbo intended to be a drop-in replacement for
> libjpeg?
Right

> It would seem that using the system libjpeg, whatever it is, is the
> right thing to do if use_system_libjpeg is specified.
That wasn't my expectation. My understanding is that the use_system_flags are
offered as an optimisation, whereby the resulting behaviour is unchanged but we
avoid having to link a static library. libjpeg_turbo provides a performance
improvement over lijpeg, so using the system libjpeg in it's place doesn't offer
the same optimisation. So I would expect use_system_libjpeg to be ignored if
we're using libjpeg_turbo.

My motivation for doing this is that I'm trying to add support for
use_system_libjpeg on Android. (use_system_libjpeg == 1 and use_libjpeg_turbo ==
1) is currently unused and removing it allows the logic around these flags to be
simplified.

[Evan Martin, 2012-07-12 18:43:36]

> That wasn't my expectation. My understanding is that the use_system_flags are
> offered as an optimisation, whereby the resulting behaviour is unchanged but
we
> avoid having to link a static library.

They're offered for systems where bundling libjpeg isn't an option.
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

These flags are only used on systems where the people presumably know what
they're doing (which is why the flags are off by default).

I guess what I'm saying is that it seems fine to me to rejigger these if you see
some better way to do it, but there should still be a way for these people to
use the system libjpeg-turbo.

[Steve Block, 2012-07-12 18:47:00]

> They're offered for systems where bundling libjpeg isn't an option.
Ah, OK, then my assumption was incorrect. Thanks for clarifying. My motivations
on Android are a little different.

I'll look into simplifying this logic, as mentioned in #msg1.

[Steve Block, 2012-07-13 14:37:23]

See http://codereview.chromium.org/10696082