My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 397909: blink_tests target never becomes clean, binding generation scripts re-run every build
2 people starred this issue and may be notified of changes. Back to list
 
Project Member Reported by thakis@chromium.org, Jul 27, 2014
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
Jul 28, 2014
#1 thakis@chromium.org
Even if I don't interrupt my build halfway through and it just fails because of a compile error somewhere, this happens too.
Jul 28, 2014
#2 thakis@chromium.org
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?
Cc: vivek...@samsung.com
Labels: -Type-Bug Type-Bug-Regression
Jul 28, 2014
#3 nbarth@chromium.org
(No comment was entered for this change.)
Cc: ba...@chromium.org
Jul 28, 2014
#4 thakis@chromium.org
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?
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)
Jul 28, 2014
#5 thakis@chromium.org
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)
Cc: j...@opera.com
Jul 28, 2014
#6 thakis@chromium.org
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)
Jul 28, 2014
#7 thakis@chromium.org
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.
Jul 29, 2014
#8 j...@opera.com
(No comment was entered for this change.)
Status: Started
Owner: j...@opera.com
Labels: -OS-Mac OS-All
Jul 29, 2014
#9 ba...@chromium.org
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/
Jul 29, 2014
#10 ba...@chromium.org
Oops, jl@ has started working on this... I guess he has a good idea.
Jul 29, 2014
#11 j...@opera.com
bashi@: I did essentially the same thing as you did, in https://codereview.chromium.org/425953002/.
Jul 29, 2014
#12 ba...@chromium.org
jl@: Thanks! I'm going to close my CL:)
Cc: haraken@chromium.org
Jul 29, 2014
#13 j...@opera.com
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.
Jul 29, 2014
#14 bugdro...@chromium.org
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
-----------------------------------------------------------------
Jul 29, 2014
#15 j...@opera.com
(No comment was entered for this change.)
Status: Fixed
Jul 29, 2014
#16 nba...@google.com
Should we also list
lextab.pyc
as an output?
...as this would presumably fix the issue with ninja -t clean?
Jul 29, 2014
#17 j...@opera.com
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.
Jul 29, 2014
#18 nbarth@chromium.org
Yup, Python code changes also necessary, as you note (>.<)

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

(Thanks for fixing these!)
Jul 29, 2014
#19 thakis@chromium.org
Thanks much for the fix!
Aug 11, 2014
#20 bugdro...@chromium.org
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
-----------------------------------------------------------------
Aug 12, 2014
#21 nbarth@chromium.org
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.
Aug 12, 2014
#22 bugdro...@chromium.org
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
-----------------------------------------------------------------
Aug 14, 2014
#23 bugdro...@chromium.org
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

Powered by Google Project Hosting