Linux sandbox: add sandbox_linux_unittests.isolate

Linux sandbox: add sandbox_linux_unittests.isolate

This is a re-land of https://codereview.chromium.org/340523006/

BUG=384514
R=maruel@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278212

[jln (very slow on Chromium), 2014-06-17 22:02:36]

maruel: is it the right thing to simply get rid of this assert?

[M-A Ruel, 2014-06-18 14:06:35]

On 2014/06/17 22:02:36, jln wrote:
> maruel: is it the right thing to simply get rid of this assert?

It's the first time I see the .gypi not being in the same directory as the .gyp.
I understand it makes sense for the sandbox since it is completely separate code
bases on each OS but this indeed breaks assumptions. Normally, it would expect a
single .isolate in the same directory as the .gyp. So I see two solutions:
1) Consider this is a one off and rename the file and move it to
sandbox/sandbox_linux_unittests.isolate.
2) Going further, it could be named sandbox_unittests.isolate and have the rules
for all OSes. But that seems moot since the unit test does have the OS name in
its file name, which is very unusual in chrome-land.
3) For isolate_driver.py to work with this use case. The assert is there because
there are assumption below and just removing the assert doesn't magically fix
the code.

If I were you, I'd vote for 1).

[jln (very slow on Chromium), 2014-06-18 17:45:34]

On 2014/06/18 14:06:35, M-A Ruel wrote:
> On 2014/06/17 22:02:36, jln wrote:
> > maruel: is it the right thing to simply get rid of this assert?
> 
> It's the first time I see the .gypi not being in the same directory as the
.gyp.
> I understand it makes sense for the sandbox since it is completely separate
code
> bases on each OS but this indeed breaks assumptions. Normally, it would expect
a
> single .isolate in the same directory as the .gyp. So I see two solutions:
> 1) Consider this is a one off and rename the file and move it to
> sandbox/sandbox_linux_unittests.isolate.
> 2) Going further, it could be named sandbox_unittests.isolate and have the
rules
> for all OSes. But that seems moot since the unit test does have the OS name in
> its file name, which is very unusual in chrome-land.
> 3) For isolate_driver.py to work with this use case. The assert is there
because
> there are assumption below and just removing the assert doesn't magically fix
> the code.
> 
> If I were you, I'd vote for 1).

This is unfortunate. sandbox/linux and sandbox/ don't have the same owners and
it seems like a pain to maintain a separate per-file owners list that duplicates
the one in sandbox/linux.

When we named the unit test, I actually did ask the infra team if I should name
it the same as the Windows one and all I got was "ohh, please no".

Would it be difficult to fix the Python script to support this use case? I think
going for (1) could be a temporary solution to be able to land this today, but
longer term, it would be nice to support this use case.

[jln (very slow on Chromium), 2014-06-18 23:27:13]

maruel: PTAL!

I've done (1) for now. Please consider supporting .gypi files in other
directories, as it would be better to have the .isolate file in sandbox/linux
instead.

[M-A Ruel, 2014-06-19 00:00:14]

On 2014/06/18 23:27:13, jln wrote:
> maruel: PTAL!
> 
> I've done (1) for now. Please consider supporting .gypi files in other
> directories, as it would be better to have the .isolate file in sandbox/linux
> instead.

lgtm

[jln (very slow on Chromium), 2014-06-19 00:22:15]

The CQ bit was checked by jln@chromium.org

[commit-bot: I haz the power, 2014-06-19 00:25:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jln@chromium.org/341623003/40001

[jln (very slow on Chromium), 2014-06-19 01:04:41]

Committed patchset #3 manually as r278212 (presubmit successful).