New issue
Advanced search Search tips

Issue 720011 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

compile_js2.gypi does not recompile when <include> files change.

Project Member Reported by dpa...@chromium.org, May 9 2017

Issue description

This was detected by rbpotter@.

Repro steps:
1) Wait for https://codereview.chromium.org/2865633004 to land, or patch it locally (fixes remaining print preview compilation errors).
2) Execute ./third_party/closure_compiler/run_compiler print_preview
3) Modify any file that is included via an <include>, for example
   touch chrome/browser/resources/print_preview/print_preview/previewarea/margin_control.js
4) Re-execute step 2.

Expected: Target should  recompile
Actual: "Nothing to do" message.


This worked correctly in previous v1 GYP syntax. Specifically compare [1] and [2], where the latter invokes inputs.py script to correctly declare all <include>d files as input. On the other hand [1] does not seem to have any equivalent logic. 

[1] https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile_js2.gypi?l=56
[2] https://cs.chromium.org/chromium/src/third_party/closure_compiler/compile_js.gypi?l=48
 
Status: Available (was: Untriaged)
Correction to step 3 above. The correct path is
chrome/browser/resources/print_preview/previewarea/margin_control.js

Comment 3 by dbeam@chromium.org, May 9 2017

the fix for this is just basically run all the .js files (after gathering) through this type of thing:

https://cs.chromium.org/chromium/src/third_party/closure_compiler/build/inputs.py?l=95

preserving order and add those to 'inputs'

Comment 4 by dbeam@chromium.org, May 9 2017

fwiw: it seems mildly hard to pass the late bound (i.e. >@(_sources)) output to a python script, so just doing this:

https://gist.github.com/danbeam/f921f5f8574b61c02e6bc4242a3642fa

doesn't work (script error), changing to <@(_sources) fixes but likely only for first-level dependencies.

Comment 5 by dpa...@chromium.org, May 10 2017

Tried the gist posted in previous comment (and changed ">" to "<"). This caused another error to surface:
./third_party/closure_compiler/run_compiler print_preview
Traceback (most recent call last):
  File "../../../../../third_party/closure_compiler/build/get_includes.py", line 23, in <module>
    print "\n".join(GetIncludes(sys.argv[1:]))
  File "../../../../../third_party/closure_compiler/build/get_includes.py", line 17, in GetIncludes
    includes.update(processor.Processor(f).included_files)
  File "../../../../../third_party/closure_compiler/build/../processor.py", line 78, in __init__
    self._lines = self._get_file(source_file)
  File "../../../../../third_party/closure_compiler/build/../processor.py", line 103, in _get_file
    lines = FileCache.read(source_file).splitlines()
  File "../../../../../third_party/closure_compiler/build/../processor.py", line 44, in read
    self._cache[abs_file] = self._cache[abs_file] or open(abs_file, "r").read()
IOError: [Errno 2] No such file or directory: '/usr/local/google/home/dpapad/workspace/chromium1/src/third_party/polymer/v1_0/components-chromium/paper-dialog-behavior/paper-dialog-shared-styles-extracted.js'
gyp: Call to 'python ../../../../../third_party/closure_compiler/build/get_includes.py paper-dialog-shared-styles-extracted.js' returned exit status 1 while in third_party/polymer/v1_0/components-chromium/paper-dialog-behavior/compiled_resources2.gyp.


Note that paper-dialog-shared-styles-extracted.js does not exist, but we have a compiled target for it at [1], which probably related to the error. The target has been auto-generated by [2], which blindly creates a target for every HTML file, without checking if a corrosponding JS file actually exists.

[1] https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/paper-dialog-behavior/compiled_resources2.gyp?l=16
[2] https://cs.chromium.org/chromium/src/tools/polymer/generate_compiled_resources_gyp.py?q=generate_compiled_resources_gyp.py&dr&l=60

Comment 6 by dbeam@chromium.org, May 10 2017

yep, bug in the generation script. fix here:
https://codereview.chromium.org/2878483003
Project Member

Comment 7 by bugdroid1@chromium.org, May 10 2017

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

commit c741e88553c0e49469bf7b8ef18bde571dc1740c
Author: dbeam <dbeam@chromium.org>
Date: Wed May 10 23:45:42 2017

Polymer/Closure: fix bug in script that generates Closure GYP

Previously, .html files being passed to the script assumed that there
was an equivalent "crisped" -extracted.js. In attempting to get other
things working, we discovered there were a bunch of non-existent targets
being created (reading the files of each target_name was failing).
Because nobody actually used these targets, they didn't really hurt
anything but also shouldn't exist.

So I fixed the glitch.

R=dpapad@chromium.org
BUG= 720011 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true

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

[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/font-roboto/compiled_resources2.gyp
[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/iron-flex-layout/compiled_resources2.gyp
[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/iron-icons/compiled_resources2.gyp
[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/iron-test-helpers/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/neon-animation/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-dialog-behavior/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-dropdown-menu/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-input/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-item/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-material/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-menu/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-spinner/compiled_resources2.gyp
[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/paper-styles/classes/compiled_resources2.gyp
[delete] https://crrev.com/bb92571ebde21cea713ce66c5330f5907ae5cafc/third_party/polymer/v1_0/components-chromium/paper-styles/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/components-chromium/paper-tabs/compiled_resources2.gyp
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/third_party/polymer/v1_0/generate_gyp.sh
[modify] https://crrev.com/c741e88553c0e49469bf7b8ef18bde571dc1740c/tools/polymer/generate_compiled_resources_gyp.py

Comment 8 by dpa...@chromium.org, May 11 2017

FYI, candidate fix is at https://codereview.chromium.org/2875543003.
Project Member

Comment 9 by bugdroid1@chromium.org, May 11 2017

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

commit 078d397a6b7af9a6a41edc55e2567fc7520b235c
Author: dpapad <dpapad@chromium.org>
Date: Thu May 11 02:57:33 2017

WebUI: Fix compile_js2.py to retrigger compilation for <include> files.

credit fix to dbeam@

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

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

[modify] https://crrev.com/078d397a6b7af9a6a41edc55e2567fc7520b235c/chrome/browser/resources/print_preview/compiled_resources2.gyp
[add] https://crrev.com/078d397a6b7af9a6a41edc55e2567fc7520b235c/third_party/closure_compiler/build/get_includes.py
[modify] https://crrev.com/078d397a6b7af9a6a41edc55e2567fc7520b235c/third_party/closure_compiler/compile_js2.gypi

Status: Fixed (was: Available)

Sign in to add a comment