Bug 160031 - REGRESSION(r62549): Objective-C DOM bindings sometimes fail to regenerate when CodeGenerator.pm is modified
Summary: REGRESSION(r62549): Objective-C DOM bindings sometimes fail to regenerate whe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brian Burg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-21 10:37 PDT by Brian Burg
Modified: 2016-07-21 20:01 PDT (History)
9 users (show)

See Also:


Attachments
Proposed Fix (2.51 KB, patch)
2016-07-21 15:29 PDT, Brian Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2016-07-21 10:37:03 PDT
Ryosuke wrote:

"Objective-C bindings files may fail to re-generate after
https://trac.webkit.org/changeset/203337.  Simply trigger clean
rebuilds or delete files under DerivedSources/WebCore/ and it should
go away."

I hit this myself when moving past r203337. I think we have a bad dependency somewhere. Let's investigate in this bug.
Comment 1 Darin Adler 2016-07-21 13:26:57 PDT
Ryosuke’s theory was that the binding generator doesn't re-generate .cpp files when CodeGeneratorJS.pm is modified (even though that dependency is explicitly expressed in DerivedSources.make).

The best way to test this is to do a build, then do another to check that nothing rebuilds (sort of a "baseline"), then touch CodeGeneratorJS.pm, and see what rebuilds.

If too little rebuilds, then we could change things so make is invoked with "-d" and see why the rule that is supposed to trigger rebuilds isn’t working.
Comment 2 Brian Burg 2016-07-21 13:30:19 PDT
(In reply to comment #1)
> Ryosuke’s theory was that the binding generator doesn't re-generate .cpp
> files when CodeGeneratorJS.pm is modified (even though that dependency is
> explicitly expressed in DerivedSources.make).
> 
> The best way to test this is to do a build, then do another to check that
> nothing rebuilds (sort of a "baseline"), then touch CodeGeneratorJS.pm, and
> see what rebuilds.
> 
> If too little rebuilds, then we could change things so make is invoked with
> "-d" and see why the rule that is supposed to trigger rebuilds isn’t working.

I am going to kick off a build and see if I can repro this particular case again.
Comment 3 Brian Burg 2016-07-21 13:50:59 PDT
(In reply to comment #1)
> Ryosuke’s theory was that the binding generator doesn't re-generate .cpp
> files when CodeGeneratorJS.pm is modified (even though that dependency is
> explicitly expressed in DerivedSources.make).

I believe the relevant Make target is for the output .h files. These are a proxy for the .cpp/.mm files. 

GENERATE_SCRIPTS = \
    bindings/scripts/CodeGenerator.pm \
    bindings/scripts/IDLParser.pm \
    bindings/scripts/generate-bindings.pl \
    bindings/scripts/preprocessor.pm

JS_BINDINGS_SCRIPTS = $(GENERATE_SCRIPTS) bindings/scripts/CodeGeneratorJS.pm

JS%.h : %.idl $(JS_BINDINGS_SCRIPTS) $(IDL_ATTRIBUTES_FILE) $(WINDOW_CONSTRUCTORS_FILE) $(WORKERGLOBALSCOPE_CONSTRUCTORS_FILE) $(PLATFORM_FEATURE_DEFINES)
	$(call generator_script, $(JS_BINDINGS_SCRIPTS)) $(IDL_COMMON_ARGS) --defines "$(FEATURE_DEFINES) $(ADDITIONAL_IDL_DEFINES) LANGUAGE_JAVASCRIPT" --generator JS --idlAttributesFile $(IDL_ATTRIBUTES_FILE) --supplementalDependencyFile $(SUPPLEMENTAL_DEPENDENCY_FILE) $<
Comment 4 Brian Burg 2016-07-21 13:53:13 PDT
The build is still going, but my hunch is that we are missing $(WEBCORE) prefixes from some bindings files.
Comment 5 Brian Burg 2016-07-21 15:09:03 PDT
Validating a fix.
Comment 6 Brian Burg 2016-07-21 15:29:34 PDT
Created attachment 284269 [details]
Proposed Fix
Comment 7 WebKit Commit Bot 2016-07-21 20:01:06 PDT
Comment on attachment 284269 [details]
Proposed Fix

Clearing flags on attachment: 284269

Committed r203549: <http://trac.webkit.org/changeset/203549>
Comment 8 WebKit Commit Bot 2016-07-21 20:01:10 PDT
All reviewed patches have been landed.  Closing bug.