New issue
Advanced search Search tips

Issue 907585 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 894045



Sign in to add a comment

mb: migrate away from isolate to isolated

Project Member Reported by mar...@chromium.org, Nov 21

Issue description

Background:
'isolate' was designed when GN didn't exist. So it is a dialect of python, which includes condition evaluation. It optionally includes the command to run and the relative_cwd is deduced from its location when the compiled '.isolated' file is generated. We used to check in the '.isolate' files. They are now generated by 'mb.py'. We do not need the condition support in '.isolate' format, thus the 'isolated' tool is now sufficient.

End goal:
Get rid of 'isolate' which was designed GYP compatibility towards 'isolated' which is a pure file archiver. That removes a lot of cruft in the code base and makes things less "magical".

Challenge:
mb.py currently puts the task's command in the '.isolate' files it generates. The location of the generated '.isolate' file is used to specify 'relative_cwd' in the generated '.isolated' file.

End state:
Specify each test's command line and relative_cwd at task creation time as a 'raw_cmd' instead of using 'extra_args', inside the recipe. This require some work on the chromium recipes.

Note that Fuchsia is already in this mode. Fuchsia never used 'isolate' IIRC.

AI:
1. Add each test shard command as 'raw_cmd' to the Swarming task creation inside the recipe.
2. Add each test shard relative directory as 'relative_cwd' to the Swarming task creation inside the recipe.
3. Remove the command in the generated .isolate file.
4. Migrate from 'isolate' to 'isolated' tool.
5. Get rid of the isolate tool (both Go and python).
 
I'll point out that isolate contents are relative to the build dir and hence independent of the build dir name (but not of its depth; "out/" and "out/gn/" will produce different .isolate files), but .isolated files currently aren't.

If we do anything here, we should make it so that it's easy to generate deterministic, build-path-independent isolated files (see also issue 907488).
@maruel - are (1) and (2) currently supported from inside the swarming recipe modules?
Joshua is rewriting the Swarming recipe as part of issue 894048 so there's a some things in flux.

That said, the current swarming recipe module:
https://chromium.googlesource.com/chromium/tools/build/+/HEAD/scripts/slave/README.recipes.md#class-swarmingapi_recipeapi

task() already has raw_cmd but misses relative_cwd. It can easily be added since 'swarming.py trigger' already supports it:
https://cs.chromium.org/chromium/infra/luci/client/swarming.py?q=swarming.py&sq=package:chromium&g=0&l=998

It's a matter of passing the value down to the flag --relative-cwd to 'swarming.py trigger'.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 7

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c403c5617118c5938c95fe489d0f9b78c790f58a

commit c403c5617118c5938c95fe489d0f9b78c790f58a
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Fri Dec 07 08:52:53 2018

[swarming] add swarming, isolated under tools/luci-go

This is preparation to switch swarming client.

Bug: chromium:894045, chromium:907585
Change-Id: I3cd5eec0ed8a81eefbac0061f0ee0eb38f5bfd7c
Reviewed-on: https://chromium-review.googlesource.com/c/1365212
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58085}
[modify] https://crrev.com/c403c5617118c5938c95fe489d0f9b78c790f58a/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7

Labels: merge-merged-7.2
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e6c38d5c49812d461f993baab724de3022416198

commit e6c38d5c49812d461f993baab724de3022416198
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Dec 07 13:20:08 2018

Merged: [swarming] add swarming, isolated under tools/luci-go

Revision: c403c5617118c5938c95fe489d0f9b78c790f58a

BUG=chromium:894045,chromium:907585
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=sergiyb@chromium.org

Change-Id: I09baa3f0383e19061db7f54ac6e0aae5c18b2227
Reviewed-on: https://chromium-review.googlesource.com/c/1367446
Reviewed-by: Sergiy Belozorov <sergiyb@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/7.2@{#9}
Cr-Branched-From: 6acd03c9b8a8232aee95f25fbf6ae822aaedae75-refs/heads/7.2.502@{#1}
Cr-Branched-From: b03041de094610ef24e0e4fb6bf4c700fa1553ed-refs/heads/master@{#57910}
[modify] https://crrev.com/e6c38d5c49812d461f993baab724de3022416198/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7

Labels: merge-merged-7.1
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/7fb26320a70fa8f685323fd84fc99bf1c4504e7a

commit 7fb26320a70fa8f685323fd84fc99bf1c4504e7a
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Dec 07 13:21:11 2018

Merged: [swarming] add swarming, isolated under tools/luci-go

Revision: c403c5617118c5938c95fe489d0f9b78c790f58a

BUG=chromium:894045,chromium:907585
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=sergiyb@chromium.org

Change-Id: I36be994d8e345797ebf8c6bd2bdeaec5ffb55501
Reviewed-on: https://chromium-review.googlesource.com/c/1367447
Reviewed-by: Sergiy Belozorov <sergiyb@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/7.1@{#61}
Cr-Branched-From: f70aaa8ab2e8815505a6145c745e50d8328cd28c-refs/heads/7.1.302@{#1}
Cr-Branched-From: 1dbcc78efa17a9047f7e923958087ef9eec43066-refs/heads/master@{#56462}
[modify] https://crrev.com/7fb26320a70fa8f685323fd84fc99bf1c4504e7a/DEPS

Sign in to add a comment