blink_rename_merge_helper: Implement run.py loader stub

blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will be
dynamically pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2802743003
Cr-Commit-Position: refs/heads/master@{#462276}
Committed: https://chromium.googlesource.com/chromium/src/+/2752771ce493bd0ee1bb35bf0ddc6fd4b9e3b802

[dcheng, 2017-04-05 19:23:52]

Description was changed from

==========
Stage 1.
==========

to

==========
blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will bedynamically
pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org
==========

[dcheng, 2017-04-05 19:23:52]

dcheng@chromium.org changed reviewers:
+ thakis@chromium.org

[dcheng, 2017-04-05 19:24:05]

Description was changed from

==========
blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will bedynamically
pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org
==========

to

==========
blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will be
dynamically pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org
==========

[dcheng, 2017-04-05 19:25:00]

Fairly simple, I hope. I got a bit blocked on other things this week, but I plan
on sending out the "we're doing the Blink rename this weekend" announcement once
this commit lands.

[Nico, 2017-04-05 20:23:18]

I don't understand the COMPONENTS bit. It sounds like it's there so that the
pylib can be versioned independent of src.git, but COMPONENTS is stored in
src.git, so how is rolling the revision in COMPONENTS easier than just updating
the script in-tree?

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/COMPONENTS (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/COMPONENTS:3: "test-win32":
"b34eaa708cf66b15725973c3d1c52d58e011cd36"
what is this file?

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/OWNERS (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/OWNERS:4: nasko@chromium.org
maybe one of these could review this?

[Nico, 2017-04-05 20:24:43]

Ah, I think I got it, in-tree-COMPONENTS isn't used.

Makes sense to me, but maybe one of the other OWNERS could review this in
detail?

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:103: 'cp',
'gs://chromium-blink-rename/COMPONENTS', hashes_path)
Aha!

[dcheng, 2017-04-05 20:28:28]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org

[dcheng, 2017-04-05 20:28:28]

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/COMPONENTS (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/COMPONENTS:3: "test-win32":
"b34eaa708cf66b15725973c3d1c52d58e011cd36"
On 2017/04/05 20:23:17, Nico wrote:
> what is this file?

This is just for testing purposes in this CL, to make sure the per-platform
stuff works.

(I didn't actually upload this to gs, since followup CLs will delete this)

If you feel strongly about not having this, I can remove.

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/OWNERS (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/OWNERS:4: nasko@chromium.org
On 2017/04/05 20:23:17, Nico wrote:
> maybe one of these could review this?

+danakj, it's your lucky day =)

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/20001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:103: 'cp',
'gs://chromium-blink-rename/COMPONENTS', hashes_path)
On 2017/04/05 20:24:43, Nico wrote:
> Aha!

Yeah, sorry, I guess it wasn't clear enough from the README. I'll update that.

[dcheng, 2017-04-05 20:37:47]

Added a bit more info to the README.md

[dcheng, 2017-04-05 20:42:35]

The CQ bit was checked by dcheng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 20:43:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 20:56:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 20:56:31]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-04-05 21:38:51]

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/README.md (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/README.md:7: exists for tracking purposes, as
the tool always fetches the latest COMPONENTS
what do u mean by tracking?

[dcheng, 2017-04-05 21:44:39]

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/README.md (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/README.md:7: exists for tracking purposes, as
the tool always fetches the latest COMPONENTS
On 2017/04/05 21:38:51, danakj wrote:
> what do u mean by tracking?

Hmm... I'm trying to say that it's used for things like:
- being able to review the diffs
- being able to go back to a known good version if something breaks
- etc

basically all the good things source control gives us. Not sure how to word this
sentence tbh

[danakj, 2017-04-05 21:54:11]

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:40: self.__gsutil = os.path.join(path,
'gsutil.py')
should we check this exists?

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:52: path =
_whereis('download_from_google_storage.py')
perhaps path2 = _whereis('gsutil.py')
and then throw if path != path2?

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:56: return
shouldnt this throw _DepotToolsNotFoundException?

[danakj, 2017-04-05 21:55:08]

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/README.md (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/README.md:7: exists for tracking purposes, as
the tool always fetches the latest COMPONENTS
On 2017/04/05 21:44:39, dcheng wrote:
> On 2017/04/05 21:38:51, danakj wrote:
> > what do u mean by tracking?
> 
> Hmm... I'm trying to say that it's used for things like:
> - being able to review the diffs
> - being able to go back to a known good version if something breaks
> - etc
> 
> basically all the good things source control gives us. Not sure how to word
this
> sentence tbh

As a mirror to what is in gs so that we can see and review what is there more
easily. Any changes to the gs should be checked in to that file also?

[dcheng, 2017-04-05 22:21:34]

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/README.md (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/README.md:7: exists for tracking purposes, as
the tool always fetches the latest COMPONENTS
On 2017/04/05 21:55:08, danakj wrote:
> On 2017/04/05 21:44:39, dcheng wrote:
> > On 2017/04/05 21:38:51, danakj wrote:
> > > what do u mean by tracking?
> > 
> > Hmm... I'm trying to say that it's used for things like:
> > - being able to review the diffs
> > - being able to go back to a known good version if something breaks
> > - etc
> > 
> > basically all the good things source control gives us. Not sure how to word
> this
> > sentence tbh
> 
> As a mirror to what is in gs so that we can see and review what is there more
> easily. Any changes to the gs should be checked in to that file also?

Done.

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:40: self.__gsutil = os.path.join(path,
'gsutil.py')
On 2017/04/05 21:54:10, danakj wrote:
> should we check this exists?

I implemented your other suggestion.

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:52: path =
_whereis('download_from_google_storage.py')
On 2017/04/05 21:54:11, danakj wrote:
> perhaps path2 = _whereis('gsutil.py')
> and then throw if path != path2?

Done.

https://codereview.chromium.org/2802743003/diff/40001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:56: return
On 2017/04/05 21:54:11, danakj wrote:
> shouldnt this throw _DepotToolsNotFoundException?

Done.

[danakj, 2017-04-05 22:38:59]

LGTM

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:33: A helper object for invoking depot
tool utils."""
Do you need to say what exceptions it can throw? #pythonrookie

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:61: if not path:
if not path2

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:128: return 1
does this need to print the exception's text or does that happen magically?

[dcheng, 2017-04-05 22:44:57]

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
File tools/blink_rename_merge_helper/run.py (right):

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:33: A helper object for invoking depot
tool utils."""
On 2017/04/05 22:38:59, danakj wrote:
> Do you need to say what exceptions it can throw? #pythonrookie

It should... I was being lazy. Fixed.

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:61: if not path:
On 2017/04/05 22:38:59, danakj wrote:
> if not path2

Done.

https://codereview.chromium.org/2802743003/diff/60001/tools/blink_rename_merg...
tools/blink_rename_merge_helper/run.py:128: return 1
On 2017/04/05 22:38:59, danakj wrote:
> does this need to print the exception's text or does that happen magically?

Doh, done.

(I wasn't using exceptions originally, then decided it was more 'pythonic')

[dcheng, 2017-04-05 23:06:07]

The CQ bit was checked by dcheng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 23:06:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 23:31:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 23:31:34]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-04-05 23:34:30]

rs-lgtm if you need owners approval

[dcheng, 2017-04-05 23:38:01]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2017-04-05 23:38:01]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2802743003/#ps80001
(title: "Meh")

[commit-bot: I haz the power, 2017-04-05 23:38:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 23:49:48]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491435481007840,
"parent_rev": "1f7a35358b0d15d040d8b10c76acecc6c6ff735b", "commit_rev":
"2752771ce493bd0ee1bb35bf0ddc6fd4b9e3b802"}

[commit-bot: I haz the power, 2017-04-05 23:50:38]

Description was changed from

==========
blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will be
dynamically pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org
==========

to

==========
blink_rename_merge_helper: Implement run.py loader stub

The entire tool will be committed in git; however, only the run.py
loader stub will run directly from the git repository. run.py only
exists to bootstrap the remainder of the toolchain, which will be
dynamically pulled from Google Storage as needed.

The reason for splitting up the tool this way is so that the tool
itself can be updated as needed after the Blink rename (e.g. by
marking the commit where the Blink rename was performed), while
still having these changes visible during a merge/rebase (when
the repository may be temporarily synced to an older commit).

BUG=676588
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2802743003
Cr-Commit-Position: refs/heads/master@{#462276}
Committed:
https://chromium.googlesource.com/chromium/src/+/2752771ce493bd0ee1bb35bf0ddc...
==========

[commit-bot: I haz the power, 2017-04-05 23:50:39]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/2752771ce493bd0ee1bb35bf0ddc...