Summary: | Web Inspector: make inspector protocol validation a part of the build process. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Pavel Feldman <pfeldman> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, levin, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Pavel Feldman
2011-09-28 06:19:36 PDT
Created attachment 109017 [details]
Patch
Comment on attachment 109017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109017&action=review > Source/WebCore/inspector/validate-protocol-compatibility:308 > + sys.stderr.write("Usage: %s -o OUTPUT_FILE INPUT_FILE\n" % sys.argv[0]) Since there are two required parameters and their positions are fixed, let's omit "-o" Comment on attachment 109017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109017&action=review > Source/WebCore/DerivedSources.make:895 > +Inspector.json.validated : Inspector.json inspector/validate-protocol-compatibility > + python $(WebCore)/inspector/validate-protocol-compatibility -o Inspector.json.validated $(WebCore)/inspector/Inspector.json > + > Inspector.idl : Inspector.json inspector/generate-inspector-idl > python $(WebCore)/inspector/generate-inspector-idl -o Inspector.idl $(WebCore)/inspector/Inspector.json Do make files require Tabs? I wonder if the 8 spaces here will cause an issue. The Inspector.idl generator example below appears to use a tab. > Source/WebCore/GNUmakefile.am:691 > +DerivedSources/WebCore/Inspector.json.validated : $(WebCore)/inspector/Inspector.json $(WebCore)/inspector/validate-protocol-compatibility > + $(PYTHON) $(WebCore)/inspector/validate-protocol-compatibility -o $(GENSOURCES_WEBCORE)/Inspector.json.validated $(WebCore)/inspector/Inspector.json ditto > Do make files require Tabs? I wonder if the 8 spaces here will cause an issue. The Inspector.idl generator example below appears to use a tab.
>
> > Source/WebCore/GNUmakefile.am:691
> > +DerivedSources/WebCore/Inspector.json.validated : $(WebCore)/inspector/Inspector.json $(WebCore)/inspector/validate-protocol-compatibility
> > + $(PYTHON) $(WebCore)/inspector/validate-protocol-compatibility -o $(GENSOURCES_WEBCORE)/Inspector.json.validated $(WebCore)/inspector/Inspector.json
>
> ditto
I'll fix those, thanks for the heads up!
Created attachment 109032 [details]
[Patch] spaces -> tab for makefiles.
Committed r96320: <http://trac.webkit.org/changeset/96320> Why is this a build failure as opposed to a test failure? Putting something in the build slows down a process (the build -- I may do several builds before I get everything ready to run the test cases -- also my machine is under more stress when I do a build, etc.) that is run more often than tests. (In reply to comment #7) > Why is this a build failure as opposed to a test failure? > > Putting something in the build slows down a process (the build -- I may do several builds before I get everything ready to run the test cases -- also my machine is under more stress when I do a build, etc.) that is run more often than tests. I have to admit that initially it was more of a paranoia on our side: we wanted this check to never be suppressed. However, now there is a patch pending that converts this validation into a header generator (https://bugs.webkit.org/show_bug.cgi?id=69092). So now this generation piece needs to be a part of the build. Talking about the several builds, new target is a quick python check that depends on two files: inspector protocol schema and the python script itself. I would expect it to trigger only once for you (unless you are modifying these files). Please tell me if this is not the case. Broader context: 1) we are consolidating all the generation steps in the inspector into a single python-based one, getting rid of the intermediate IDL step that is using bindings' perl script 2) once (1) is complete, we will add runtime traffic validation while in the debug mode. 3) once (2) is ready, we will consider migrating to the layouttest-based validation. (In reply to comment #8) > (In reply to comment #7) > > Why is this a build failure as opposed to a test failure? > > > > Putting something in the build slows down a process (the build -- I may do several builds before I get everything ready to run the test cases -- also my machine is under more stress when I do a build, etc.) that is run more often than tests. > > I have to admit that initially it was more of a paranoia on our side: we wanted this check to never be suppressed. However, now there is a patch pending that converts this validation into a header generator (https://bugs.webkit.org/show_bug.cgi?id=69092). So now this generation piece needs to be a part of the build. > > Talking about the several builds, new target is a quick python check that depends on two files: inspector protocol schema and the python script itself. I would expect it to trigger only once for you (unless you are modifying these files). Please tell me if this is not the case. > > Broader context: > 1) we are consolidating all the generation steps in the inspector into a single python-based one, getting rid of the intermediate IDL step that is using bindings' perl script > 2) once (1) is complete, we will add runtime traffic validation while in the debug mode. > 3) once (2) is ready, we will consider migrating to the layouttest-based validation. Thanks for the info. I just saw this and got a bit paranoid about unnecessary things being added to the build. :) |