Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 397909 blink_tests target never becomes clean, binding generation scripts re-run every build
Starred by 2 users Project Member Reported by tha...@chromium.org, Jul 28 2014 Back to list
Status: Fixed
Owner:
Closed: Jul 2014
Cc:
Components:
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression


Sign in to add a comment
In the below, I've interrupted my build (via ctrl-c) somewhere in content a few times, and then started building blink_tests again immediately.

Note how the build always starts at a bit over 1800 steps to run, even though I waited for about 600 steps to complete each time (lots of .idl compilations):


Nicos-MacBook-Pro:src thakis$ time caffeinate ninja -C out/Release blink_tests
ninja: Entering directory `out/Release'
[683/1819] CXX obj/content/browser/frame_host/content.navigator_delegate.o^C
ninja: build stopped: interrupted by user.

real	0m33.841s
user	3m10.364s
sys	0m33.582s

Nicos-MacBook-Pro:src thakis$ time caffeinate ninja -C out/Release blink_tests
ninja: Entering directory `out/Release'
[760/1802] CXX obj/content/browser/media/capture/content.web_contents_capture_util.o^C
ninja: build stopped: interrupted by user.

real	0m59.291s
user	6m22.087s
sys	0m43.632s

Nicos-MacBook-Pro:src thakis$ time caffeinate ninja -C out/Release blink_tests
ninja: Entering directory `out/Release'
[680/1708] CXX obj/content/browser/net/content.browser_online_state_observer.o^C
ninja: build stopped: interrupted by user.

real	0m35.023s
user	3m4.746s
sys	0m34.232s

Nicos-MacBook-Pro:src thakis$ time caffeinate ninja -C out/Release blink_tests
ninja: Entering directory `out/Release'
[86/1879] RULE Generating binding from ../../../modules/mediastream/RTCErrorCallback.i
 
Comment 1 by tha...@chromium.org, Jul 28 2014
Even if I don't interrupt my build halfway through and it just fails because of a compile error somewhere, this happens too.
Comment 2 by tha...@chromium.org, Jul 28 2014
Cc: vivek...@samsung.com
Labels: -Type-Bug Type-Bug-Regression
The top of `-d explain` output:

$ time caffeinate ninja -C out/Release -k 0 unit_tests -d explain
ninja: Entering directory `out/Release'
ninja explain: gen/ui/ui_resources/grit/ui_resources.h is dirty
ninja explain: output gen/blink/bindings/scripts/lextab.py doesn't exist
ninja explain: gen/blink/bindings/scripts/lextab.py is dirty
ninja explain: gen/blink/bindings/scripts/parsetab.pickle is dirty
ninja explain: gen/blink/bindings/scripts/lextab.py is dirty
ninja explain: gen/blink/bindings/scripts/parsetab.pickle is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/scripts/cached_lex_yacc_tables.actions_rules_copies.stamp is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8Animation.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8Animation.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationEffect.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationEffect.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationPlayer.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationPlayer.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationNode.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationNode.h is dirty

As it lists a grit file first, maybe this is related to the work on issue 312586?
Comment 3 by nbarth@chromium.org, Jul 28 2014
Cc: bashi@chromium.org
Comment 4 by tha...@chromium.org, Jul 28 2014
Summary: blink_tests target never becomes clean, binding generation scripts re-run every build (was: Binding generation scripts re-run if I interrupt the build after they have all completed and some part of content is building)
Even after a build of `blink_tests` completes, 800 .idl rules rerun on the next build. The build of blink_tests never becomes clean.

Explain output for just that:

$ time caffeinate ninja -C out/Release -k 0 blink_tests -d explain
ninja: Entering directory `out/Release'
ninja explain: output gen/blink/bindings/scripts/lextab.py doesn't exist
ninja explain: gen/blink/bindings/scripts/lextab.py is dirty
ninja explain: gen/blink/bindings/scripts/parsetab.pickle is dirty
ninja explain: gen/blink/bindings/scripts/lextab.py is dirty
ninja explain: gen/blink/bindings/scripts/parsetab.pickle is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/scripts/cached_lex_yacc_tables.actions_rules_copies.stamp is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8Animation.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8Animation.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationEffect.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationEffect.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationPlayer.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationPlayer.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_individual.binding.stamp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationNode.cpp is dirty
ninja explain: gen/blink/bindings/core/v8/V8AnimationNode.h is dirty
ninja explain: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_v8_generated_indi



Looks like lextab.py isn't generated, even though some gyp file claims that it should get generated?
Comment 5 by tha...@chromium.org, Jul 28 2014
Cc: j...@opera.com
jl@ touched code in this area about 5 weeks ago: http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/blink_idl_parser.py?view=log#revHEAD (but I only started seeing this now, so that's probably not the direct cause)
Comment 6 by tha...@chromium.org, Jul 28 2014
cached_lex_yacc_tables is a smaller target that doesn't become clean for me (it contains only 2 build steps, so it's a faster repro)
Comment 7 by tha...@chromium.org, Jul 28 2014
Looks like third_party/ply/lex.py tries to read lextab.py by doing "import lextab", and if that succeeds doesn't try to write it. In my case, `out/Release/gen/blink/bindings/lextab.py` exists for some reason, so I'm guessing the import finds that and ply is happy, but the build system isn't.

How is this case supposed to be handled?

…hm, even removing that existing lextab.py doesn't make things go??

Oh! There's still out/Release/gen/blink/bindings/scripts/lextab.pyc which satisfies the import. If I remove the two pyc files (
$ find out/Release -name lextab.pyc
out/Release/gen/blink/bindings/lextab.pyc
out/Release/gen/blink/bindings/scripts/lextab.pyc
) then the build becomes happy.

So I guess repro steps are e.g. "rm out/Release/gen/blink/bindings/scripts/lextab.py && ninja -C out/Release cached_lex_yacc_tables" (just checked; that does repro the bug. Removing the pyc file makes it go away again.)

I don't know how I ended up in this state. I don't think I did anything weird (like deleting parts of my build directory.)

Please make sure that this is handled in some robust way.
Comment 8 by j...@opera.com, Jul 29 2014
Labels: -OS-Mac OS-All
Owner: j...@opera.com
Status: Started
Comment 9 by bashi@chromium.org, Jul 29 2014
I can't figure out why you had lextab.pyc in gen/blink/bindings/ (maybe we put the .py file there at some point?). I uploaded a CL to make the generation a bit robust, but we may want to be more conservative, like traverse all directories under gen/blink/bindings and remove related *.pyc when we try to cache the lex table. WDYT?

https://codereview.chromium.org/429523002/
Comment 10 by bashi@chromium.org, Jul 29 2014
Oops, jl@ has started working on this... I guess he has a good idea.
Comment 11 by j...@opera.com, Jul 29 2014
bashi@: I did essentially the same thing as you did, in https://codereview.chromium.org/425953002/.
Comment 12 by bashi@chromium.org, Jul 29 2014
Cc: haraken@chromium.org
jl@: Thanks! I'm going to close my CL:)
Comment 13 by j...@opera.com, Jul 29 2014
One way to end up in the state that triggers the bug is to run "ninja -t clean". The lextab.py file itself is listed as one of the cached_lex_yacc_tables action's outputs, but not related lextab.py* files. Aborting the build while that action is running could also possibly do the trick.
Project Member Comment 14 by bugdroid1@chromium.org, Jul 29 2014
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=179129

------------------------------------------------------------------
r179129 | jl@opera.com | 2014-07-29T12:17:47.042871Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/blink_idl_parser.py?r1=179129&r2=179128&pathrev=179129
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/blink_idl_lexer.py?r1=179129&r2=179128&pathrev=179129

IDL parser: fix rebuilding of (stale) cached lexer tables

The PLY lexer caches its table as a Python module (lextab.py). The
cache is loaded by attempting to import the module, which succeeds
if there's a lextab.py{,c,o} anywhere in the Python path. If the
import succeeds, the cache is assumed by PLY to be up-to-date, so
to update the cache, we need to remove those files before calling
PLY to initialize the lexer.

This patch makes blink_idl_lexer.main() do just that. It also
extends blink_idl_parser.main() to call the former, since the GYP
(and GN) build systems only actually call the latter script.

BUG= 397909 

Review URL: https://codereview.chromium.org/425953002
-----------------------------------------------------------------
Comment 15 by j...@opera.com, Jul 29 2014
Status: Fixed
Comment 16 by nbarth@google.com, Jul 29 2014
Should we also list
lextab.pyc
as an output?
...as this would presumably fix the issue with ninja -t clean?
Comment 17 by j...@opera.com, Jul 29 2014
We could do that (and we should then also import 'lextab' when updating the cache so that it really is an output.) It would be cleaner, for sure.

But what we do now (after the fix) would still be necessary since PLY doesn't have any dependency tracking or validation of its own; IOW we stil need to make sure 'lextab' can't be imported (by deleting all lextab.py*) before calling into PLY.
Yup, Python code changes also necessary, as you note (>.<)

...but perhaps GYP/GN change (and import one) in follow-up?

(Thanks for fixing these!)
Thanks much for the fix!
Project Member Comment 20 by bugdroid1@chromium.org, Aug 11 2014
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=179971

------------------------------------------------------------------
r179971 | jl@opera.com | 2014-08-11T17:13:02.134402Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/blink_idl_lexer.py?r1=179971&r2=179970&pathrev=179971

IDL parser: clean up cached lexer table updating code

Follow-up to
  https://codereview.chromium.org/425953002/
addressing some late comments.

BUG= 397909 

Review URL: https://codereview.chromium.org/462613004
-----------------------------------------------------------------
BTW, I've updated the docs:
http://www.chromium.org/developers/generated-files
...linking here, so hopefully this'll be avoided (or more easily fixed) in future.
Project Member Comment 22 by bugdroid1@chromium.org, Aug 13 2014
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=180142

------------------------------------------------------------------
r180142 | jl@opera.com | 2014-08-13T06:03:45.687185Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/BUILD.gn?r1=180142&r2=180141&pathrev=180142
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/scripts.gyp?r1=180142&r2=180141&pathrev=180142

Add lextab.pyc as an output of the cached_lex_yacc_tables action

The primary output is lextab.py, but since it is imported, Python also
writes lextab.pyc. Listing it too as an output means it won't be left
around by "ninja -t clean" (and not much else, I think.)

Prior to the patch
  https://codereview.chromium.org/425953002/
the result of not having lextab.pyc listed as an output and running
"ninja -t clean" was a tree that would build correctly, but that would
always rerun all the IDL code generation scripts, and thus never become
"clean".

BUG= 397909 

Review URL: https://codereview.chromium.org/463063003
-----------------------------------------------------------------
Project Member Comment 23 by bugdroid1@chromium.org, Aug 15 2014
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=180329

------------------------------------------------------------------
r180329 | jl@opera.com | 2014-08-15T06:10:51.838498Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/scripts.gyp?r1=180329&r2=180328&pathrev=180329
   M http://src.chromium.org/viewvc/blink/trunk/Source/bindings/scripts/BUILD.gn?r1=180329&r2=180328&pathrev=180329

Revert of Add lextab.pyc as an output of the cached_lex_yacc_tables action (patchset #1 of https://codereview.chromium.org/463063003/)

Reason for revert:
This CL added the assumption that Python will write the lextab.pyc file. If that's a wrong assumption, which seems to be the case in some situations, the result is that the cached_lex_yacc_tables target never becomes clean, which in turn means that all IDL files are recompiled on each build.

The problem fixed by this CL was cosmetic only, so is not worth the risk.

Original issue's description:
> Add lextab.pyc as an output of the cached_lex_yacc_tables action
> 
> The primary output is lextab.py, but since it is imported, Python also
> writes lextab.pyc. Listing it too as an output means it won't be left
> around by "ninja -t clean" (and not much else, I think.)
> 
> Prior to the patch
>   https://codereview.chromium.org/425953002/
> the result of not having lextab.pyc listed as an output and running
> "ninja -t clean" was a tree that would build correctly, but that would
> always rerun all the IDL code generation scripts, and thus never become
> "clean".
> 
> BUG= 397909 
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180142

TBR=haraken@chromium.org,nbarth@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG= 397909 

Review URL: https://codereview.chromium.org/468743003
-----------------------------------------------------------------
Sign in to add a comment