WebIDL codegen failure no longer fails the build

RESOLVED FIXED in mozilla29

Status

defect
--
blocker
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: Ehsan, Assigned: gps)

Tracking

({regression})

Trunk
mozilla29
x86
macOS
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

STR: Apply this patch on top of the current inbound and ./mach build dom/bindings:

diff --git a/dom/bindings/Bindings.conf b/dom/bindings/Bindings.conf
index eac1e7e..da47f96 100644
--- a/dom/bindings/Bindings.conf
+++ b/dom/bindings/Bindings.conf
@@ -702,6 +702,10 @@ DOMInterfaces = {
     'headerFile': 'nsIMediaList.h',
 },

+'MediaQueryList': {
+    'wrapperCache': False
+},
+
 'MediaSource': [{
     'resultNotAddRefed': [ 'sourceBuffers', 'activeSourceBuffers' ],
 },


 0:06.43 Traceback (most recent call last):
 0:06.43   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
 0:06.43     "__main__", fname, loader, pkg_name)
 0:06.43   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
 0:06.43     exec code in run_globals
 0:06.43   File "/Users/ehsan/moz/src/python/mozbuild/mozbuild/action/webidl.py", line 17, in <module>
 0:06.43     sys.exit(main(sys.argv[1:]))
 0:06.43   File "/Users/ehsan/moz/src/python/mozbuild/mozbuild/action/webidl.py", line 13, in main
 0:06.43     manager.generate_build_files()
 0:06.43   File "/Users/ehsan/moz/src/dom/bindings/mozwebidlcodegen/__init__.py", line 273, in generate_build_files
 0:06.43     written, deps = self._generate_build_files_for_webidl(filename)
 0:06.43   File "/Users/ehsan/moz/src/dom/bindings/mozwebidlcodegen/__init__.py", line 461, in _generate_build_files_for_webidl
 0:06.43     root = CGBindingRoot(self._config, binding_stem, filename)
 0:06.43   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 9632, in __init__
 0:06.43     for c in mainCallbacks)
 0:06.43   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 9632, in <genexpr>
 0:06.43     for c in mainCallbacks)
 0:06.43   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 10773, in __init__
 0:06.44     methods=[CallCallback(callback, descriptorProvider)])
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 11087, in __init__
 0:06.44     descriptorProvider, needThisHandling=True)
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 11061, in __init__
 0:06.44     needThisHandling, rethrowContentException)
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 10861, in __init__
 0:06.44     self.body = self.getImpl()
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 10868, in getImpl
 0:06.44     "convertArgs": self.getArgConversions(),
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 10928, in getArgConversions
 0:06.44     in enumerate(self.originalSig[1])]
 0:06.44   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 10975, in getArgConversion
 0:06.45     'exceptionCode' : self.exceptionCode
 0:06.45   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 4531, in wrapForType
 0:06.45     False))[0]
 0:06.45   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 4339, in getWrapTemplateForType
 0:06.45     raise MethodNotNewObjectError(descriptor.interface.identifier.name)
 0:06.45 Codegen.MethodNotNewObjectError

Note that this doesn't even fail the build, the bindings are just not regenerated.
Keywords: regression
This reeks of a process exit code not being issued correctly.
Parse errors in WebIDL are also silently doing nothing to stop the build.

Ehsan is on Mac seeing this; I am too.
Severity: normal → blocker
This is a "feature" of make:

> If an included makefile cannot be found in any of these directories, a warning message is
> generated, but it is not an immediately fatal error; processing of the makefile containing
> the include continues.

I can confirm from the output of `make -d` that we go to rebuild the codegen.pp target, that process exits with non-0 exit code (expected if we raise), and make happily continues processing using the old codegen.pp. This is why we see an apparent loop of codegen failures during builds.

Solution is to rejiggle the make targets so codegen isn't hooked up to an "include" target.

Seriously, I hate make so much.
Make blissfully ignores failures when processing targets of "include"
directives. This was causing failures of WebIDL codegen to be ignored
and causing make to get in a loop.
Attachment #8348147 - Flags: review?(ted)
Attachment #8348147 - Flags: review?(ted) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cd353bda680

Thanks for the speedy review, Ted. FWIW, this was also likely the cause of the weird loop observed in bug 949875.
Blocks: 949875
Status: NEW → ASSIGNED
Flags: in-testsuite-
I'm still getting this exact problem even in a tree that contains this patch....
Attachment #8348270 - Attachment mime type: application/x-tar → application/gzip
From the uploaded log:

WebIDL.WebIDLError: error: Syntax Error at end of file. Possibly due to missing semicolon(;), braces(}) or both, /Users/bzbarsky/mozilla/inbound/mozilla/dom/webidl/ImageData.webidl
Reaping losing child 0x7ff8714f4e10 PID 93236 
Removing child 0x7ff8714f4e10 PID 93236 from chain.
 Considering target file `codegen.pp'.
   Considering target file `codegen.done'.
   Recently tried and failed to update file `codegen.done'.
  Finished prerequisites of target file `codegen.pp'.
 Giving up on target file `codegen.pp'.
Updating goal targets....
Considering target file `default'.

...

   Must remake target `codegen.done'.
Need a job token; we don't have children
Putting child 0x7ffeaa4633c0 (codegen.done) PID 93247 on the chain.
   Commands of `codegen.done' are being run.
  Finished prerequisites of target file `codegen.pp'.
 The prerequisites of `codegen.pp' are being made.

WebIDL.WebIDLError: error: Syntax Error at end of file. Possibly due to missing semicolon(;), braces(}) or both, /Users/bzbarsky/mozilla/inbound/mozilla/dom/webidl/ImageData.webidl
Reaping losing child 0x7ffeaa4633c0 PID 93247 
Removing child 0x7ffeaa4633c0 PID 93247 from chain.
 Considering target file `codegen.pp'.
   Considering target file `codegen.done'.
   Recently tried and failed to update file `codegen.done'.
  Finished prerequisites of target file `codegen.pp'.
 Giving up on target file `codegen.pp'.
Updating goal targets....
Considering target file `export'.


*sigh*. It looks like the same issue. Strangely, the patch seemed to work on my machine and I confirmed with bz our build configs are similar (/usr/bin/make version 3.8.1 on OS X). Most weird.

Perhaps we should have codegen.done chain up to export:: instead of an included file. Oy.
Whiteboard: [leave open]
Oh, son of a.

One of the pieces of review feedback from bug 928195 was to switch a hardcoded "include" into $(call include_deps). This, of course, uses "-include" instead of "include." The "-" causes make to ignore failures. Derp.

That's what I get for not leaving a comment on why we are using "include." Will patch this... again.
Glandium should be online soon, so will have him look at this.

This patch reverts part 1 and fixes the -include problem.

bz: Please confirm this fixes it for you. You may need to rm codegen.*
from obj/dom/bindings for the change to fully pick up. We shouldn't need
to touch CLOBBER since only build failures will trigger this little bug.
Attachment #8348289 - Flags: review?(mh+mozilla)
Attachment #8348289 - Flags: feedback?(bzbarsky)
Comment on attachment 8348289 [details] [diff] [review]
Part 2: Make build failures of codegen.pp fatal

Yes, this makes things happy, as far as I can tell.  Excellent!

Add a comment about why the 1 is there in dom/bindings/Makefile.in?
Attachment #8348289 - Flags: feedback?(bzbarsky) → feedback+
Comment on attachment 8348289 [details] [diff] [review]
Part 2: Make build failures of codegen.pp fatal

Review of attachment 8348289 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/baseconfig.mk
@@ +33,2 @@
>  else
> +include_deps = $(eval $(if $(2),,-)include $(1))

Please file a followup to make the lack of - the default.
Attachment #8348289 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/884b554dea2d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Blocks: 952206
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.