define USE_FILE32API on Android too

define USE_FILE32API on Android too

BUG=
TEST=compile

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=115121

[Brad Chen, 2011-12-18 05:29:15]

LGTM

On Sat, Dec 17, 2011 at 9:19 PM, <jnd@chromium.org> wrote:

> Reviewers: agl, Brad Chen (chromium),
>
> Description:
> define USE_FILE32API on Android too
>
> BUG=
> TEST=compile
>
> Please review this at
http://codereview.chromium.**org/8987002/<http://codereview.chromium.org/8987...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>  M     third_party/zlib/zlib.gyp
>
>
> Index: third_party/zlib/zlib.gyp
> ==============================**==============================**=======
> --- third_party/zlib/zlib.gyp   (revision 114935)
> +++ third_party/zlib/zlib.gyp   (working copy)
> @@ -71,9 +71,10 @@
>                 'contrib/minizip/iowin32.c'
>               ],
>             }],
> -            ['OS=="mac" or os_bsd==1', {
> -              # Mac and the BSDs don't have fopen64, ftello64, or
> fseeko64.
> -              # We use fopen, ftell, and fseek instead on these systems.
> +            ['OS=="mac" or os_bsd==1 or OS=="android"', {
> +              # Mac, Android and the BSDs don't have fopen64, ftello64, or
> +              # fseeko64. We use fopen, ftell, and fseek instead on these
> +              # systems.
>               'defines': [
>                 'USE_FILE32API'
>               ],
>
>
>

[Johnny(Jianning) Ding, 2011-12-18 05:57:43]

Hi Brad,

Thanks for quick response.
I just found we need to define USE_FILE32API when use_system_zlib==1 as well,
it's because contrib/minizip/ioapi.c may use file 64bits APIs.

I have uploaded patch 2, please reexamine.

[Brad Chen (chromium), 2011-12-18 16:17:44]

With these changes it looks like this CL could impact Mac and BSD builds, not
just Android. Did you test on those systems? Try jobs?

On 2011/12/18 05:57:43, Johnny(Jianning) Ding wrote:
> Hi Brad,
> 
> Thanks for quick response.
> I just found we need to define USE_FILE32API when use_system_zlib==1 as well,
> it's because contrib/minizip/ioapi.c may use file 64bits APIs.
> 
> I have uploaded patch 2, please reexamine.

[Johnny(Jianning) Ding, 2011-12-19 06:42:20]

Tried on mac_rel, win_rel, linux_rel, linux_shared, all passed except ui_test
was failed on mac_rel, but according to the log, that should be fault of this
change. Please refer to the following try recorders.

http://build.chromium.org/p/tryserver.chromium/builders/mac_rel/builds/5457
http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/5735
http://build.chromium.org/p/tryserver.chromium/builders/linux_rel/builds/5339
http://build.chromium.org/p/tryserver.chromium/builders/linux_shared/builds/812

On 2011/12/18 16:17:44, Brad Chen (chromium) wrote:
> With these changes it looks like this CL could impact Mac and BSD builds, not
> just Android. Did you test on those systems? Try jobs?
> 
> On 2011/12/18 05:57:43, Johnny(Jianning) Ding wrote:
> > Hi Brad,
> > 
> > Thanks for quick response.
> > I just found we need to define USE_FILE32API when use_system_zlib==1 as
well,
> > it's because contrib/minizip/ioapi.c may use file 64bits APIs.
> > 
> > I have uploaded patch 2, please reexamine.

[Hironori Bono, 2011-12-19 06:51:26]

Greetings,

Thank you for your build fix.
Just our of curiosity, does Android use the system zlib or our copy of zlib?

Regards,

Hironori Bono

[Johnny(Jianning) Ding, 2011-12-19 11:45:37]

Hi Bono-san,
The android build currently uses system zlib.

On 2011/12/19 06:51:26, Hironori Bono wrote:
> Greetings,
> 
> Thank you for your build fix.
> Just our of curiosity, does Android use the system zlib or our copy of zlib?
> 
> Regards,
> 
> Hironori Bono

[Hironori Bono, 2011-12-20 00:20:20]

Greetings,

Many thanks for your explanation.
This change looks good to me and please feel free to commit it. :)

Regards,

Hironori Bono

[Johnny(Jianning) Ding, 2011-12-20 03:47:58]

On 2011/12/19 06:42:20, Johnny(Jianning) Ding wrote:
> Tried on mac_rel, win_rel, linux_rel, linux_shared, all passed except ui_test
> was failed on mac_rel, but according to the log, that should be fault of this
Sorry for typo here, should -> should not
> change. Please refer to the following try recorders.
> 
> http://build.chromium.org/p/tryserver.chromium/builders/mac_rel/builds/5457
> http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/5735
> http://build.chromium.org/p/tryserver.chromium/builders/linux_rel/builds/5339
>
http://build.chromium.org/p/tryserver.chromium/builders/linux_shared/builds/812
> 
> On 2011/12/18 16:17:44, Brad Chen (chromium) wrote:
> > With these changes it looks like this CL could impact Mac and BSD builds,
not
> > just Android. Did you test on those systems? Try jobs?
> > 
> > On 2011/12/18 05:57:43, Johnny(Jianning) Ding wrote:
> > > Hi Brad,
> > > 
> > > Thanks for quick response.
> > > I just found we need to define USE_FILE32API when use_system_zlib==1 as
> well,
> > > it's because contrib/minizip/ioapi.c may use file 64bits APIs.
> > > 
> > > I have uploaded patch 2, please reexamine.

[Johnny(Jianning) Ding, 2011-12-20 03:49:04]

Thanks, Bono-san!

Hi Brad,any comment?
On 2011/12/20 00:20:20, Hironori Bono wrote:
> Greetings,
> 
> Many thanks for your explanation.
> This change looks good to me and please feel free to commit it. :)
> 
> Regards,
> 
> Hironori Bono

[Brad Chen (chromium), 2011-12-20 11:56:44]

LGTM

On 2011/12/20 03:49:04, Johnny(Jianning) Ding wrote:
> Thanks, Bono-san!
> 
> Hi Brad,any comment?
> On 2011/12/20 00:20:20, Hironori Bono wrote:
> > Greetings,
> > 
> > Many thanks for your explanation.
> > This change looks good to me and please feel free to commit it. :)
> > 
> > Regards,
> > 
> > Hironori Bono

[commit-bot: I haz the power, 2011-12-20 12:12:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jnd@chromium.org/8987002/2002

[commit-bot: I haz the power, 2011-12-20 13:29:36]

Change committed as 115121