Summary: | Web Inspector: generated backend commands should reflect build system ENABLE settings | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | berto, bunhere, commit-queue, graouts, gyuyoung.kim, joepeck, mrobinson, rakuco, sergio, timothy, webkit-bug-importer, zan | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | 130110 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
BJ Burg
2014-03-11 19:27:31 PDT
Using the C preprocessor to filter things out of the JSON files and prevent generation sounds fine to me. Joe suggested an even easier solution: conditionally add protocol spec files inside DerivedSources.make. I am going to wait to submit this until 130110 because it has no effect presently (the ruby script to update doesn't take into account ENABLE flags). Created attachment 226759 [details]
the patch
Comment on attachment 226759 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=226759&action=review > Source/WebCore/CMakeLists.txt:765 > inspector/protocol/IndexedDB.json Same for IndexedDB as Databases? > Source/WebCore/GNUmakefile.am:374 > +EnabledInspectorDomains: force > + echo '$(WebCore_INSPECTOR_DOMAINS)' | cmp -s - $@ || echo '$(WebCore_INSPECTOR_DOMAINS)' > $@ > + > DerivedSources/WebCore/InspectorWeb.json: $(INSPECTOR_SCRIPTS_DIR)/generate-combined-inspector-json.py $(WebCore_INSPECTOR_DOMAINS) Doesn't InspectorWeb.json need to have "EnabledInspectorDomains" as a dependency? Created attachment 226792 [details]
patch round 2
Comment on attachment 226792 [details]
patch round 2
This breaks the generation of InspectorWeb.json on GTK at least. And it screws up the following builds on the GTK EWS, at least..
Comment on attachment 226792 [details] patch round 2 View in context: https://bugs.webkit.org/attachment.cgi?id=226792&action=review > Source/WebCore/CMakeLists.txt:2604 > + inspector/protocol/Database.json This line needs to be ${WEBCORE_DIR}/inspector/protocol/Database.json (In reply to comment #5) > (From update of attachment 226759 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226759&action=review > > > Source/WebCore/CMakeLists.txt:765 > > inspector/protocol/IndexedDB.json > > Same for IndexedDB as Databases? nothing else is conditional on ENABLE_INDEXED_DATABASE so I didn't bother. > > > Source/WebCore/GNUmakefile.am:374 > > +EnabledInspectorDomains: force > > + echo '$(WebCore_INSPECTOR_DOMAINS)' | cmp -s - $@ || echo '$(WebCore_INSPECTOR_DOMAINS)' > $@ > > + > > DerivedSources/WebCore/InspectorWeb.json: $(INSPECTOR_SCRIPTS_DIR)/generate-combined-inspector-json.py $(WebCore_INSPECTOR_DOMAINS) > > Doesn't InspectorWeb.json need to have "EnabledInspectorDomains" as a dependency? Yes. (In reply to comment #8) > (From update of attachment 226792 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226792&action=review > > > Source/WebCore/CMakeLists.txt:2604 > > + inspector/protocol/Database.json > > This line needs to be ${WEBCORE_DIR}/inspector/protocol/Database.json Oops, I should have changed that one too. Thanks. Created attachment 226846 [details]
patch round 3
Ooops, forgot that we can't guard generating WEB_REPLAY on mac builds because Xcode doesn't like that. Will remove that before landing. Committed r165714: <http://trac.webkit.org/changeset/165714> |