Convert isolate.py to exclusively use .isolate files.

Convert isolate.py to exclusively use .isolate files.

Convert base_unittests_run and net_unittests_run to use the new format.

Rename isolate_files to isolate_dependency_tracked and ..._dirs to ..._untracked.

Rewrite merge_isolate.py to have fully deterministic and sane behavior, especially for 'else' conditions.

Convert 'python' to sys.executable when running the command.

Add a lot of unit tests.

Move hashtable default directory into dirname(result) + '/hashtable'.

R=rogerta@chromium.org
R=mark@chromium.org
R=rsleevi@chromium.org
BUG=98834
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131896

[M-A Ruel, 2012-04-09 20:16:22]

Sorry this is a pretty heavy CL but afterward they'll be much easier.

This one was complex as I wanted to convert the current units tests to use the
native isolate file format that is GYP based and convert the code at the same
time, resulting in a much larger CL.

Manually tested to work on OSX, Windows and Linux. Dependent on
https://chromiumcodereview.appspot.com/10027006

[Roger Tawa OOO till Jul 10th, 2012-04-11 15:15:03]

lgtm

http://codereview.chromium.org/10019014/diff/4001/tools/isolate/isolate.py
File tools/isolate/isolate.py (right):

http://codereview.chromium.org/10019014/diff/4001/tools/isolate/isolate.py#ne...
tools/isolate/isolate.py:172: # Used the skip recalculating the hash. Use the
most recent update time.
the --> to ?

http://codereview.chromium.org/10019014/diff/4001/tools/isolate/isolate.py#ne...
tools/isolate/isolate.py:274: 'trace': 2,
shouldn't you declare some constants instead of hard coding ints?

http://codereview.chromium.org/10019014/diff/4001/tools/isolate/merge_isolate.py
File tools/isolate/merge_isolate.py (right):

http://codereview.chromium.org/10019014/diff/4001/tools/isolate/merge_isolate...
tools/isolate/merge_isolate.py:178: dict(deptype, dict(dependency, set(OSes))
for easier processing.
also returns 'OSes' key in dict.  maybe the return value should be a tuple:
dict, set(OSes)

[M-A Ruel, 2012-04-11 15:52:18]

Roger, ptal. I created 3 constants with arbitrary values instead of using
numbers and modified invert_map, reduce_inputs and convert_map_to_gyp to accept
and/or return oses as a separate value.

Mark, FYI with base.gyp

Ryan, FYI with net.gyp

[Ryan Sleevi, 2012-04-11 17:53:24]

I'm not thrilled with all the uses of <(DEPTH). Is my understanding of why it's
being used (see below) correct?

http://codereview.chromium.org/10019014/diff/15001/net/net_unittests.isolate
File net/net_unittests.isolate (right):

http://codereview.chromium.org/10019014/diff/15001/net/net_unittests.isolate#...
net/net_unittests.isolate:10:
'<(DEPTH)/third_party/pyftpdlib/src/pyftpdlib/ftpserver.py',
Outside of the Chromium-Webkit-Port-In-Webkit, almost all of our GYP files use
relative paths, rather than <(DEPTH).

I seem to recall Mark specifically preferring them in a past CL - mark, am I
remembering correctly?

If you're trying to do this so that variables get properly absolutized, you
should just rename the variable so that it's treated as a pathspec - see
http://code.google.com/p/gyp/wiki/InputFormatReference#Pathname_Relativization

[M-A Ruel, 2012-04-11 18:22:29]

ptal

http://codereview.chromium.org/10019014/diff/15001/net/net_unittests.isolate
File net/net_unittests.isolate (right):

http://codereview.chromium.org/10019014/diff/15001/net/net_unittests.isolate#...
net/net_unittests.isolate:10:
'<(DEPTH)/third_party/pyftpdlib/src/pyftpdlib/ftpserver.py',
On 2012/04/11 17:53:24, Ryan Sleevi wrote:
> Outside of the Chromium-Webkit-Port-In-Webkit, almost all of our GYP files use
> relative paths, rather than <(DEPTH).

I don't mind much. I didn't realize DEPTH wasn't used anymore, I must be
sight-blind.

I replaced all DEPTH references with '..'. I haven't upgraded the tracing script
to use relative paths instead of DEPTH, I'll do it in a later change to not add
more complexity to this change.

[Mark Mentovai, 2012-04-11 18:34:10]

Some part of GYP might actually care that the extension of an include file is
'.gypi'.

(…checking…)

Nope, couldn’t find anything. The only checks are for '.gyp' (which is treated
differently from included files like .gypi and I guess now .isolate.)

OK, LGTM.

[Ryan Sleevi, 2012-04-11 18:37:03]

GYP files LGTM. One more comment below from taking a second look at it, but I
think it's fine assuming I'm understanding how you're using this.

http://codereview.chromium.org/10019014/diff/14002/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10019014/diff/14002/net/net.gyp#newcode1366
net/net.gyp:1366: '<@(isolate_dependency_tracked)',
In terms of the purest of pedantry, it might be good to also add a dependency on
net.isolate itself, since it influences the action. If net_unittests.isolate
changes on disk, you want the action re-run, correct?

Or does it not matter with how you're intending to use this target, since it's
just a meta-target and not a build product?

[M-A Ruel, 2012-04-11 18:41:44]

On 2012/04/11 18:34:10, Mark Mentovai wrote:
> Some part of GYP might actually care that the extension of an include file is
> '.gypi'.

I know I'm abusing it a bit. :) But doing it this way permits me to define a
strict subset of language and make it clear for users. Also it permits to reduce
the data from inside gyp and have isolate.py be able to parse it.

The fact that the language is a strict subset prompted me to use a different
extension. I can revisit this design if any issue arise.

http://codereview.chromium.org/10019014/diff/14002/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/10019014/diff/14002/net/net.gyp#newcode1366
net/net.gyp:1366: '<@(isolate_dependency_tracked)',
On 2012/04/11 18:37:04, Ryan Sleevi wrote:
> In terms of the purest of pedantry, it might be good to also add a dependency
on
> net.isolate itself, since it influences the action. If net_unittests.isolate
> changes on disk, you want the action re-run, correct?
> 
> Or does it not matter with how you're intending to use this target, since it's
> just a meta-target and not a build product?

Technically, if you edit the file, you need to re-run build/gyp_chromium. I
guess it'd make sense to have it be a dependency anyway so I added it.

[commit-bot: I haz the power, 2012-04-11 18:43:20]

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

[Mark Mentovai, 2012-04-11 18:43:32]

maruel@chromium.org wrote:

> On 2012/04/11 18:34:10, Mark Mentovai wrote:
>
>> Some part of GYP might actually care that the extension of an include
>> file is
>> '.gypi'.
>>
>
> I know I'm abusing it a bit. :) But doing it this way permits me to define
> a
> strict subset of language and make it clear for users. Also it permits to
> reduce
> the data from inside gyp and have isolate.py be able to parse it.
>
> The fact that the language is a strict subset prompted me to use a
> different
> extension. I can revisit this design if any issue arise.
>

As long as GYP doesn’t currently care about the extension, neither do I.

It doesn’t. Therefore…

[commit-bot: I haz the power, 2012-04-11 18:43:37]

Presubmit check for 10019014-14003 failed and returned exit status 1.

Running presubmit commit checks ...
************* Module merge_isolate_test
E1101:265,4:MergeGyp.test_reduce_inputs: Instance of 'MergeGyp' has no
'assertIs' member

** Presubmit Messages **
--tbr was specified, skipping OWNERS check

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Fix pylint errors first.

/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
failed!
Command
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
.........E
======================================================================
ERROR: test_trace (__main__.Isolate)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 220, in test_trace
    out = self._execute('ok.isolate', ['--mode', 'trace'], True)
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 129, in _execute
    raise CalledProcessError(p.returncode, cmd, out, cwd)
CalledProcessError: Command '['/home/chrome-bot/python2.6/bin/python',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate.py',
'--variable', 'DEPTH=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate',
'--result', '/tmp/tmp2o21ge/result',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/data/isolate/ok.isolate',
'--mode', 'trace']' returned non-zero exit status 2
cwd=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
Failure: 2
child: verify the test data files were mapped properly
['.svn', 'test_file2.txt', 'test_file1.txt'] != ['test_file1.txt',
'test_file2.txt']



----------------------------------------------------------------------
Ran 10 tests in 1.157s

FAILED (errors=1)


/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/merge_isolate_test.py
failed!
Command
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/merge_isolate_test.py
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
.......E...
======================================================================
ERROR: test_reduce_inputs (__main__.MergeGyp)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/merge_isolate_test.py",
line 265, in test_reduce_inputs
    self.assertIs(oses, actual_oses)
AttributeError: 'MergeGyp' object has no attribute 'assertIs'

----------------------------------------------------------------------
Ran 11 tests in 0.003s

FAILED (errors=1)


Presubmit checks took 7.7s to calculate.

[commit-bot: I haz the power, 2012-04-11 18:54:30]

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

[commit-bot: I haz the power, 2012-04-11 18:54:51]

Presubmit check for 10019014-22001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
failed!
Command
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
.........E
======================================================================
ERROR: test_trace (__main__.Isolate)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 222, in test_trace
    out = self._execute('ok.isolate', ['--mode', 'trace'], True)
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 131, in _execute
    raise CalledProcessError(p.returncode, cmd, out, cwd)
CalledProcessError: Command '['/home/chrome-bot/python2.6/bin/python',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate.py',
'--variable', 'DEPTH=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate',
'--result', '/tmp/tmplwymwA/result',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/data/isolate/ok.isolate',
'--mode', 'trace']' returned non-zero exit status 2
cwd=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
Failure: 2
child: verify the test data files were mapped properly
['.svn', 'test_file2.txt', 'test_file1.txt'] != ['test_file1.txt',
'test_file2.txt']



----------------------------------------------------------------------
Ran 10 tests in 1.057s

FAILED (errors=1)


Presubmit checks took 7.7s to calculate.

[commit-bot: I haz the power, 2012-04-11 18:56:46]

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

[commit-bot: I haz the power, 2012-04-11 18:57:06]

Presubmit check for 10019014-15029 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
failed!
Command
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
.........E
======================================================================
ERROR: test_trace (__main__.Isolate)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 222, in test_trace
    out = self._execute('ok.isolate', ['--mode', 'trace'], True)
  File
"/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate_smoke_test.py",
line 131, in _execute
    raise CalledProcessError(p.returncode, cmd, out, cwd)
CalledProcessError: Command '['/home/chrome-bot/python2.6/bin/python',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/isolate.py',
'--variable', 'DEPTH=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate',
'--result', '/tmp/tmppSgwEn/result',
'/mnt/data/b/commit-queue/workdir/chromium/tools/isolate/data/isolate/ok.isolate',
'--mode', 'trace']' returned non-zero exit status 2
cwd=/mnt/data/b/commit-queue/workdir/chromium/tools/isolate
Failure: 2
child: verify the test data files were mapped properly
['test_file2.txt', 'test_file1.txt'] != ['test_file1.txt', 'test_file2.txt']



----------------------------------------------------------------------
Ran 10 tests in 1.023s

FAILED (errors=1)


Presubmit checks took 7.6s to calculate.

[commit-bot: I haz the power, 2012-04-11 18:59:52]

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

[commit-bot: I haz the power, 2012-04-11 19:29:36]

Try job failure for 10019014-20005 (retry) on linux_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-04-11 19:31:54]

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

[commit-bot: I haz the power, 2012-04-11 20:08:13]

Try job failure for 10019014-20007 (retry) on linux_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-04-11 20:08:31]

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

[commit-bot: I haz the power, 2012-04-11 20:47:29]

Try job failure for 10019014-11083 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-04-11 23:49:26]

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

[commit-bot: I haz the power, 2012-04-12 01:59:32]

Change committed as 131896

[M-A Ruel, 2012-04-12 12:43:58]

On 2012/04/12 01:59:32, I haz the power (commit-bot) wrote:
> Change committed as 131896

Sorry, I really seriously failed at properly testing this patch. :/ Anyway it's
done now. :)