Summary: | split webkit gyp files for chromium build to break the circular dependency | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||||||||
Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, tony, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 73996 | ||||||||||||
Bug Blocks: | 68463, 73384 | ||||||||||||
Attachments: |
|
Description
Dirk Pranke
2011-11-29 18:19:07 PST
Note that the duplication should be short-lived, and eventually we'll delete all the rules from WebKit.gyp once we can assume that the build_webkit_exes_from_webkit_gyp file is false. Created attachment 117091 [details]
Patch
Comment on attachment 117091 [details] Patch Attachment 117091 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10709151 Comment on attachment 117091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117091&action=review r- for bot failures > Source/WebKit/chromium/All.gyp:42 > + 'WebKit.gyp:*', > + 'WebKitUnitTests.gyp:*', > + '../../../Tools/Tools.gyp:*', Is it sufficient to just depend on webkit_unittests and DumpRenderTree? That way we can be more confident that the other dependencies are correct. > Source/WebKit/chromium/WebKit.gyp:1100 > + # https://bugs.webkit.org/show_bug.cgi?id=68463. Until then, we > + # leave the indentation alone to minimize whitespace diffs. I would go ahead and indent. PrettyPatch should be smart enough to show that it's just an indention change. > Source/WebKit/chromium/WebKitUnitTests.gyp:35 > + # https://bugs.webkit.org/show_bug.cgi?id=68463. Until then, we > + # leave the indentation alone to minimize whitespace diffs. I would indent this properly too. > Source/WebKit/chromium/gyp_webkit:136 > + # Change to the base dir needed for the gyp file path above to work correctly. > + os.chdir(script_dir) This failed on the bots since when gclient runs it, it's already in the script_dir. > Tools/Tools.gyp:35 > + # https://bugs.webkit.org/show_bug.cgi?id=68463. Until then, we > + # leave the indentation alone to minimize whitespace diffs. Same as above. (In reply to comment #4) > > Source/WebKit/chromium/All.gyp:42 > > + 'WebKit.gyp:*', > > + 'WebKitUnitTests.gyp:*', > > + '../../../Tools/Tools.gyp:*', > > Is it sufficient to just depend on webkit_unittests and DumpRenderTree? That way we can be more confident that the other dependencies are correct. > I waffled on which way to do this. Your way should work as well, so I'll change to that. > > Source/WebKit/chromium/WebKit.gyp:1100 > > + # https://bugs.webkit.org/show_bug.cgi?id=68463. Until then, we > > + # leave the indentation alone to minimize whitespace diffs. > > I would go ahead and indent. PrettyPatch should be smart enough to show that it's just an indention change. > Ok, will do. > > Source/WebKit/chromium/gyp_webkit:136 > > + # Change to the base dir needed for the gyp file path above to work correctly. > > + os.chdir(script_dir) > > This failed on the bots since when gclient runs it, it's already in the script_dir. > Yeah, I noticed that this didn't seem to be working right; I'm not sure why that is failing, but I'll delete it. Created attachment 117276 [details]
update w/ review feedback
Comment on attachment 117276 [details] update w/ review feedback View in context: https://bugs.webkit.org/attachment.cgi?id=117276&action=review Please make sure the chromium ews bot passes before landing. > Source/WebKit/chromium/gyp_webkit:133 > - 'WebKit.gyp']) > + os.path.join('All.gyp')]) Why os.path.join on a single item? (In reply to comment #7) > (From update of attachment 117276 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117276&action=review > > Please make sure the chromium ews bot passes before landing. > Roger. > > Source/WebKit/chromium/gyp_webkit:133 > > - 'WebKit.gyp']) > > + os.path.join('All.gyp')]) > > Why os.path.join on a single item? No reason; it's left over from a previous version. I will fix. Created attachment 117520 [details] merge to r101718 Comment on attachment 117520 [details] merge to r101718 Attachment 117520 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10693926 New failing tests: fast/layers/clip-rects-transformed.html fast/replaced/no-focus-ring-object.html http/tests/security/contentSecurityPolicy/object-src-none-allowed.html http/tests/security/contentSecurityPolicy/object-src-no-url-allowed.html fast/layers/clip-rects-transformed-2.html http/tests/plugins/npapi-response-headers.html fast/frames/sandboxed-iframe-about-blank.html http/tests/plugins/get-url.html http/tests/plugins/interrupted-get-url.html fast/replaced/no-focus-ring-embed.html compositing/plugins/invalidate_rect.html fast/frames/iframe-plugin-load-remove-document-crash.html editing/selection/selection-plugin-clear-crash.html fast/replaced/invalid-object-with-fallback.html http/tests/plugins/cross-frame-object-access.html fast/frames/sandboxed-iframe-navigation-allowed.html http/tests/plugins/geturlnotify-from-npp-destroystream.html fast/events/tabindex-focus-blur-all.html http/tests/plugins/third-party-cookie-accept-policy.html Created attachment 117971 [details]
Patch
Committed r102201: <http://trac.webkit.org/changeset/102201> |