Bug 41669 – [Chromium] Upstreaming inspector_resources target

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Kent Tamura]

Upstreaming 'inspector_resources', which is currently in src/webkit/webkit.gyp of Chromium tree, to WebKit.gyp.

[Kent Tamura]

Created attachment 60605 [details]
Patch

[Adam Barth]

Comment on attachment 60605 [details]
Patch

This gyp magic is too deep for me, sorry.

[Tony Chang]

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]

Created attachment 60824 [details]
Patch 2

[Kent Tamura]

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]

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 on attachment 60824 [details]
Patch 2

Clearing flags on attachment: 60824

Committed r63171: <http://trac.webkit.org/changeset/63171>

[Kent Tamura]

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

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]

Please don't land this unless tested on all platforms. I'd like to have us rolled today.

[Kent Tamura]

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]

Re-landed as r63285: http://trac.webkit.org/changeset/63285