New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 730866 link

Starred by 1 user
Status: Fixed
Owner:
dpa...@chromium.org
Closed: Jul 2017
Cc:
█ tbreisacher@chromium.org
dbeam@chromium.org
dpa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

compile_js2.gypi swallows certain type of errors.

Project Member Reported by dpa...@chromium.org, Jun 7 2017 
See https://codereview.chromium.org/2930773003 for repro case. Pasting output below

> ./third_party/closure_compiler/run_compiler search_settings
ninja: Entering directory `out/Default'
[1/1] ACTION Compiling chrome/browser/resources/settings/search_settings.js
(INFO) Externs: ['../../../../third_party/closure_compiler/externs/polymer-1.0.js', '../../../../third_party/closure_compiler/externs/chrome_send.js']
(INFO) Dependencies: ['../../../../ui/webui/resources/js/assert.js', '../../../../ui/webui/resources/js/promise_resolver.js', '../../../../ui/webui/resources/js/cr.js']
(INFO) Target: search_settings.js
(INFO) Meta file: /tmp/tmpMNpJLx
(INFO) Expanded file: /tmp/tmpMX31Iu
(INFO) Args: --externs=../../../../third_party/closure_compiler/externs/polymer-1.0.js --externs=../../../../third_party/closure_compiler/externs/chrome_send.js --js=/tmp/tmpMX31Iu --compilation_level=SIMPLE_OPTIMIZATIONS --extra_annotation_name=attribute --extra_annotation_name=demo --extra_annotation_name=element --extra_annotation_name=group --extra_annotation_name=hero --extra_annotation_name=homepage --extra_annotation_name=status --extra_annotation_name=submodule --jscomp_error=accessControls --jscomp_error=ambiguousFunctionDecl --jscomp_error=checkTypes --jscomp_error=checkVars --jscomp_error=constantProperty --jscomp_error=deprecated --jscomp_error=externsValidation --jscomp_error=globalThis --jscomp_error=invalidCasts --jscomp_error=missingProperties --jscomp_error=missingReturn --jscomp_error=nonStandardJsDocs --jscomp_error=suspiciousCode --jscomp_error=undefinedNames --jscomp_error=undefinedVars --jscomp_error=unknownDefines --jscomp_error=uselessCode --jscomp_error=visibility --language_in=ECMASCRIPT_NEXT --language_out=ECMASCRIPT_NEXT --checks_only --chrome_pass --polymer_pass --source_map_format=V3 --jscomp_off=duplicate --jscomp_off=misplacedTypeAnnotation --summary_detail_level=3 --continue_after_errors
(INFO) Running jar: java -jar -Xms1024m -client -XX:+TieredCompilation ../../../../third_party/closure_compiler/compiler/compiler.jar --externs=../../../../third_party/closure_compiler/externs/polymer-1.0.js --externs=../../../../third_party/closure_compiler/externs/chrome_send.js --js=/tmp/tmpMX31Iu --compilation_level=SIMPLE_OPTIMIZATIONS --extra_annotation_name=attribute --extra_annotation_name=demo --extra_annotation_name=element --extra_annotation_name=group --extra_annotation_name=hero --extra_annotation_name=homepage --extra_annotation_name=status --extra_annotation_name=submodule --jscomp_error=accessControls --jscomp_error=ambiguousFunctionDecl --jscomp_error=checkTypes --jscomp_error=checkVars --jscomp_error=constantProperty --jscomp_error=deprecated --jscomp_error=externsValidation --jscomp_error=globalThis --jscomp_error=invalidCasts --jscomp_error=missingProperties --jscomp_error=missingReturn --jscomp_error=nonStandardJsDocs --jscomp_error=suspiciousCode --jscomp_error=undefinedNames --jscomp_error=undefinedVars --jscomp_error=unknownDefines --jscomp_error=uselessCode --jscomp_error=visibility --language_in=ECMASCRIPT_NEXT --language_out=ECMASCRIPT_NEXT --checks_only --chrome_pass --polymer_pass --source_map_format=V3 --jscomp_off=duplicate --jscomp_off=misplacedTypeAnnotation --summary_detail_level=3 --continue_after_errors
(INFO) Summary: 0 error(s), 0 warning(s)
com.google.javascript.jscomp.CompilerOptionsPreprocessor$InvalidOptionsException: ES6 is only supported for transpilation to a lower ECMAScript version. Set --language_out to ES3, ES5, or ES5_STRICT.
	at com.google.javascript.jscomp.CompilerOptionsPreprocessor.preprocess(CompilerOptionsPreprocessor.java:54)
	at com.google.javascript.jscomp.Compiler.parseForCompilationInternal(Compiler.java:1004)
	at com.google.javascript.jscomp.Compiler.access$300(Compiler.java:99)
	at com.google.javascript.jscomp.Compiler$7.call(Compiler.java:988)
	at com.google.javascript.jscomp.Compiler$7.call(Compiler.java:985)
	at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:93)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
(INFO) Deleting temp files: /tmp/tmpMNpJLx, /tmp/tmpMX31Iu
> ./third_party/closure_compiler/run_compiler search_settings
ninja: Entering directory `out/Default'
ninja: no work to do.

Comment 1 by dpa...@chromium.org, Jun 7 2017 

Per offline discussion with dbeam.

- We can't rely solely on the java command's exit code, because there are some errors that we discard (https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile2.py?q=compile2.py&sq=package:chromium&dr&l=125).
- We can probably fix this issue by tweaking the regex at https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile2.py?q=compile2.py&sq=package:chromium&dr&l=276.

Comment 2 by dpa...@chromium.org, Jun 8 2017 

@dbeam: I added a print statement at https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile2.py?q=compile2.py&sq=package:chromium&dr&l=125, to inspect whether we actually filter out any errors for the currently compiled targets. It did not find anything.

I would like to deduce that this codepath is obsolete maybe and that we can simplify. Do you know any concrete examples of compile targets that would failed in the past without error filtering?

Also the comment about ignoring Promise related errors at line 131 seems obsolete, because we don't actually check for any "Promise" token in the error string (and the referenced github issue is marked as fixed).

Comment 3 by dpa...@chromium.org, Jun 8 2017 

Regarding Promise related errors, my previous comment was inaccurate. We do still check at https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile2.py?q=compile2.py&sq=package:chromium&dr&l=48. Perhaps we could start by removing those checks first.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e463990741a71d61a48cfe3dce6769b9d97c5164

commit e463990741a71d61a48cfe3dce6769b9d97c5164
Author: dpapad <dpapad@chromium.org>
Date: Thu Jun 08 20:24:20 2017

Closure compilation: Remove compile_js2.py error filtering.

Errors related to Promises have been addressed by
https://github.com/google/closure-compiler/issues/715.

The other type of errors
"Variable x first declared in ..."
does not seem to happen anymore.

BUG= 730866 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2932743002
Cr-Commit-Position: refs/heads/master@{#478069}

[modify] https://crrev.com/e463990741a71d61a48cfe3dce6769b9d97c5164/third_party/closure_compiler/compile2.py
[delete] https://crrev.com/dfb76f24c05c31ce433a30fc9b774b25c01c89da/third_party/closure_compiler/error_filter.py
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a670d56a08acfc570ce624a52f80237c2552e7c

commit 9a670d56a08acfc570ce624a52f80237c2552e7c
Author: dpapad <dpapad@chromium.org>
Date: Fri Jun 09 23:53:43 2017

Closure compilation: Modify compile_js2.py to not swallow errors.

Previously certain types of errors would not cause compile_js2.py to exit
with an error code. Only type-checking related errors would trigger an
error code, for example a InvalidOptionsException would erroneously not
be considered an error.

BUG= 730866 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2930773003
Cr-Commit-Position: refs/heads/master@{#478463}

[modify] https://crrev.com/9a670d56a08acfc570ce624a52f80237c2552e7c/third_party/closure_compiler/compile2.py

Comment 6 by dpa...@chromium.org, Jul 13 2017

Owner: dpa...@chromium.org
Status: Fixed (was: Available)
This should be fixed now.
►

Sign in to add a comment