Move app/sql/* files to sql/ directory.

Move app/sql/* files to sql/ directory.

I can't remove app/app.gyp and app/app_base.gypi yet because they are referenced
by third_party gyp files :(

BUG=72317
TEST=None

R=rsesek@chromium.org

move app/sql to sql

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

[Robert Sesek, 2011-07-13 18:55:50]

Two questions:

1) What in third_party depends on this?

2) Does chrome/DEPS need to be updated?

http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
File chrome/browser/history/history_database.h (right):

http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
chrome/browser/history/history_database.h:9: #include "sql/connection.h"
alphabetize?

[tfarina, 2011-07-13 20:12:40]

On Wed, Jul 13, 2011 at 3:55 PM,  <rsesek@chromium.org> wrote:
> Two questions:
>
> 1) What in third_party depends on this?
>
third_party/WebKit/Source/WebKit/chromium/WebKit.gyp +71

> 2) Does chrome/DEPS need to be updated?
>
Yes.

>
>
http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
> File chrome/browser/history/history_database.h (right):
>
>
http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
> chrome/browser/history/history_database.h:9: #include "sql/connection.h"
> alphabetize?
>
> http://codereview.chromium.org/7353026/
>



-- 
Thiago

[tfarina, 2011-07-13 20:24:39]

Please, take another look.

http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
File chrome/browser/history/history_database.h (right):

http://codereview.chromium.org/7353026/diff/1102/chrome/browser/history/histo...
chrome/browser/history/history_database.h:9: #include "sql/connection.h"
On 2011/07/13 18:55:51, rsesek wrote:
> alphabetize?

Done.

[Robert Sesek, 2011-07-13 22:35:03]

So this breaks upstream because WebKit pulls in /src/app and depends on the sql/
files being there. It looks like this may have to be a two-sided patch.

Specifically:
The following build commands failed:
	CompileC
../../xcodebuild/webkit_support.build/Debug/appcache.build/Objects-normal/i386/appcache_database.o
../appcache/appcache_database.cc normal i386 c++ com.apple.compilers.gcc.4_2
	CompileC
../../xcodebuild/webkit_support.build/Debug/appcache.build/Objects-normal/i386/appcache_storage_impl.o
../appcache/appcache_storage_impl.cc normal i386 c++ com.apple.compilers.gcc.4_2

+jamesr for help

[jamesr, 2011-07-13 22:52:12]

Yeah, I think you'll have to do this as a two-sided patch:

1.) Move the files to their new location, but leave them behind in the old
location so that the WebKit config keeps building
2.) Update Source/WebKit/chromium/DEPS in the WebKit tree to point to the new
location for /app stuff
3.) Delete the stuff from its old location.

It might be that you just have to leave app.gyp behind, or you might have to
leave a bunch of headers behind.

[Robert Sesek, 2011-07-18 20:21:33]

On 2011/07/13 22:52:12, jamesr wrote:
> Yeah, I think you'll have to do this as a two-sided patch:
> 
> 1.) Move the files to their new location, but leave them behind in the old
> location so that the WebKit config keeps building

James, can you explain (1)? FWICT, WebKit DEPS are tied to a specific Chromium
revision. So can't the steps be:

1. Land this change as-is
2. Create a WebKit change to roll DEPS, perform any updates, and remove app/
from DEPS

Assuming that (2) is done shortly afterwards? I don't think anything in upstream
actually depends on the files. It pulls in webkit/ which uses sql/, so just
rolling DEPS and updating a .gyp file should be sufficient.

[jamesr, 2011-07-18 20:26:05]

On 2011/07/18 20:21:33, rsesek wrote:
> On 2011/07/13 22:52:12, jamesr wrote:
> > Yeah, I think you'll have to do this as a two-sided patch:
> > 
> > 1.) Move the files to their new location, but leave them behind in the old
> > location so that the WebKit config keeps building
> 
> James, can you explain (1)? FWICT, WebKit DEPS are tied to a specific Chromium
> revision. So can't the steps be:
> 
> 1. Land this change as-is
> 2. Create a WebKit change to roll DEPS, perform any updates, and remove app/
> from DEPS
> 
> Assuming that (2) is done shortly afterwards? I don't think anything in
upstream
> actually depends on the files. It pulls in webkit/ which uses sql/, so just
> rolling DEPS and updating a .gyp file should be sufficient.

We also maintain a ToT chromium + ToT WebKit configuration, which the gardeners
use.  It would not be OK to compile break that configuration.

[Robert Sesek, 2011-07-18 21:04:38]

On 2011/07/18 20:26:05, jamesr wrote:
> On 2011/07/18 20:21:33, rsesek wrote:
> > On 2011/07/13 22:52:12, jamesr wrote:
> > > Yeah, I think you'll have to do this as a two-sided patch:
> > > 
> > > 1.) Move the files to their new location, but leave them behind in the old
> > > location so that the WebKit config keeps building
> > 
> > James, can you explain (1)? FWICT, WebKit DEPS are tied to a specific
Chromium
> > revision. So can't the steps be:
> > 
> > 1. Land this change as-is
> > 2. Create a WebKit change to roll DEPS, perform any updates, and remove app/
> > from DEPS
> > 
> > Assuming that (2) is done shortly afterwards? I don't think anything in
> upstream
> > actually depends on the files. It pulls in webkit/ which uses sql/, so just
> > rolling DEPS and updating a .gyp file should be sufficient.
> 
> We also maintain a ToT chromium + ToT WebKit configuration, which the
gardeners
> use.  It would not be OK to compile break that configuration.

Got it. It also occurred to me just now that the build errors are in objects
pulled by DEPS. So if everything is updated in this CL, then the ToT builder
should stay green.  Then the only upstream change that needs to happen is to
remove app/ from the DEPS file. Does that sound right?

[jamesr, 2011-07-18 21:06:27]

On 2011/07/18 21:04:38, rsesek wrote:
> On 2011/07/18 20:26:05, jamesr wrote:
> > On 2011/07/18 20:21:33, rsesek wrote:
> > > On 2011/07/13 22:52:12, jamesr wrote:
> > > > Yeah, I think you'll have to do this as a two-sided patch:
> > > > 
> > > > 1.) Move the files to their new location, but leave them behind in the
old
> > > > location so that the WebKit config keeps building
> > > 
> > > James, can you explain (1)? FWICT, WebKit DEPS are tied to a specific
> Chromium
> > > revision. So can't the steps be:
> > > 
> > > 1. Land this change as-is
> > > 2. Create a WebKit change to roll DEPS, perform any updates, and remove
app/
> > > from DEPS
> > > 
> > > Assuming that (2) is done shortly afterwards? I don't think anything in
> > upstream
> > > actually depends on the files. It pulls in webkit/ which uses sql/, so
just
> > > rolling DEPS and updating a .gyp file should be sufficient.
> > 
> > We also maintain a ToT chromium + ToT WebKit configuration, which the
> gardeners
> > use.  It would not be OK to compile break that configuration.
> 
> Got it. It also occurred to me just now that the build errors are in objects
> pulled by DEPS. So if everything is updated in this CL, then the ToT builder
> should stay green.  Then the only upstream change that needs to happen is to
> remove app/ from the DEPS file. Does that sound right?

You also need to ensure that the upstream DEPS file pulls down any directories
needed. For example, if sql/ is depended on, even if it's a transitive
dependency, then the upstream DEPS file needs to list it explicit so it is
checked out in upsteram builds.

[Robert Sesek, 2011-07-18 21:07:12]

On 2011/07/18 21:06:27, jamesr wrote:
> You also need to ensure that the upstream DEPS file pulls down any directories
> needed. For example, if sql/ is depended on, even if it's a transitive
> dependency, then the upstream DEPS file needs to list it explicit so it is
> checked out in upsteram builds.

Forgot to mention that upstream is already pulling down sql/.

[jamesr, 2011-07-18 21:39:23]

On 2011/07/18 21:07:12, rsesek wrote:
> On 2011/07/18 21:06:27, jamesr wrote:
> > You also need to ensure that the upstream DEPS file pulls down any
directories
> > needed. For example, if sql/ is depended on, even if it's a transitive
> > dependency, then the upstream DEPS file needs to list it explicit so it is
> > checked out in upsteram builds.
> 
> Forgot to mention that upstream is already pulling down sql/.

Awesome, then you should be fine (I think)!  Just remove app/ whenever it's not
needed.

[Robert Sesek, 2011-07-19 18:38:56]

LGTM