Add a wrapper script which modifies the .isolate on the fly.

Add a wrapper script which modifies the .isolate on the fly.

This tool will ultimately process the .ninja files, in the meantime it archives
all the DLLs. It's cheezy but this will unblock the component build short term.
This is very crude but this will get folks going. This means once it's done, we
can remove all the .isolate representing the dynamic libraries.

Remove swarming_client/ as a dependency in the .isolate files, it's not needed
anymore.

R=vadimsh@chromium.org
BUG=333473
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261652

[Ken Russell (switch to Gerrit), 2014-03-13 22:29:53]

Cool! Thanks for moving this forward. It will be great to eliminate the src/
checkout on the Debug GPU bots.

[Vadim Sh., 2014-03-14 01:51:35]

I was thinking we will add some additional parameter to *.isolate that enables
this mechanism.

Without it *.isolate will look like a valid one, while in fact it will required
this driver to do something meaningful.

With additional parameter we at least making it backward incompatible (which is
a good thing, imho, in this case).

Something like:
{
  'variables': {
    'isolate_dependency_external': 'ninja'
  },
}

I dunno... wdyt?

https://codereview.chromium.org/196283016/diff/60001/base/base_unittests.isolate
File base/base_unittests.isolate (left):

https://codereview.chromium.org/196283016/diff/60001/base/base_unittests.isol...
base/base_unittests.isolate:35: 'isolate_dependency_untracked': [
We kept it only for googletest/run_test_cases.py stuff, right?

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:6: """Adaptor script called through build/isolate.gyp.
isolate.gypi (with 'i')?

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:36: cwd = os.getcwd()
What is cwd? Does ninja gives any guarantees about it?

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:47: isolate_content += (
that's is very hacky... why not exec it, modify as python and pprint it back? no
one cares about comments and formating in generated files.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:69: for i, arg in enumerate(args):
why not optparse? It's even imported.

[M-A Ruel, 2014-03-14 12:51:56]

On 2014/03/14 01:51:35, Vadim Sh. wrote:
> I was thinking we will add some additional parameter to *.isolate that enables
> this mechanism.
> 
> Without it *.isolate will look like a valid one, while in fact it will
required
> this driver to do something meaningful.
> 
> With additional parameter we at least making it backward incompatible (which
is
> a good thing, imho, in this case).
> 
> Something like:
> {
>   'variables': {
>     'isolate_dependency_external': 'ninja'
>   },
> }
> 
> I dunno... wdyt?

There was a bug in the code. I only want to do it in component=shared_library
mode because in that case the situation is a mess. I still want the .isolate to
be correct when the build is in static mode. Fixed the code to only mangle when
in shared_library.

Once I do the proper ninja parsing, I'll probably refactor significantly more.
But at this this solution gives us a lot of breathing room, and I'll delete a
few .isolate files along the way, i.e. all the ones having .so that are only
references in shared_library.

https://codereview.chromium.org/196283016/diff/60001/base/base_unittests.isolate
File base/base_unittests.isolate (left):

https://codereview.chromium.org/196283016/diff/60001/base/base_unittests.isol...
base/base_unittests.isolate:35: 'isolate_dependency_untracked': [
On 2014/03/14 01:51:35, Vadim Sh. wrote:
> We kept it only for googletest/run_test_cases.py stuff, right?

Yep, and Pawel replaced it with test launcher a while ago.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:6: """Adaptor script called through build/isolate.gyp.
On 2014/03/14 01:51:35, Vadim Sh. wrote:
> isolate.gypi (with 'i')?

Done.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:36: cwd = os.getcwd()
On 2014/03/14 01:51:35, Vadim Sh. wrote:
> What is cwd? Does ninja gives any guarantees about it?

ninja guarantees the script will be called from the directory containing the
.isolate file. At least if this assumption breaks, it'll be very visible. :) The
paths in the .isolate file are also relative to its directory.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:47: isolate_content += (
On 2014/03/14 01:51:35, Vadim Sh. wrote:
> that's is very hacky... why not exec it, modify as python and pprint it back?
no
> one cares about comments and formating in generated files.

There's a few options;
1. Hack the way it's currently done. Note that the step will immediately break
if the hack breaks.
2. Use eval() as you suggested.
3. Use isolate_format.py as a library to load the file and write it back. It's
the safest approach but makes refactoring to isolate_format.py more annoying
afterward.

As a matter of fact, #2 doesn't buy us much from #1. Both are huge ugly hacks.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:69: for i, arg in enumerate(args):
On 2014/03/14 01:51:35, Vadim Sh. wrote:
> why not optparse? It's even imported.

Yeah I thought about using it first and proxying all the flags but it's more
trouble than worth. In practice some flags are tricky to implement (e.g.
--*-variable) as they accept a variable number of arguments. In the end, this
script is what it is: a big hack. So the shorter the better.

[Vadim Sh., 2014-03-14 18:01:38]

lgtm, assuming it passes try server :)

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:36: cwd = os.getcwd()
On 2014/03/14 12:51:56, M-A Ruel wrote:
> On 2014/03/14 01:51:35, Vadim Sh. wrote:
> > What is cwd? Does ninja gives any guarantees about it?
> 
> ninja guarantees the script will be called from the directory containing the
> .isolate file. At least if this assumption breaks, it'll be very visible. :)
The
> paths in the .isolate file are also relative to its directory.

Please add a comment describing what cwd is expected to be.

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:47: isolate_content += (
On 2014/03/14 12:51:56, M-A Ruel wrote:
> On 2014/03/14 01:51:35, Vadim Sh. wrote:
> > that's is very hacky... why not exec it, modify as python and pprint it
back?
> no
> > one cares about comments and formating in generated files.
> 
> There's a few options;
> 1. Hack the way it's currently done. Note that the step will immediately break
> if the hack breaks.
> 2. Use eval() as you suggested.
> 3. Use isolate_format.py as a library to load the file and write it back. It's
> the safest approach but makes refactoring to isolate_format.py more annoying
> afterward.
> 
> As a matter of fact, #2 doesn't buy us much from #1. Both are huge ugly hacks.

I think #1 is more fragile than #2. It's a big relieve that try server covers
changes to *.isolate :)

https://codereview.chromium.org/196283016/diff/60001/tools/isolate_driver.py#...
tools/isolate_driver.py:69: for i, arg in enumerate(args):
On 2014/03/14 12:51:56, M-A Ruel wrote:
> On 2014/03/14 01:51:35, Vadim Sh. wrote:
> > why not optparse? It's even imported.
> 
> Yeah I thought about using it first and proxying all the flags but it's more
> trouble than worth. In practice some flags are tricky to implement (e.g.
> --*-variable) as they accept a variable number of arguments. In the end, this
> script is what it is: a big hack. So the shorter the better.
Yeah, makes sense.

[M-A Ruel, 2014-03-30 02:31:55]

Rewrote, tested on linux, will test on mac&win tomorrow to double-check it works
fine.

I changed the code so no mangling is needed anymore, it's much cleaner.

[Vadim Sh., 2014-03-31 18:21:53]

lgtm

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py...
tools/isolate_driver.py:34: SRC_DIR = os.path.dirname(TOOLS_DIR)
minor nit: move this right after SWARMING_CLIENT_DIR? So all *_DIR constants are
in one block.

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py...
tools/isolate_driver.py:108: print ' '.join(sys.argv)
Is it helpful?

[M-A Ruel, 2014-04-04 00:35:18]

Looks like the DEPS roll is sticking now so CQ'ing.

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py...
tools/isolate_driver.py:34: SRC_DIR = os.path.dirname(TOOLS_DIR)
On 2014/03/31 18:21:53, Vadim Sh. wrote:
> minor nit: move this right after SWARMING_CLIENT_DIR? So all *_DIR constants
are
> in one block. 

Done.

https://codereview.chromium.org/196283016/diff/140001/tools/isolate_driver.py...
tools/isolate_driver.py:108: print ' '.join(sys.argv)
On 2014/03/31 18:21:53, Vadim Sh. wrote:
> Is it helpful?

No, I had forgot it from debugging.

[M-A Ruel, 2014-04-04 00:35:49]

The CQ bit was checked by maruel@chromium.org

[commit-bot: I haz the power, 2014-04-04 00:36:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/196283016/160001

[commit-bot: I haz the power, 2014-04-04 03:44:04]

Change committed as 261652