android: run zip/extract/compile steps in the parent of src/, like on other platforms.

android: run zip/extract/compile steps in the parent of src/, like on other platforms.

BUG=294387
R=ilevy@chromium.org

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

[agable, 2013-10-21 17:57:21]

Reupload? looks like base files missing.

[Nico, 2013-10-21 18:02:53]

That doesn't work in 80% of uploads for me. Click "Raw", that always works, and
it's just 4 lines.

[Nico, 2013-10-21 18:13:33]

reuploaded

[Nico, 2013-10-21 18:47:37]

+yfriedman

ilevy isn't on irc, maybe he's out today

[Isaac (away), 2013-10-21 20:08:32]

Srry, under weather.

This looks OK but I wouldn't be surprised if it breaks something.  Various tools
expect (and use) src/ as base.

https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
File build/android/buildbot/bb_run_bot.py (right):

https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
build/android/buildbot/bb_run_bot.py:269: return_code = subprocess.call(command,
cwd=bb_utils.CHROME_SRC, env=env)
We use env here and the cwd is CHROME_SRC.

[Nico, 2013-10-21 20:12:10]

I wouldn't be surprised if this broke anything either :-) But I did look through
the steps called in this folder, and they look fine from what I can tell.
zip/extract_build are called on the other bots too, the license checker
qualifies all paths, etc.

https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
File build/android/buildbot/bb_run_bot.py (right):

https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
build/android/buildbot/bb_run_bot.py:269: return_code = subprocess.call(command,
cwd=bb_utils.CHROME_SRC, env=env)
On 2013/10/21 20:08:32, Isaac wrote:
> We use env here and the cwd is CHROME_SRC.

Since this sets an explicit cwd it'll clobber whatever PWD is in env anyhow.

[Isaac (away), 2013-10-21 20:20:55]

On 2013/10/21 20:12:10, Nico wrote:
> I wouldn't be surprised if this broke anything either :-) But I did look
through
> the steps called in this folder, and they look fine from what I can tell.
> zip/extract_build are called on the other bots too, the license checker
> qualifies all paths, etc.
> 
>
https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
> File build/android/buildbot/bb_run_bot.py (right):
> 
>
https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb...
> build/android/buildbot/bb_run_bot.py:269: return_code =
subprocess.call(command,
> cwd=bb_utils.CHROME_SRC, env=env)
> On 2013/10/21 20:08:32, Isaac wrote:
> > We use env here and the cwd is CHROME_SRC.
> 
> Since this sets an explicit cwd it'll clobber whatever PWD is in env anyhow.

Doesn't that mean PWD will always be clobbered?  See:
https://goto.google.com/zosmd

[Nico, 2013-10-21 20:25:04]

On Mon, Oct 21, 2013 at 1:20 PM, <ilevy@chromium.org> wrote:

> On 2013/10/21 20:12:10, Nico wrote:
>
>> I wouldn't be surprised if this broke anything either :-) But I did look
>>
> through
>
>> the steps called in this folder, and they look fine from what I can tell.
>> zip/extract_build are called on the other bots too, the license checker
>> qualifies all paths, etc.
>>
>
>
> https://codereview.chromium.**org/32323005/diff/70001/build/**
>
android/buildbot/bb_run_bot.py<https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb_run_bot.py>
>
>> File build/android/buildbot/bb_run_**bot.py (right):
>>
>
>
> https://codereview.chromium.**org/32323005/diff/70001/build/**
>
android/buildbot/bb_run_bot.**py#newcode269<https://codereview.chromium.org/32323005/diff/70001/build/android/buildbot/bb_run_bot.py#newcode269>
>
>> build/android/buildbot/bb_run_**bot.py:269: return_code =
>>
> subprocess.call(command,
>
>> cwd=bb_utils.CHROME_SRC, env=env)
>> On 2013/10/21 20:08:32, Isaac wrote:
>> > We use env here and the cwd is CHROME_SRC.
>>
>
>  Since this sets an explicit cwd it'll clobber whatever PWD is in env
>> anyhow.
>>
>
> Doesn't that mean PWD will always be clobbered?  See:
> https://goto.google.com/zosmd


Oh, good point. bb_utils.py's SpawnCmd() always passes CHROME_SRC as cwd,
boo :-/ It does get rid of the PWD +/- line in env diff on the bots though,
and it's a prerequisite for changing the other places. This means this CL
here won't have an impact in practice yet.


>
>
>
https://codereview.chromium.**org/32323005/<https://codereview.chromium.org/3...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2013-10-21 20:40:38]

Slightly changed this patch, added this to the CL description:

"""Also change the default cwd for SpawnCmd() to None. The two commands
run through SpawnCmd shouldn't be affected by this.

The majority of commands runs through RunCmd(), whose cwd still defaults
to CHROME_SRC, so this change won't have much of an effect in pratice yet.
I'll change this in a followup."""

[Nico, 2013-10-21 21:16:38]

…and https://codereview.chromium.org/33303004/ for the more interesting part of
this.

[Yaron, 2013-10-21 22:06:40]

Removing myself as reviewer since ilevy is around and much better suited to
review

[Nico, 2013-10-21 22:36:29]

Changed this to be less invasive, as discussed.

[Isaac (away), 2013-10-21 22:38:10]

LGTM -- but watch closely after landing.

It's OK to dcommit this after android trybots (incl testers) pass the relevant
steps.

[commit-bot: I haz the power, 2013-10-21 23:10:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/32323005/240001

[Nico, 2013-10-22 01:04:26]

Committed patchset #4 manually as r229993 (presubmit successful).

[Nico, 2013-10-22 02:40:59]

https://codereview.chromium.org/32323005/diff/240001/build/android/buildbot/b...
File build/android/buildbot/bb_host_steps.py (right):

https://codereview.chromium.org/32323005/diff/240001/build/android/buildbot/b...
build/android/buildbot/bb_host_steps.py:61: RunCmd(cmd + ['--build-args=%s' %
build_target], halt_on_failure=True)
Oh hmm, I guess I should do the same here?