Bug 47877 - [chromium] DumpRenderTree shouldn't put '.' in include path
Summary: [chromium] DumpRenderTree shouldn't put '.' in include path
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-18 20:33 PDT by Tony Chang
Modified: 2010-10-19 10:37 PDT (History)
3 users (show)

See Also:


Attachments
Patch (26.20 KB, patch)
2010-10-18 20:42 PDT, Tony Chang
tkent: 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-10-18 20:33:04 PDT
Dirk was having problems compiling in a chromium checkout that also has an upstream checkout.  The problem is in WebKit.gyp in the DumpRenderTree target.  It adds "." to include_dirs.  When a file in DumpRenderTree tries to include e.g., webkit/support/webkit_support.h, it ends up including the file from third_party/WebKit/WebKit/chromium/webkit/support rather than from webkit/support.

We can fix this by replacing '.' in the include path with '<(chromium_src_dir)'.  I had to do this of the TestNetscapePlugin target.  The problem right now is that DRT tries to include WebKit api files from public/.  So we need to replace the #include lines to not include public and add public to the include dirs.  That should be consistent with how the other targets in WebKit work.
Comment 1 Tony Chang 2010-10-18 20:42:14 PDT
Created attachment 71126 [details]
Patch
Comment 2 Tony Chang 2010-10-18 20:42:51 PDT
This seems consistent with how we do includes for other targets in WebKit.gyp (e.g., webkit_unit_tests).
Comment 3 Dirk Pranke 2010-10-18 20:46:01 PDT
So, why am I the only one hitting this? Does no one else have a chromium checkout that includes an upstream checkout that includes a separate (build-webkit --chromium) checkout?
Comment 4 Tony Chang 2010-10-18 20:48:43 PDT
(In reply to comment #3)
> So, why am I the only one hitting this? Does no one else have a chromium checkout that includes an upstream checkout that includes a separate (build-webkit --chromium) checkout?

I suspect it's been a while time since you ran update-webkit --chromium?  Alternately, maybe we haven't updated WebKit/chromium/DEPS in a while?
Comment 5 Dirk Pranke 2010-10-18 21:10:46 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > So, why am I the only one hitting this? Does no one else have a chromium checkout that includes an upstream checkout that includes a separate (build-webkit --chromium) checkout?
> 
> I suspect it's been a while time since you ran update-webkit --chromium?  Alternately, maybe we haven't updated WebKit/chromium/DEPS in a while?

Either is possible. That said, should I have to run update-webkit --chromium before I can build src/webkit/webkit.gyp ? Is this something weird because I have a webkit checkout that gclient sync normally takes care of?

Or does the code have logic to only try and build DRT if there is an upstream checkout, and so people who don't have upstream checkouts don't hit this?
Comment 6 Kent Tamura 2010-10-18 21:52:07 PDT
Comment on attachment 71126 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71126&action=review

> WebKitTools/DumpRenderTree/chromium/MockSpellCheck.cpp:35
> +

nit: We don't need this blank line.
Comment 7 Tony Chang 2010-10-19 09:33:49 PDT
(In reply to comment #5)
> Either is possible. That said, should I have to run update-webkit --chromium before I can build src/webkit/webkit.gyp ? Is this something weird because I have a webkit checkout that gclient sync normally takes care of?

You only need to run update-webkit --chromium if you want to build as a webkit only checkout (e.g., if you want to use build-webkit --chromium).  This is only a problem for people who have previously run update-webkit --chromium in their third_party/WebKit directory.

> Or does the code have logic to only try and build DRT if there is an upstream checkout, and so people who don't have upstream checkouts don't hit this?

The gyp files will always try to build DRT, however, some of the include paths are different depending on whether you're building with a chromium checkout or if you're using build-webkit --chromium.  The bug was that we weren't making the include path sensitive to those differences.
Comment 8 Tony Chang 2010-10-19 09:44:12 PDT
Committed r70064: <http://trac.webkit.org/changeset/70064>
Comment 9 Dirk Pranke 2010-10-19 10:11:06 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > Either is possible. That said, should I have to run update-webkit --chromium before I can build src/webkit/webkit.gyp ? Is this something weird because I have a webkit checkout that gclient sync normally takes care of?
> 
> You only need to run update-webkit --chromium if you want to build as a webkit only checkout (e.g., if you want to use build-webkit --chromium).  This is only a problem for people who have previously run update-webkit --chromium in their third_party/WebKit directory.
>

Okay, so I should not have to run update-webkit --chromium ever if I have a chromium downstream checkout. Is there a way to delete everything that was checked out by update-webkit --chromium in the third_party/WebKit  directory, or should I just dump this checkout and create a new one to be clean?
Comment 10 Tony Chang 2010-10-19 10:37:39 PDT
(In reply to comment #9)
> Okay, so I should not have to run update-webkit --chromium ever if I have a chromium downstream checkout. Is there a way to delete everything that was checked out by update-webkit --chromium in the third_party/WebKit  directory, or should I just dump this checkout and create a new one to be clean?

Correct.  If it's a git repo, I think you could use git reset --hard (you'll probably have to re-run gyp after that).  If it's an svn directory, I would just rm -rf the files in WebKit/chromium/ and svn update that directory.