Bug 41669

Summary: [Chromium] Upstreaming inspector_resources target
Product: WebKit Reporter: Kent Tamura <tkent>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, pfeldman, rolandsteiner, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: All   
Bug Depends on: 42144    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch 2 none

Kent Tamura
Reported Tuesday, July 6, 2010 10:16:59 AM UTC
Upstreaming 'inspector_resources', which is currently in src/webkit/webkit.gyp of Chromium tree, to WebKit.gyp.
Attachments
Patch (5.82 KB, patch)
2010-07-06 02:22 PDT, Kent Tamura
no flags
Patch 2 (5.68 KB, patch)
2010-07-07 19:17 PDT, Kent Tamura
no flags
Kent Tamura
Comment 1 Tuesday, July 6, 2010 10:22:57 AM UTC
Adam Barth
Comment 2 Wednesday, July 7, 2010 11:23:52 AM UTC
Comment on attachment 60605 [details] Patch This gyp magic is too deep for me, sorry.
Tony Chang
Comment 3 Wednesday, July 7, 2010 4:30:08 PM UTC
Comment on attachment 60605 [details] Patch I realize that this is mostly a copy/paste from the existing gyp file, but might as well clean up the code a bit. WebKit/chromium/WebKit.gyp:68 + 'debug_devtools%': 0, Nit: It would be nice if there was a comment describing what this does. WebKit/chromium/WebKit.gyp:562 + # 'msvs_guid': '5330F8EE-00F5-D65C-166E-E3150171055D', Can we just remove this? WebKit/chromium/WebKit.gyp:566 + 'dependencies+': ['concatenated_devtools_js'], What does the + at the end do? WebKit/chromium/WebKit.gyp:578 + 'files/': [['exclude', '\\.js$']], What does the / at the end of files do? WebKit/chromium/WebKit.gyp:599 + '<(chromium_src_dir)/webkit/build/generate_devtools_html.py', Can we upstream generate_devtools_html.py too?
Kent Tamura
Comment 4 Thursday, July 8, 2010 3:17:39 AM UTC
Created attachment 60824 [details] Patch 2
Kent Tamura
Comment 5 Thursday, July 8, 2010 3:23:57 AM UTC
Thank you for looking at the patch. (In reply to comment #3) > WebKit/chromium/WebKit.gyp:68 > + 'debug_devtools%': 0, > Nit: It would be nice if there was a comment describing what this does. Added a comment. > WebKit/chromium/WebKit.gyp:562 > + # 'msvs_guid': '5330F8EE-00F5-D65C-166E-E3150171055D', > Can we just remove this? Removed. According to http://code.google.com/p/gyp/wiki/GypUserDocumentation#Skeleton_of_a_typical_executable_target_in_a_.gyp_file, we don't need msvs_guid anymore, right? > WebKit/chromium/WebKit.gyp:566 > + 'dependencies+': ['concatenated_devtools_js'], > What does the + at the end do? Prepending something to 'dependencies' makes no sense. I removed '+'. > WebKit/chromium/WebKit.gyp:578 > + 'files/': [['exclude', '\\.js$']], > What does the / at the end of files do? I think we need to use pattern exclusion here. http://code.google.com/p/gyp/wiki/InputFormatReference#Pattern_Lists_(/) > WebKit/chromium/WebKit.gyp:599 > + '<(chromium_src_dir)/webkit/build/generate_devtools_html.py', > Can we upstream generate_devtools_html.py too? We can. Where should we put it to? WebKitTools/Scripts?
Tony Chang
Comment 6 Thursday, July 8, 2010 6:39:38 PM UTC
LGTM '<(chromium_src_dir)/webkit/build/generate_devtools_html.py', > > Can we upstream generate_devtools_html.py too? > > We can. Where should we put it to? WebKitTools/Scripts? Hmm, good question. I guess I would be nice to make a directory at WebKit/WebKit/chromium/WebKit.gyp/ to put all the build related files (kind of like how we have WebCore/WebCore.gyp/). It's kind of a pain to move WebKit.gyp now, so maybe just put it in WebKit/WebKit/chromium/ for now. WebKitTools/Scripts doesn't seem to contain any build related scripts.
Kent Tamura
Comment 7 Tuesday, July 13, 2010 6:37:45 AM UTC
Comment on attachment 60824 [details] Patch 2 Clearing flags on attachment: 60824 Committed r63171: <http://trac.webkit.org/changeset/63171>
Kent Tamura
Comment 8 Tuesday, July 13, 2010 6:37:53 AM UTC
All reviewed patches have been landed. Closing bug.
Kent Tamura
Comment 9 Tuesday, July 13, 2010 7:06:43 AM UTC
I rolled r63171 out. It failed by the following errors on Chromium Linux canary: make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/codemap.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/consarray.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/csvparser.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/logreader.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/profile.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/profile_view.js', needed by `out/Release/resources/inspector/DevTools.js'. make: *** No rule to make target `third_party/WebKit/WebKit/chromium/v8/tools/splaytree.js', needed by `out/Release/resources/inspector/DevTools.js'. I'm investigating...
Pavel Feldman
Comment 10 Tuesday, July 13, 2010 7:09:37 AM UTC
Please don't land this unless tested on all platforms. I'd like to have us rolled today.
Kent Tamura
Comment 11 Tuesday, July 13, 2010 7:21:43 AM UTC
I identified the problem. (In reply to comment #10) > Please don't land this unless tested on all platforms. I'd like to have us rolled today. ok, I'll test and will re-land the change tomorrow just in case.
Kent Tamura
Comment 12 Wednesday, July 14, 2010 8:04:57 AM UTC
Note You need to log in before you can comment on or make changes to this bug.