Add a stale .pyc cleanup step before running gyp

Add a stale .pyc cleanup step before running gyp

Clean up stale .pyc files for directories that contain (or contained)
scripts used during gyp files processing. The list was built manually
by searching for "<!@(python" in .gyp* files.

BUG=486158, 500078
Committed: https://crrev.com/b14f3a3d54090c7cc8937aa29efdd1190d3c9f0c
Cr-Commit-Position: refs/heads/master@{#334310}

[mnaganov (inactive), 2015-06-12 22:50:15]

mnaganov@chromium.org changed reviewers:
+ scottmg@chromium.org

[mnaganov (inactive), 2015-06-12 22:50:16]

Hi Scott,

WDYT? Generating the list automatically of course seems better, but isn't very
trivial, because a lot of time scripts are referenced using gyp variables. So,
essentially, we need gyp to be able to emit the list of scripts it needs to run
before generating project files :)

[scottmg, 2015-06-12 22:58:25]

scottmg@chromium.org changed reviewers:
+ maruel@chromium.org

[scottmg, 2015-06-12 22:58:25]

I think this is probably fine (the manual list is a bit unfortunate but not the
end of the world), but it seems like it'd be better to set
https://docs.python.org/2/using/cmdline.html#envvar-PYTHONDONTWRITEBYTECODE at
the beginning of gclient invocations since that appears to be what we actually
want anyway. Did you consider that and decide against it?

+maruel who I know has wrestled with this annoyance before.

https://codereview.chromium.org/1186593003/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode750
DEPS:750: # Ensure that while generating dependencies lists in .gyp files we
don't accidentally
nit; Wrap at 80 col since there's nothing preventing it here.

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode752
DEPS:752: 'name': 'remove_stale_pyc_files_for_gyp',
Probably just 'remove_stale_pyc_files' is better.

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode763
DEPS:763: ],
I was worried this would be kind of slow, but on Windows it's not that bad:

tim cmd /c python tools\remove_stale_pyc_files.py android_webview/tools
gpu/gles2_conform_support ppapi printing third_party/closure_compiler/build
tools
real: 0m0.453s

(Doing '.' on the other hand takes 18s, so that's no good.)

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode784
DEPS:784: # Ensure that we don't accidentally reference any .pyc files whose
Seems unnecessary to do this here too?

[mnaganov (inactive), 2015-06-12 23:26:17]

> Did you consider that and decide against it?

I think this option prevents Python from *generating* .pyc files, not from
sucking them in. E.g. in theory you can have a third-party lib with closed
source, and I guess, setting this flag should not be preventing you from using
it.

https://codereview.chromium.org/1186593003/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode750
DEPS:750: # Ensure that while generating dependencies lists in .gyp files we
don't accidentally
On 2015/06/12 22:58:25, scottmg wrote:
> nit; Wrap at 80 col since there's nothing preventing it here.

Done.

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode752
DEPS:752: 'name': 'remove_stale_pyc_files_for_gyp',
On 2015/06/12 22:58:25, scottmg wrote:
> Probably just 'remove_stale_pyc_files' is better.

Done.

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode763
DEPS:763: ],
On 2015/06/12 22:58:25, scottmg wrote:
> I was worried this would be kind of slow, but on Windows it's not that bad:
> 
> tim cmd /c python tools\remove_stale_pyc_files.py android_webview/tools
> gpu/gles2_conform_support ppapi printing third_party/closure_compiler/build
> tools
> real: 0m0.453s
> 
> (Doing '.' on the other hand takes 18s, so that's no good.)

RemoveAllStalePycFiles doesn't skip output dirs, and third_party dirs can also
be quite big (e.g. layout test results). It may be possible to parallelize this
task, but that's not easy due to recursive nature of the traversal.

https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode784
DEPS:784: # Ensure that we don't accidentally reference any .pyc files whose
On 2015/06/12 22:58:25, scottmg wrote:
> Seems unnecessary to do this here too?

Right. I guess, nobody deletes .py files in hooks.

[M-A Ruel, 2015-06-12 23:27:54]

rubberstamp lgtm

[scottmg, 2015-06-12 23:28:39]

On 2015/06/12 23:26:17, mnaganov wrote:
> > Did you consider that and decide against it?
> 
> I think this option prevents Python from *generating* .pyc files, not from
> sucking them in. E.g. in theory you can have a third-party lib with closed
> source, and I guess, setting this flag should not be preventing you from using
> it.

Sure, but in general it would be equivalent for our purposes? (other than this
one transition point where it won't help)

> 
> https://codereview.chromium.org/1186593003/diff/20001/DEPS
> File DEPS (right):
> 
> https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode750
> DEPS:750: # Ensure that while generating dependencies lists in .gyp files we
> don't accidentally
> On 2015/06/12 22:58:25, scottmg wrote:
> > nit; Wrap at 80 col since there's nothing preventing it here.
> 
> Done.
> 
> https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode752
> DEPS:752: 'name': 'remove_stale_pyc_files_for_gyp',
> On 2015/06/12 22:58:25, scottmg wrote:
> > Probably just 'remove_stale_pyc_files' is better.
> 
> Done.
> 
> https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode763
> DEPS:763: ],
> On 2015/06/12 22:58:25, scottmg wrote:
> > I was worried this would be kind of slow, but on Windows it's not that bad:
> > 
> > tim cmd /c python tools\remove_stale_pyc_files.py android_webview/tools
> > gpu/gles2_conform_support ppapi printing third_party/closure_compiler/build
> > tools
> > real: 0m0.453s
> > 
> > (Doing '.' on the other hand takes 18s, so that's no good.)
> 
> RemoveAllStalePycFiles doesn't skip output dirs, and third_party dirs can also
> be quite big (e.g. layout test results). It may be possible to parallelize
this
> task, but that's not easy due to recursive nature of the traversal.
> 
> https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode784
> DEPS:784: # Ensure that we don't accidentally reference any .pyc files whose
> On 2015/06/12 22:58:25, scottmg wrote:
> > Seems unnecessary to do this here too?
> 
> Right. I guess, nobody deletes .py files in hooks.

[scottmg, 2015-06-12 23:44:31]

lgtm

[mnaganov (inactive), 2015-06-12 23:45:07]

On 2015/06/12 23:28:39, scottmg wrote:
> On 2015/06/12 23:26:17, mnaganov wrote:
> > > Did you consider that and decide against it?
> > 
> > I think this option prevents Python from *generating* .pyc files, not from
> > sucking them in. E.g. in theory you can have a third-party lib with closed
> > source, and I guess, setting this flag should not be preventing you from
using
> > it.
> 
> Sure, but in general it would be equivalent for our purposes? (other than this
> one transition point where it won't help)
> 

Yeah. My primary intention is to resolve this particular issue with the stale
.pyc
file.

I filed a bug for evaluating feasibility of avoiding generating .pyc files.

> > 
> > https://codereview.chromium.org/1186593003/diff/20001/DEPS
> > File DEPS (right):
> > 
> > https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode750
> > DEPS:750: # Ensure that while generating dependencies lists in .gyp files we
> > don't accidentally
> > On 2015/06/12 22:58:25, scottmg wrote:
> > > nit; Wrap at 80 col since there's nothing preventing it here.
> > 
> > Done.
> > 
> > https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode752
> > DEPS:752: 'name': 'remove_stale_pyc_files_for_gyp',
> > On 2015/06/12 22:58:25, scottmg wrote:
> > > Probably just 'remove_stale_pyc_files' is better.
> > 
> > Done.
> > 
> > https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode763
> > DEPS:763: ],
> > On 2015/06/12 22:58:25, scottmg wrote:
> > > I was worried this would be kind of slow, but on Windows it's not that
bad:
> > > 
> > > tim cmd /c python tools\remove_stale_pyc_files.py android_webview/tools
> > > gpu/gles2_conform_support ppapi printing
third_party/closure_compiler/build
> > > tools
> > > real: 0m0.453s
> > > 
> > > (Doing '.' on the other hand takes 18s, so that's no good.)
> > 
> > RemoveAllStalePycFiles doesn't skip output dirs, and third_party dirs can
also
> > be quite big (e.g. layout test results). It may be possible to parallelize
> this
> > task, but that's not easy due to recursive nature of the traversal.
> > 
> > https://codereview.chromium.org/1186593003/diff/20001/DEPS#newcode784
> > DEPS:784: # Ensure that we don't accidentally reference any .pyc files whose
> > On 2015/06/12 22:58:25, scottmg wrote:
> > > Seems unnecessary to do this here too?
> > 
> > Right. I guess, nobody deletes .py files in hooks.

[mnaganov (inactive), 2015-06-12 23:46:51]

The CQ bit was checked by mnaganov@chromium.org

[mnaganov (inactive), 2015-06-12 23:46:51]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/1186593003/#ps60001
(title: "Added a bug reference")

[commit-bot: I haz the power, 2015-06-12 23:48:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1186593003/60001

[commit-bot: I haz the power, 2015-06-13 01:14:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-13 01:15:45]

Patchset 4 (id:??) landed as
https://crrev.com/b14f3a3d54090c7cc8937aa29efdd1190d3c9f0c
Cr-Commit-Position: refs/heads/master@{#334310}