Do not use MB for Clang Upload bots.

Do not use MB for Clang Upload bots.

Also, pass LLVM_FORCE_LOCAL_BUILD in the env. This is the reason
to stay away from MB for now, as it does not pass env vars to update.py.

BUG=578306, 582737

[krasin1, 2016-03-04 21:47:50]

krasin@chromium.org changed reviewers:
+ hans@chromium.org, krasin@chromium.org, thakis@chromium.org

[krasin1, 2016-03-04 21:47:50]

Please, review this build/ side of https://codereview.chromium.org/1757733004/

[hans, 2016-03-07 18:30:40]

Sorry for the slow reply.

lgtm

(Though I don't know exactly what MB is; could this be spelled out in the
commit, or is it a well known thing?)

[krasin1, 2016-03-07 18:32:33]

On 2016/03/07 18:30:40, hans wrote:
> Sorry for the slow reply.
> 
> lgtm
> 
> (Though I don't know exactly what MB is; could this be spelled out in the
> commit, or is it a well known thing?)

it's src/tools/mb:
https://chromium.googlesource.com/chromium/src/+/master/tools/mb/docs/user_gu...
"mb is a simple python wrapper around the GYP and GN meta-build tools to be used
as part of the GYP->GN migration."

[krasin1, 2016-03-07 18:32:52]

The CQ bit was checked by krasin@chromium.org

[commit-bot: I haz the power, 2016-03-07 18:33:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1766073002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1766073002/1

[commit-bot: I haz the power, 2016-03-07 18:33:06]

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

[commit-bot: I haz the power, 2016-03-07 18:33:07]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-infra-committers".
Note that this has nothing to do with OWNERS files.

[krasin1, 2016-03-07 18:35:37]

Nico,

can you please approve as well?

[Nico, 2016-03-08 21:19:16]

If you still want to do this after my comment on the other cl, then lgtm

[krasin1, 2016-03-09 00:40:01]

The CQ bit was checked by krasin@chromium.org

[commit-bot: I haz the power, 2016-03-09 00:40:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1766073002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1766073002/1

[commit-bot: I haz the power, 2016-03-09 00:40:08]

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

[commit-bot: I haz the power, 2016-03-09 00:40:08]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-infra-committers".
Note that this has nothing to do with OWNERS files.

[krasin1, 2016-03-09 00:52:04]

krasin@chromium.org changed reviewers:
+ dpranke@google.com

[krasin1, 2016-03-09 00:52:04]

Hi Dirk,

please, review this follow up to https://codereview.chromium.org/1688043002/

krasin

[Dirk Pranke, 2016-03-09 01:37:42]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2016-03-09 01:37:43]

I'm confused. I thought we were going to move to a flag in GN; why are we adding
more code that depends on environment variables instead?

[krasin1, 2016-03-09 01:39:51]

On 2016/03/09 01:37:43, Dirk Pranke wrote:
> I'm confused. I thought we were going to move to a flag in GN; why are we
adding
> more code that depends on environment variables instead?

I am now even more confused! :)
What do you mean by "going to move to a flag in GN"? Can you please give me a
link to a discussion or just explain the proposal inline?

[Dirk Pranke, 2016-03-09 02:04:29]

On 2016/03/09 01:39:51, krasin1 wrote:
> On 2016/03/09 01:37:43, Dirk Pranke wrote:
> > I'm confused. I thought we were going to move to a flag in GN; why are we
> adding
> > more code that depends on environment variables instead?
> 
> I am now even more confused! :)
> What do you mean by "going to move to a flag in GN"? Can you please give me a
> link to a discussion or just explain the proposal inline?

https://bugs.chromium.org/p/chromium/issues/detail?id=582737#c9

[krasin1, 2016-03-09 02:19:17]

On 2016/03/09 02:04:29, Dirk Pranke wrote:
> On 2016/03/09 01:39:51, krasin1 wrote:
> > On 2016/03/09 01:37:43, Dirk Pranke wrote:
> > > I'm confused. I thought we were going to move to a flag in GN; why are we
> > adding
> > > more code that depends on environment variables instead?
> > 
> > I am now even more confused! :)
> > What do you mean by "going to move to a flag in GN"? Can you please give me
a
> > link to a discussion or just explain the proposal inline?
> 
> https://bugs.chromium.org/p/chromium/issues/detail?id=582737#c9

Ah!
Well, that's a different (but similar) issue. In this case, we could eventually
not even need a flag or an env var, if we can get an upload bot not to run
gclient sync (and only run git pull). See https://crbug.com/593155

Let me know if you feel strongly. In this case, I would like to hear more
specific suggestions. For instance, a flag to gn won't work, as gclient sync
runs update.py directly, it's a part of DEPS:
https://code.google.com/p/chromium/codesearch#chromium/src/DEPS&q=DEPS&sq=pac...

[Dirk Pranke, 2016-03-09 02:40:59]

On 2016/03/09 02:19:17, krasin1 wrote:
> On 2016/03/09 02:04:29, Dirk Pranke wrote:
> > On 2016/03/09 01:39:51, krasin1 wrote:
> > > On 2016/03/09 01:37:43, Dirk Pranke wrote:
> > > > I'm confused. I thought we were going to move to a flag in GN; why are
we
> > > adding
> > > > more code that depends on environment variables instead?
> > > 
> > > I am now even more confused! :)
> > > What do you mean by "going to move to a flag in GN"? Can you please give
me
> a
> > > link to a discussion or just explain the proposal inline?
> > 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=582737#c9
> 
> Ah!
> Well, that's a different (but similar) issue. In this case, we could
eventually
> not even need a flag or an env var, if we can get an upload bot not to run
> gclient sync (and only run git pull). See https://crbug.com/593155

Got it, this is the upload bot, not the ToT bot.

That said, now I'm confused by the rest of this CL :).

You're removing the 'clang_tot' config; doesn't that mean that you'll pull the
DEPS'ed
in version of clang, rather than HEAD? 

Also, you say you don't want to run mb, but you're still calling
api.chromium.run_mb() 
on line 72 of the recipe. That seems wrong? 

If you don't want a full / normal chromium checkout, it is theoretically
possible to 
just clone the git repo rather than running bot_update but that's more work.

If you just want to skip the default set of hooks and only run a couple like the
update script directly, you can just not call api.chromium.runhooks() on line 70
instead.

[krasin1, 2016-03-09 03:12:07]

On 2016/03/09 02:40:59, Dirk Pranke wrote:
> On 2016/03/09 02:19:17, krasin1 wrote:
> > On 2016/03/09 02:04:29, Dirk Pranke wrote:
> > > On 2016/03/09 01:39:51, krasin1 wrote:
> > > > On 2016/03/09 01:37:43, Dirk Pranke wrote:
> > > > > I'm confused. I thought we were going to move to a flag in GN; why are
> we
> > > > adding
> > > > > more code that depends on environment variables instead?
> > > > 
> > > > I am now even more confused! :)
> > > > What do you mean by "going to move to a flag in GN"? Can you please give
> me
> > a
> > > > link to a discussion or just explain the proposal inline?
> > > 
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=582737#c9
> > 
> > Ah!
> > Well, that's a different (but similar) issue. In this case, we could
> eventually
> > not even need a flag or an env var, if we can get an upload bot not to run
> > gclient sync (and only run git pull). See https://crbug.com/593155
> 
> Got it, this is the upload bot, not the ToT bot.
> 
> That said, now I'm confused by the rest of this CL :).
> 
> You're removing the 'clang_tot' config; doesn't that mean that you'll pull the
> DEPS'ed
> in version of clang, rather than HEAD? 
I believe that the script does not remove 'clang_tot' config. Does it?

> 
> Also, you say you don't want to run mb, but you're still calling
> api.chromium.run_mb() 
> on line 72 of the recipe. That seems wrong? 
Yes, this is very wrong. Thank you for the catch!

> 
> If you don't want a full / normal chromium checkout, it is theoretically
> possible to 
> just clone the git repo rather than running bot_update but that's more work.
> 
> If you just want to skip the default set of hooks and only run a couple like
the
> update script directly, you can just not call api.chromium.runhooks() on line
70
> instead.
Thank you for the suggestion. It now seems obvious. Please take a look:
https://codereview.chromium.org/1776893002/

[Dirk Pranke, 2016-03-09 03:24:19]

On 2016/03/09 03:12:07, krasin1 wrote:
> On 2016/03/09 02:40:59, Dirk Pranke wrote:
> > You're removing the 'clang_tot' config; doesn't that mean that you'll pull
the
> > DEPS'ed
> > in version of clang, rather than HEAD? 
> I believe that the script does not remove 'clang_tot' config. Does it?

Sorry, I misremembered what the code was doing and wrote the wrong thing.

What I meant was, don't you want to *add* the 'clang_tot' config, so that you
actually
get ToT clang? And, if so, doesn't the 'clang_upload' config become redundant?

I'm assuming that this CL is now moot, and we should switch over to the other
one, so I'll re-post there as well.

[krasin1, 2016-03-09 03:26:32]

On 2016/03/09 03:24:19, Dirk Pranke wrote:
> On 2016/03/09 03:12:07, krasin1 wrote:
> > On 2016/03/09 02:40:59, Dirk Pranke wrote:
> > > You're removing the 'clang_tot' config; doesn't that mean that you'll pull
> the
> > > DEPS'ed
> > > in version of clang, rather than HEAD? 
> > I believe that the script does not remove 'clang_tot' config. Does it?
> 
> Sorry, I misremembered what the code was doing and wrote the wrong thing.
> 
> What I meant was, don't you want to *add* the 'clang_tot' config, so that you
> actually
> get ToT clang? And, if so, doesn't the 'clang_upload' config become redundant?
> 
> I'm assuming that this CL is now moot, and we should switch over to the other
> one, so I'll re-post there as well.

Yes, this CL is now just for a reference only. I will kill it, if the other one
gets approved and works.