Bug 36578 – [chromium] update chromium DEPS for upstream compile

• Component: New Bugs
• Status: RESOLVED FIXED

[Tony Chang]

update chromium DEPS for upstream compile

[Tony Chang]

Created attachment 51594 [details]
Patch

[Tony Chang]

(In reply to comment #1)
> Created an attachment (id=51594) [details]
> Patch

This doesn't build webkit/glue, it just brings in all the dependencies so we could build it.  Currently, no targets are using it, so it's just a bunch of extra files being pulled.

[Adam Barth]

This looks complicated.  Would you like an actual review or just a rubber stamp?

[Tony Chang]

(In reply to comment #3)
> This looks complicated.  Would you like an actual review or just a rubber
> stamp?

There's no hurry to get this committed, so waiting on an actual review from dglazkov or fishd is fine.

[Darin Fisher (:fishd, Google)]

Comment on attachment 51594 [details]
Patch

> +++ b/WebKit/chromium/DEPS

> +   'third_party/wtl':
> +     Var('chromium_svn')+'/third_party/wtl@'+Var('chromium_rev'),

can we avoid the wtl dependencies?

would it be possible for some of these dependencies to be expressed
through a GYP variable that is defined within the chromium repository?

it seems like it would be very nice if chromium/DEPS only mentioned
the things that are directly used by WebKit.  then, if we could
somehow infer the dependencies that those things require, we could
greatly minimize what gets hard coded into chromium/DEPS.  That
would help minimize the cost associated with changing the set of
dependencies.

R=me all the same

[Tony Chang]

(In reply to comment #5)
> can we avoid the wtl dependencies?

This is only needed by webkit/default_plugin/plugin_impl_win.h, which inherits from app/win/window_impl.h to pop up the dialog about installing plugins.  I'm not sure how much effort it would be to re-write this or to make a stub implementation since we don't actually need it for DRT.

> would it be possible for some of these dependencies to be expressed
> through a GYP variable that is defined within the chromium repository?
> 
> it seems like it would be very nice if chromium/DEPS only mentioned
> the things that are directly used by WebKit.  then, if we could
> somehow infer the dependencies that those things require, we could
> greatly minimize what gets hard coded into chromium/DEPS.  That
> would help minimize the cost associated with changing the set of
> dependencies.

fishd just reminded me on chat that gclient supports "From" to include version from other DEPS files.  I'm going to try to rewrite the DEPS file using "From"

[Tony Chang]

Comment on attachment 51594 [details]
Patch

My current plan is to do the following:
1) Modify gclient to allow checking out a single file.  The syntax will be like:
chromium_deps: File("svn://..../DEPS")

2) Modify gclient's From syntax so we can pull revisions that map to different directories.  E.g., 
net: From("chromium_deps", "src/net")
This would pull the svn revision specific for 'src/net' in the chromium DEPS file into our local checkout.

3) Rewrite WebKit/chromium/DEPS with these values.

[Darin Fisher (:fishd, Google)]

(In reply to comment #7)
> (From update of attachment 51594 [details])
> My current plan is to do the following:
> 1) Modify gclient to allow checking out a single file.  The syntax will be
> like:
> chromium_deps: File("svn://..../DEPS")
> 
> 2) Modify gclient's From syntax so we can pull revisions that map to different
> directories.  E.g., 
> net: From("chromium_deps", "src/net")
> This would pull the svn revision specific for 'src/net' in the chromium DEPS
> file into our local checkout.
> 
> 3) Rewrite WebKit/chromium/DEPS with these values.

This sounds great.  For the chromium_deps item, you probably want to specify DEPS@REVISION.  I assume this is what you had in mind.

[Tony Chang]

Created attachment 52931 [details]
Patch

[Tony Chang]

Committed r57460: <http://trac.webkit.org/changeset/57460>

[WebKit Review Bot]

http://trac.webkit.org/changeset/57460 might have broken Chromium Linux Release

[Tony Chang]

Broke all three chromium bots, analysis and some fixes coming up.

[Tony Chang]

Created attachment 53122 [details]
Patch

[Tony Chang]

(In reply to comment #13)
> Created an attachment (id=53122) [details]
> Patch

This patch fixes compile on linux by adding third_party/yasm/source/patched-yasm and third_party/ffmpeg/source/patched-ffmpeg-mt to the checkout.

Mac broke because it the new gclient File() syntax uses svn checkout --depth which is available in svn 1.5+.  It looks like OSX 10.5 comes with svn 1.4.4.

Windows broke because we need to rm -rf third_party/ffmpeg before updating (we're checkout out some files there, but a subdirectory already has files checkout there).


Linux and Windows should be fixable, but I'm not sure about Mac.  I don't want to require new subversion to be installed.  I will talk with MA and maybe we can get a newer subversion included in depot tools on mac.

[Tony Chang]

Ok, I fixed gclient to work with svn1.4 so mac should now be safe to land too.  I'm going to try to land this with coordination from yaar (who can access the windows slave and clobber a directory for me).

[Tony Chang]

Committed r57620: <http://trac.webkit.org/changeset/57620>