WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
130110
Web Inspector: vended backend commands file should be generated as part of the build
https://bugs.webkit.org/show_bug.cgi?id=130110
Summary
Web Inspector: vended backend commands file should be generated as part of th...
Blaze Burg
Reported
2014-03-11 19:22:03 PDT
Files like InspectorWebBackendCommands.js are currently hand-generated but the update script just calls the code generator that already runs during the build. AFAIK, there's no reason to not tie it back into the Derived Sources build phase. The WebInspectorUI project will then depend on WebCore so it can copy over the JS file to its framework.
Attachments
wip (missing CMake, some GTK bundling magic)
(66.13 KB, patch)
2014-03-13 19:43 PDT
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
the patch - missing Win/CMake bits for WebInspectorUI
(72.54 KB, patch)
2014-03-14 13:27 PDT
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
patch round 2
(75.34 KB, patch)
2014-03-14 19:12 PDT
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Timothy Hatcher
Comment 1
2014-03-12 11:55:11 PDT
Making it depend on WebCore is fine. WebCore will just need to copy the commands into its bundle resources for the WebInspectorUI project to find and copy out.
Joseph Pecoraro
Comment 2
2014-03-13 13:22:26 PDT
(In reply to
comment #1
)
> Making it depend on WebCore is fine. WebCore will just need to copy the commands into its bundle resources for the WebInspectorUI project to find and copy out.
Probably not "bundle resources" but "private headers". If it went to the Resources directory, we might accidentally include the scripts in WebCore.framework/Resources. PrivateHeaders we already use for this kind of thing with code generator scripts, and they are not included in what we ship.
Timothy Hatcher
Comment 3
2014-03-13 13:56:24 PDT
Ah, yes that is what I meant.
Blaze Burg
Comment 4
2014-03-13 19:43:59 PDT
Created
attachment 226641
[details]
wip (missing CMake, some GTK bundling magic)
Blaze Burg
Comment 5
2014-03-14 13:27:17 PDT
Created
attachment 226758
[details]
the patch - missing Win/CMake bits for WebInspectorUI
Radar WebKit Bug Importer
Comment 6
2014-03-14 16:33:36 PDT
<
rdar://problem/16332581
>
Blaze Burg
Comment 7
2014-03-14 19:12:08 PDT
Created
attachment 226791
[details]
patch round 2
WebKit Commit Bot
Comment 8
2014-03-16 11:44:22 PDT
Comment on
attachment 226791
[details]
patch round 2 Clearing flags on attachment: 226791 Committed
r165704
: <
http://trac.webkit.org/changeset/165704
>
WebKit Commit Bot
Comment 9
2014-03-16 11:44:28 PDT
All reviewed patches have been landed. Closing bug.
Joseph Pecoraro
Comment 10
2014-03-17 10:56:52 PDT
Comment on
attachment 226791
[details]
patch round 2 View in context:
https://bugs.webkit.org/attachment.cgi?id=226791&action=review
Late comments. Good stuff!
> Source/WebCore/CMakeLists.txt:3023 > + OUTPUT ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebBackendDispatchers.cpp ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebBackendDispatchers.h ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebFrontendDispatchers.cpp ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebFrontendDispatchers.h ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebTypeBuilders.cpp ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebTypeBuilders.h ${DERIVED_SOURCES_WEBCORE_DIR}/InspectorWebBackendCommands.js
Oops! Good catch on these.
> Source/WebInspectorUI/GNUmakefile.am:37 > $(WebInspectorUI)/Localizations/en.lproj/localizedStrings.js \ > + $(shell ls $(GENSOURCES_JAVASCRIPTCORE)/InspectorJSBackendCommands.js) \ > + $(shell ls $(GENSOURCES_WEBCORE)/InspectorWebBackendCommands.js) \
I don't think the "shell ls" is needed for individual files. This could be just like the line above it.
Brent Fulgham
Comment 11
2014-03-27 09:59:03 PDT
This change broke the Windows WebInspector. I'm putting together a fix.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug