Bug 36578 - [chromium] update chromium DEPS for upstream compile
Summary: [chromium] update chromium DEPS for upstream compile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 37424
Blocks: 35902
  Show dependency treegraph
 
Reported: 2010-03-24 22:07 PDT by Tony Chang
Modified: 2010-04-14 17:13 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.30 KB, patch)
2010-03-24 22:08 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2010-04-08 19:16 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (8.11 KB, patch)
2010-04-11 18:10 PDT, Tony Chang
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2010-03-24 22:07:22 PDT
update chromium DEPS for upstream compile
Comment 1 Tony Chang 2010-03-24 22:08:29 PDT
Created attachment 51594 [details]
Patch
Comment 2 Tony Chang 2010-03-24 22:09:29 PDT
(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.
Comment 3 Adam Barth 2010-03-24 22:22:50 PDT
This looks complicated.  Would you like an actual review or just a rubber stamp?
Comment 4 Tony Chang 2010-03-24 22:24:23 PDT
(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.
Comment 5 Darin Fisher (:fishd, Google) 2010-03-24 22:28:31 PDT
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
Comment 6 Tony Chang 2010-03-24 22:44:35 PDT
(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"
Comment 7 Tony Chang 2010-03-26 02:29:13 PDT
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.
Comment 8 Darin Fisher (:fishd, Google) 2010-03-26 16:19:17 PDT
(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.
Comment 9 Tony Chang 2010-04-08 19:16:40 PDT
Created attachment 52931 [details]
Patch
Comment 10 Tony Chang 2010-04-11 17:50:00 PDT
Committed r57460: <http://trac.webkit.org/changeset/57460>
Comment 11 WebKit Review Bot 2010-04-11 17:57:52 PDT
http://trac.webkit.org/changeset/57460 might have broken Chromium Linux Release
Comment 12 Tony Chang 2010-04-11 18:05:41 PDT
Broke all three chromium bots, analysis and some fixes coming up.
Comment 13 Tony Chang 2010-04-11 18:10:20 PDT
Created attachment 53122 [details]
Patch
Comment 14 Tony Chang 2010-04-11 18:17:02 PDT
(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.
Comment 15 Tony Chang 2010-04-13 21:02:09 PDT
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).
Comment 16 Tony Chang 2010-04-14 17:13:33 PDT
Committed r57620: <http://trac.webkit.org/changeset/57620>