Roll DEPS for chromite.

Roll DEPS for chromite.

This is needed to pick up changes to deploy_chrome
(https://chromium-review.googlesource.com/457260).

The chromeos-base/chromeos-chrome ebuild runs deploy_chrome from the
Chrome checkout, so we need to roll deps to use the correct version.

BUG=701617
Review-Url: https://codereview.chromium.org/2797703003
Cr-Commit-Position: refs/heads/master@{#461772}
Committed: https://chromium.googlesource.com/chromium/src/+/f721f93701a353deb099f775cbe5131b2377aa3c

[teravest, 2017-04-04 15:59:23]

teravest@chromium.org changed reviewers:
+ derat@chromium.org

[Daniel Erat, 2017-04-04 16:38:06]

lgtm

oh, huh. it's a bit surprising to me that the chrome os build process runs the
version of deploy_chrome that's pulled in by chrome deps instead of the version
that's present in the chrome os checkout.

a question for steven: why is deploy_chrome part of the chrome os checkout at
this point? if both the ebuild and simple chrome are using the version from the
chrome tree, do we even run the copy in the chrome os tree?

i suspect that the answer is going to be that it's there because it shares a
bunch of code with the rest of chromite.

[teravest, 2017-04-04 16:46:15]

On 2017/04/04 16:38:06, Daniel Erat wrote:
> lgtm
> 
> oh, huh. it's a bit surprising to me that the chrome os build process runs the
> version of deploy_chrome that's pulled in by chrome deps instead of the
version
> that's present in the chrome os checkout.
> 
> a question for steven: why is deploy_chrome part of the chrome os checkout at
> this point? if both the ebuild and simple chrome are using the version from
the
> chrome tree, do we even run the copy in the chrome os tree?
> 
> i suspect that the answer is going to be that it's there because it shares a
> bunch of code with the rest of chromite.

Thankfully, there's a justification in the ebuild:
        # Use the deploy_chrome from the *Chrome* checkout.  The benefit of
        # doing this is if a new buildspec of Chrome requires a non-backwards
        # compatible change to deploy_chrome, we can commit the fix to
        # deploy_chrome without breaking existing Chrome OS release builds,
        # and then roll the DEPS for chromite in the Chrome checkout.
        #
        # Another benefit is each version of Chrome will have the right
        # corresponding version of deploy_chrome.

Given that, it seems like it's wrong for deploy_chrome to be part of the
chrome os checkout, and that version shouldn't ever be used?

[stevenjb, 2017-04-04 16:59:29]

You answered your own question Dan. In addition to the chromite
dependencies in deploy_chrome, other chromite scripts invoke it.

Extracting all of Simple Chrome into a separate repository would certainly
have my support, but I suspect it would be a rather large refactoring.

That said, this pattern is pretty much the same as all of chrome's third
party libraries. It's just confusing because one chrome os repo (the chrome
ebuild) is invoking a separate chrome os repo (chromite) through the chrome
repo, pinned via DEPS. i.e. this isn't really any different than how we
use, say, catapult, except that chromite also happens to be part of
chromeos.



On Tue, Apr 4, 2017 at 9:46 AM, <teravest@chromium.org> wrote:

> On 2017/04/04 16:38:06, Daniel Erat wrote:
> > lgtm
> >
> > oh, huh. it's a bit surprising to me that the chrome os build process
> runs the
> > version of deploy_chrome that's pulled in by chrome deps instead of the
> version
> > that's present in the chrome os checkout.
> >
> > a question for steven: why is deploy_chrome part of the chrome os
> checkout at
> > this point? if both the ebuild and simple chrome are using the version
> from
> the
> > chrome tree, do we even run the copy in the chrome os tree?
> >
> > i suspect that the answer is going to be that it's there because it
> shares a
> > bunch of code with the rest of chromite.
>
> Thankfully, there's a justification in the ebuild:
> # Use the deploy_chrome from the *Chrome* checkout. The benefit of
> # doing this is if a new buildspec of Chrome requires a non-backwards
> # compatible change to deploy_chrome, we can commit the fix to
> # deploy_chrome without breaking existing Chrome OS release builds,
> # and then roll the DEPS for chromite in the Chrome checkout.
> #
> # Another benefit is each version of Chrome will have the right
> # corresponding version of deploy_chrome.
>
> Given that, it seems like it's wrong for deploy_chrome to be part of the
> chrome os checkout, and that version shouldn't ever be used?
>
> https://codereview.chromium.org/2797703003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[teravest, 2017-04-04 17:04:48]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2017-04-04 17:05:13]

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-04 18:19:40]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491325488969960, "parent_rev":
"daebb5887b27f4a4f0a09b9920cb1db86b379459", "commit_rev":
"f721f93701a353deb099f775cbe5131b2377aa3c"}

[commit-bot: I haz the power, 2017-04-04 18:20:34]

Description was changed from

==========
Roll DEPS for chromite.

This is needed to pick up changes to deploy_chrome
(https://chromium-review.googlesource.com/457260).

The chromeos-base/chromeos-chrome ebuild runs deploy_chrome from the
Chrome checkout, so we need to roll deps to use the correct version.

BUG=701617
==========

to

==========
Roll DEPS for chromite.

This is needed to pick up changes to deploy_chrome
(https://chromium-review.googlesource.com/457260).

The chromeos-base/chromeos-chrome ebuild runs deploy_chrome from the
Chrome checkout, so we need to roll deps to use the correct version.

BUG=701617

Review-Url: https://codereview.chromium.org/2797703003
Cr-Commit-Position: refs/heads/master@{#461772}
Committed:
https://chromium.googlesource.com/chromium/src/+/f721f93701a353deb099f775cbe5...
==========

[commit-bot: I haz the power, 2017-04-04 18:20:34]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/f721f93701a353deb099f775cbe5...