Remove incorrect usages of USE_SYSTEM_SQLITE.

Remove incorrect usages of USE_SYSTEM_SQLITE.

Currently we don't provide any support to the system sqlite.

BUG=None
TEST=trybots

Signed-off-by: Thiago Farina <tfarina@chromium.org>;

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

[tfarina, 2010-07-18 00:45:41]

Scott, may you have a look? Is that what you were purposing?

[Scott Hess - ex-Googler, 2010-07-18 02:05:02]

On 2010/07/18 00:45:41, tfarina wrote:
> Scott, may you have a look? Is that what you were purposing?

This is what I was thinking, but while I feel it is generally reasonable, I
don't feel qualified to speak to the intentions for app/sql/.  I'm concerned
that the goal of app/sql/ is to discourage direct sqlite calls in the first
place.  Looping in brettw for some color on that.

If it's going to be in app/sql/, then you should change the app/sql/*.cc files
appropriately so that everyone is in sync.  Also missed:

chrome/browser/diagnostics/sqlite_diagnostics.cc
chrome/browser/sync/syncable/directory_backing_store.cc
chrome/browser/sync/syncable/syncable_unittest.cc

[Actually, to flush out all cases it might be easiest to put a temporary #error
line in third_party/sqlite/preprocessed/sqlite3.h, guarded by an #ifndef for
something defined in app/sql/sqlite3.h, then rebuild.  The compiler should find
all the files which need to be converted for you.]

[brettw, 2010-07-18 16:56:30]

We don't support the system sqlite (which you can see from the
includes used in app/sql). So I think the right solution would be to
just remove the ifdefs and include the one in third_party.

Half of these files look like they don't use sqlite at all. Can you
try removing it first and only keep it if it's necessary?

Brett

On Sat, Jul 17, 2010 at 7:05 PM,  <shess@chromium.org> wrote:
> On 2010/07/18 00:45:41, tfarina wrote:
>>
>> Scott, may you have a look? Is that what you were purposing?
>
> This is what I was thinking, but while I feel it is generally reasonable, I
> don't feel qualified to speak to the intentions for app/sql/.  I'm concerned
> that the goal of app/sql/ is to discourage direct sqlite calls in the first
> place.  Looping in brettw for some color on that.
>
> If it's going to be in app/sql/, then you should change the app/sql/*.cc
> files
> appropriately so that everyone is in sync.  Also missed:
>
> chrome/browser/diagnostics/sqlite_diagnostics.cc
> chrome/browser/sync/syncable/directory_backing_store.cc
> chrome/browser/sync/syncable/syncable_unittest.cc
>
> [Actually, to flush out all cases it might be easiest to put a temporary
> #error
> line in third_party/sqlite/preprocessed/sqlite3.h, guarded by an #ifndef for
> something defined in app/sql/sqlite3.h, then rebuild.  The compiler should
> find
> all the files which need to be converted for you.]
>
> http://codereview.chromium.org/3049001/show
>

[tfarina, 2010-07-18 18:51:04]

On 2010/07/18 16:56:30, brettw wrote:
> We don't support the system sqlite (which you can see from the
> includes used in app/sql). So I think the right solution would be to
> just remove the ifdefs and include the one in third_party.
>
Brett I removed the usages of USE_SYSTEM_SQLITE. I left the define in the
sqlite3.gyp because I'm not familiar with gyp, so I didn't touch there. 

> Half of these files look like they don't use sqlite at all. Can you
> try removing it first and only keep it if it's necessary?
> 
I tried, but some of there are really using sqlite3.

Brett, Scott, please take another look.

[brettw, 2010-07-18 22:18:06]

LGTM, thanks. I thought the files weren't using it since they weren't calling
sqlite3_* functions. But it looks like those files need some of the #defines
fromsqlite3.h, so I was mistaken that the includes could be removed.

[Scott Hess - ex-Googler, 2010-07-19 13:49:04]

On 2010/07/18 18:51:04, tfarina wrote:
> On 2010/07/18 16:56:30, brettw wrote:
> > We don't support the system sqlite (which you can see from the
> > includes used in app/sql). So I think the right solution would be to
> > just remove the ifdefs and include the one in third_party.
> >
> Brett I removed the usages of USE_SYSTEM_SQLITE. I left the define in the
> sqlite3.gyp because I'm not familiar with gyp, so I didn't touch there. 

See, glad we pulled in Brett, because I didn't realize we had deprecated the use
of USE_SYSTEM_SQLITE.  In fact, I'm still kind of unconvinced.  Brett, were you
at fault for that in the first place, or should we loop in Evan or someone, just
to be sure?  I don't want to see these creeping back in piecemeal, if we intend
them to be gone.

> > Half of these files look like they don't use sqlite at all. Can you
> > try removing it first and only keep it if it's necessary?
> > 
> I tried, but some of there are really using sqlite3.
> 
> Brett, Scott, please take another look.

You're still missing the files I referenced a couple comments back.

[tfarina, 2010-07-19 16:34:22]

On 2010/07/19 13:49:04, shess wrote:
> You're still missing the files I referenced a couple comments back.
What I'm missing? These files are including #include
"third_party/sqlite/preprocessed/sqlite3.h".

[Scott Hess - ex-Googler, 2010-07-19 17:13:34]

Apologies - I wasn't keeping the context of my earlier post in mind.

LGTM.

On Mon, Jul 19, 2010 at 9:34 AM,  <tfarina@chromium.org> wrote:
> On 2010/07/19 13:49:04, shess wrote:
>>
>> You're still missing the files I referenced a couple comments back.
>
> What I'm missing? These files are including #include
> "third_party/sqlite/preprocessed/sqlite3.h".
>
>
>
> http://codereview.chromium.org/3049001/show
>