GYP changes to get sql and sqlite building on iOS.

GYP changes to get sql and sqlite building on iOS.

BUG=None
TEST=None


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

[rohitrao (ping after 24h), 2012-07-13 11:56:35]

These are the logically smallest changes I can make, but they lead to an ugly
gyp file.  See if you can think of a better way to rewrite this.

[stuartmorgan, 2012-07-13 12:18:10]

http://codereview.chromium.org/10696219/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10696219/diff/1/build/common.gypi#newcode836
build/common.gypi:836: 'use_system_sqlite': 1,
Put this in the existing section a bit further down where we disable_nacl (it's
still the same scope)

http://codereview.chromium.org/10696219/diff/1/third_party/sqlite/sqlite.gyp
File third_party/sqlite/sqlite.gyp (right):

http://codereview.chromium.org/10696219/diff/1/third_party/sqlite/sqlite.gyp#...
third_party/sqlite/sqlite.gyp:83: }, { # else: os_posix == 1 or OS == "mac" or !
use_system_sqlite
As long as you are here, s/1/0/ so this comment stops being a lie :)

Although, part of what makes this whole set of conditions so horrible is that it
makes no sense that if, say mac set use_system_sqlite=1, it would actually end
up in this path and silently just build from scratch. That's terrible.

How we structure the conditions in this target as
[ use_system_sqlite, {
  < direct_dependent_settings -> define USE_SYSTEM_SQLITE >
  'conditions': [
    [ OS == "ios", {
      < the ios-specific stuff >
    }],
    [ os_posix and OS != mac and OS != ios, {
      < the linux stuff >
    }],
  ]
}, {
  < everything that's currently below this line>
}]

That way it's clear that mac + use_system_sqlite isn't currently implemented.
Also, doing a direct_dependent_settings again in the Linux part to add cflags
avoids the confusing use of iOS checks in two different but similar places.

What do you think?

[rohitrao (ping after 24h), 2012-07-13 12:51:54]

I think this is equivalent to the original, plus iOS support.  Thoughts?

[stuartmorgan, 2012-07-13 13:17:33]

I find this *way* more readable; LGTM! Hopefully it's not just me :)

[rohitrao (ping after 24h), 2012-07-13 13:22:21]

+Pawel, in case he wants to take a look.  I think he wrote most of this file
originally.

[stuartmorgan, 2012-07-24 15:26:17]

LGTM still with one question in common.gypi

http://codereview.chromium.org/10696219/diff/18001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10696219/diff/18001/build/common.gypi#newcode881
build/common.gypi:881: 'use_system_sqlite': 1,
Will this work with a % at the end? I think it should, and that's preferred (so
that it's externally overrideable)

[rohitrao (ping after 24h), 2012-07-24 16:10:46]

Pawel, I'm going to go ahead and CQ this.  Please feel free to do a post-commit
driveby if you see things that you'd like cleaned up.

http://codereview.chromium.org/10696219/diff/18001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10696219/diff/18001/build/common.gypi#newcode881
build/common.gypi:881: 'use_system_sqlite': 1,
On 2012/07/24 15:26:17, stuartmorgan wrote:
> Will this work with a % at the end? I think it should, and that's preferred
(so
> that it's externally overrideable)

Done.

[commit-bot: I haz the power, 2012-07-24 16:11:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rohitrao@chromium.org/10696219/21001

[commit-bot: I haz the power, 2012-07-24 18:03:48]

Change committed as 148136

[Paweł Hajdan Jr., 2012-08-29 14:46:48]

LGTM

This is fine, just one thing that might affect you: we have custom modifications
that add chromium_sqlite3_... functions to sqlite. If your system copy doesn't
have those modifications, I don't know how it works for you. You may want to
take a closer look.

[stuartmorgan, 2012-08-29 15:56:55]

On 2012/08/29 14:46:48, Paweł Hajdan Jr. wrote:
> This is fine, just one thing that might affect you: we have custom
modifications
> that add chromium_sqlite3_... functions to sqlite.

Looks like they are only used from WebCore, which we don't (can't) use.

[Paweł Hajdan Jr., 2012-08-29 16:42:17]

On 2012/08/29 15:56:55, stuartmorgan wrote:
> On 2012/08/29 14:46:48, Paweł Hajdan Jr. wrote:
> > This is fine, just one thing that might affect you: we have custom
> modifications
> > that add chromium_sqlite3_... functions to sqlite.
> 
> Looks like they are only used from WebCore, which we don't (can't) use.

Ah right, totally LGTM then. :)