Use generated shim headers for zlib.

Use generated shim headers for zlib.

BUG=165264
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173801

[Paweł Hajdan Jr., 2012-12-18 19:16:10]

Note that I had to move the include dir back to SHARED_INTERMEDIATE_DIR, because
otherwise INTERMEDIATE_DIR expanded to a different path in the dependent target.
I've added _target_name to the path though, to address the original problem.

By the way, the shim_headers path does not appear *first* in the include path
despite the 'include_dirs+' (the plus at the end). Is that expected?

[Mark Mentovai, 2012-12-18 19:48:17]

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi
File build/shim_headers.gypi (right):

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi#ne...
build/shim_headers.gypi:16: 'include_dirs+': [
You probably need one more plus here to have the effect you want:

 - one plus refers to the merge of this gypi file into whatever target includes
it
 - one plus refers to the merge of the direct_dependent_settings section into
whatever target depends on the target that this is moved into.

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi#ne...
build/shim_headers.gypi:16: 'include_dirs+': [
Perhaps you’d also like to add 'include_dirs+' outside of
direct_dependent_settings, with one plus fewer than in
direct_dependent_settings, so that anything in the target that includes this
gypi file gets the proper shim header directory in its own include path.

https://codereview.chromium.org/11632002/diff/3001/skia/config/SkUserConfig.h
File skia/config/SkUserConfig.h (right):

https://codereview.chromium.org/11632002/diff/3001/skia/config/SkUserConfig.h...
skia/config/SkUserConfig.h:113: #define SK_ZLIB_INCLUDE
"third_party/zlib/zlib.h"
#include MACRO is usually hard on preprocessors and breaks optimizations that
certain tools that try to speed things up try to use (like distcc-pump and other
things that do a “light” preprocessing, maybe goma too). If this is not
absolutely necessary now, we should just switch to #including at this path
directly.

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
File third_party/zlib/contrib/minizip/mztools.h (right):

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
third_party/zlib/contrib/minizip/mztools.h:14: #ifndef _ZLIB_H
Is this (and the __cplusplus) in the upstream file? Because zlib.h seems to have
its own #include guards and extern "C".

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
File third_party/zlib/contrib/minizip/unzip.h (right):

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
third_party/zlib/contrib/minizip/unzip.h:50: #ifndef _ZLIB_H
Same question.

https://codereview.chromium.org/11632002/diff/3001/ui/gfx/codec/png_codec.cc
File ui/gfx/codec/png_codec.cc (right):

https://codereview.chromium.org/11632002/diff/3001/ui/gfx/codec/png_codec.cc#...
ui/gfx/codec/png_codec.cc:18: #include "third_party/zlib/zlib.h"
Same question here.

https://codereview.chromium.org/11632002/diff/3001/webkit/support/webkit_supp...
File webkit/support/webkit_support_gfx.cc (right):

https://codereview.chromium.org/11632002/diff/3001/webkit/support/webkit_supp...
webkit/support/webkit_support_gfx.cc:13: #include "third_party/zlib/zlib.h"
And here.

[Paweł Hajdan Jr., 2012-12-18 20:57:40]

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi
File build/shim_headers.gypi (right):

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi#ne...
build/shim_headers.gypi:16: 'include_dirs+': [
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> You probably need one more plus here to have the effect you want:
> 
>  - one plus refers to the merge of this gypi file into whatever target
includes
> it
>  - one plus refers to the merge of the direct_dependent_settings section into
> whatever target depends on the target that this is moved into.

Done.

https://codereview.chromium.org/11632002/diff/3001/build/shim_headers.gypi#ne...
build/shim_headers.gypi:16: 'include_dirs+': [
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> Perhaps you’d also like to add 'include_dirs+' outside of
> direct_dependent_settings, with one plus fewer than in
> direct_dependent_settings, so that anything in the target that includes this
> gypi file gets the proper shim header directory in its own include path.

Done.

https://codereview.chromium.org/11632002/diff/3001/skia/config/SkUserConfig.h
File skia/config/SkUserConfig.h (right):

https://codereview.chromium.org/11632002/diff/3001/skia/config/SkUserConfig.h...
skia/config/SkUserConfig.h:113: #define SK_ZLIB_INCLUDE
"third_party/zlib/zlib.h"
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> #include MACRO is usually hard on preprocessors and breaks optimizations that
> certain tools that try to speed things up try to use (like distcc-pump and
other
> things that do a “light” preprocessing, maybe goma too). If this is not
> absolutely necessary now, we should just switch to #including at this path
> directly.

This is some skia mess, see e.g. third_party/skia/gyp/zlib.gyp . Changing this
now would make the landing pretty risky.

I'm fine with adding a TODO and doing that later. However, that would make us
use shim headers at least on iOS. That's fine for me, but some people might not
like it.

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
File third_party/zlib/contrib/minizip/mztools.h (right):

https://codereview.chromium.org/11632002/diff/3001/third_party/zlib/contrib/m...
third_party/zlib/contrib/minizip/mztools.h:14: #ifndef _ZLIB_H
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> Is this (and the __cplusplus) in the upstream file? Because zlib.h seems to
have
> its own #include guards and extern "C".

Yes to both.

https://codereview.chromium.org/11632002/diff/3001/ui/gfx/codec/png_codec.cc
File ui/gfx/codec/png_codec.cc (right):

https://codereview.chromium.org/11632002/diff/3001/ui/gfx/codec/png_codec.cc#...
ui/gfx/codec/png_codec.cc:18: #include "third_party/zlib/zlib.h"
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> Same question here.

Done.

https://codereview.chromium.org/11632002/diff/3001/webkit/support/webkit_supp...
File webkit/support/webkit_support_gfx.cc (right):

https://codereview.chromium.org/11632002/diff/3001/webkit/support/webkit_supp...
webkit/support/webkit_support_gfx.cc:13: #include "third_party/zlib/zlib.h"
On 2012/12/18 19:48:17, Mark Mentovai wrote:
> And here.

Done.

[Mark Mentovai, 2012-12-18 21:17:56]

LGTM if it works

https://codereview.chromium.org/11632002/diff/13001/build/shim_headers.gypi
File build/shim_headers.gypi (right):

https://codereview.chromium.org/11632002/diff/13001/build/shim_headers.gypi#n...
build/shim_headers.gypi:15: 'include_dirs+++': [
Surprised it took this many pluses.