[android] Adds constants.GetOutDirectory() and converts test scripts to use it.

[android] Adds constants.GetOutDirectory() and converts test scripts to use it.

BUG=260494
TEST=None
NOTRY=True

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

[frankf, 2013-08-21 01:50:06]

Can you update the forwarder in this patch

https://codereview.chromium.org/22903016/diff/1/build/android/pylib/constants.py
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/1/build/android/pylib/constants...
build/android/pylib/constants.py:99: os.path.join(cmd_helper.OutDirectory.get(),
GetBuildType()))
Remove cmd_helper.OutDirectory alltogether.

[craigdh, 2013-08-22 22:09:42]

Please take a careful look.

https://codereview.chromium.org/22903016/diff/1/build/android/pylib/constants.py
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/1/build/android/pylib/constants...
build/android/pylib/constants.py:99: os.path.join(cmd_helper.OutDirectory.get(),
GetBuildType()))
On 2013/08/21 01:50:07, frankf wrote:
> Remove cmd_helper.OutDirectory alltogether.

Done.

[frankf, 2013-08-27 00:44:56]

+bulach

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
build/android/pylib/constants.py:105: DIR_SOURCE_ROOT,
os.environ.get('CHROMIUM_OUT_DIR', 'out'),
who sets this?

[craigdh, 2013-08-27 16:57:16]

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
build/android/pylib/constants.py:105: DIR_SOURCE_ROOT,
os.environ.get('CHROMIUM_OUT_DIR', 'out'),
On 2013/08/27 00:44:57, frankf wrote:
> who sets this?

I'm not sure who overrides the out directory (took this from the existing code),
but if it's not set it gets defaulted to 'out' so it's not really an issue.

[bulach, 2013-09-02 09:11:18]

lgtm for the overall idea, thanks! :)

I have just a small concern regarding the name of the function, and an
explanation below:

https://codereview.chromium.org/22903016/diff/17001/build/android/adb_reverse...
File build/android/adb_reverse_forwarder.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/adb_reverse...
build/android/adb_reverse_forwarder.py:18: from pylib import android_commands,
forwarder, constants
nit: sort order

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
build/android/pylib/constants.py:97: def GetBuildDirectory(build_type=None):
hmm... "BuildDirectory" is a bit ambiguous, there's build/ from the buildbot
repository, then $CHROMIUM_SRC/build (where this script lives), and yet it
returns "out/BUILDTYPE" :)

how about keeping as GetOutDirectory() and something like this:
"""Returns the out directory where the output binaries are built."""

wdyt?

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
build/android/pylib/constants.py:105: DIR_SOURCE_ROOT,
os.environ.get('CHROMIUM_OUT_DIR', 'out'),
On 2013/08/27 16:57:17, craigdh wrote:
> On 2013/08/27 00:44:57, frankf wrote:
> > who sets this?
> 
> I'm not sure who overrides the out directory (took this from the existing
code),
> but if it's not set it gets defaulted to 'out' so it's not really an issue.

yeah, the idea behind this is that we'd eventually have envsetup.sh, (or maybe
some other wrapper script) setting this environment variable to something else,
say, "out_android", and then we'd be able to build both android and linux
side-by-side (not the same shell, just the same repository / checkout)...

we're not there yet, but iancottrel@ is working on some parts of it..

[craigdh, 2013-09-03 18:36:09]

https://codereview.chromium.org/22903016/diff/17001/build/android/adb_reverse...
File build/android/adb_reverse_forwarder.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/adb_reverse...
build/android/adb_reverse_forwarder.py:18: from pylib import android_commands,
forwarder, constants
On 2013/09/02 09:11:18, bulach wrote:
> nit: sort order

Done.

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/17001/build/android/pylib/const...
build/android/pylib/constants.py:97: def GetBuildDirectory(build_type=None):
On 2013/09/02 09:11:18, bulach wrote:
> hmm... "BuildDirectory" is a bit ambiguous, there's build/ from the buildbot
> repository, then $CHROMIUM_SRC/build (where this script lives), and yet it
> returns "out/BUILDTYPE" :)
> 
> how about keeping as GetOutDirectory() and something like this:
> """Returns the out directory where the output binaries are built."""
> 
> wdyt?

Done.

[frankf, 2013-09-03 18:42:00]

lgtm w/ nit

https://codereview.chromium.org/22903016/diff/36001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/36001/build/android/pylib/const...
build/android/pylib/constants.py:134: def GetOutDirectory(build_type=None):
So you can also do:

SetBuildType('Debug')
GetOutDirectory()

and get rid of the parameter.

[craigdh, 2013-09-03 22:48:18]

https://codereview.chromium.org/22903016/diff/36001/build/android/pylib/const...
File build/android/pylib/constants.py (right):

https://codereview.chromium.org/22903016/diff/36001/build/android/pylib/const...
build/android/pylib/constants.py:134: def GetOutDirectory(build_type=None):
On 2013/09/03 18:42:01, frankf wrote:
> So you can also do:
> 
> SetBuildType('Debug')
> GetOutDirectory()
> 
> and get rid of the parameter.

As discussed offline, this provides a way to generate the out directory without
changing the build type.

[craigdh, 2013-09-03 23:02:10]

Ran telemetry tests locally.

[commit-bot: I haz the power, 2013-09-03 23:03:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/craigdh@chromium.org/22903016/36001

[commit-bot: I haz the power, 2013-09-03 23:03:45]

Failed to apply patch for
tools/telemetry/telemetry/core/backends/adb_commands.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/telemetry/telemetry/core/backends/adb_commands.py
  Hunk #4 FAILED at 162.
  1 out of 4 hunks FAILED -- saving rejects to file
tools/telemetry/telemetry/core/backends/adb_commands.py.rej

Patch:       tools/telemetry/telemetry/core/backends/adb_commands.py
Index: tools/telemetry/telemetry/core/backends/adb_commands.py
diff --git a/tools/telemetry/telemetry/core/backends/adb_commands.py
b/tools/telemetry/telemetry/core/backends/adb_commands.py
index
be822c2d3ca773ffe682c800be005035e1d59dd6..e02a3faf241a5c14696110a5849975290bdd28b3
100644
--- a/tools/telemetry/telemetry/core/backends/adb_commands.py
+++ b/tools/telemetry/telemetry/core/backends/adb_commands.py
@@ -14,7 +14,7 @@ from telemetry.core import util
 util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'build', 'android')
 try:
   from pylib import android_commands  # pylint: disable=F0401
-  from pylib import cmd_helper  # pylint: disable=F0401
+  from pylib import constants  # pylint: disable=F0401
   from pylib import forwarder  # pylint: disable=F0401
   from pylib import ports  # pylint: disable=F0401
 except Exception:
@@ -41,10 +41,6 @@ def ResetTestServerPortAllocation():
   return ports.ResetTestServerPortAllocation()
 
 
-def GetOutDirectory():
-  return cmd_helper.OutDirectory.get()
-
-
 class AdbCommands(object):
   """A thin wrapper around ADB"""
 
@@ -151,10 +147,11 @@ def HasForwarder(buildtype=None):
   if not buildtype:
     return (HasForwarder(buildtype='Release') or
             HasForwarder(buildtype='Debug'))
-  return (os.path.exists(os.path.join(GetOutDirectory(), buildtype,
-                                      'device_forwarder')) and
-          os.path.exists(os.path.join(GetOutDirectory(), buildtype,
-                                      'host_forwarder')))
+  device_forwarder = os.path.join(
+      constants.GetOutDirectory(build_type=buildtype), 'device_forwarder')
+  host_forwarder = os.path.join(
+      constants.GetOutDirectory(build_type=buildtype), 'host_forwarder')
+  return os.path.exists(device_forwarder) and os.path.exists(host_forwarder)
 
 class Forwarder(object):
   def __init__(self, adb, *port_pairs):
@@ -165,13 +162,14 @@ class Forwarder(object):
                       for port_pair in port_pairs]
 
     self._port_pairs = new_port_pairs
+    if HasForwarder('Release'):
+      constants.SetBuildType('Release')
+    elif HasForwarder('Debug'):
+      constants.SetBuildType('Debug')
+    else:
+      raise Exception('Build forwarder2')
     forwarder.Forwarder.Map(new_port_pairs, self._adb)
 
-  @staticmethod
-  def _GetBuildType():
-    assert HasForwarder()
-    return 'Debug' if HasForwarder('Debug') else 'Release'
-
   @property
   def url(self):
     return 'http://localhost:%i' % self._host_port

[commit-bot: I haz the power, 2013-09-06 18:09:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/craigdh@chromium.org/22903016/50001

[commit-bot: I haz the power, 2013-09-06 18:11:05]

Change committed as 221736

[johnme, 2013-09-09 12:25:17]

On 2013/09/06 18:11:05, I haz the power (commit-bot) wrote:
> Change committed as 221736

Reverted by crrev.com/222008 because this is consistently breaking WebKit
Android (Nexus4). See crbug.com/260494 for details.

[craigdh, 2013-09-09 17:43:08]

+cc johnme: fixed provision_devices.py, relanding.

[bulach, 2013-09-09 19:02:22]

I think you need a new "codereview" to reland? :)

on the new one, please, upload patchset 1 exactly as it was, then patchset 2
with the changes to fix, so reviewers can diff just the change... and thanks for
fixing! :)

[craigdh, 2013-09-09 20:19:05]

On 2013/09/09 19:02:22, bulach wrote:
> I think you need a new "codereview" to reland? :)

A new codereview makes sense to me. I've had conflicting recommendations from
reviewers on this point in the past which suggested I just reuse the patch :)

See https://codereview.chromium.org/23494039/

> 
> on the new one, please, upload patchset 1 exactly as it was, then patchset 2
> with the changes to fix, so reviewers can diff just the change... and thanks
for
> fixing! :)