[Android] Add a configurable environment for devil/.

build/android/devil/env/android_sdk_config.py

+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+
+from devil.env import env_config
+
+DEFAULT_BUILD_TOOLS_VERSION = '23.0.0'
+DEVIL_ANDROID_SDK_ROOT = 'DEVIL_ANDROID_SDK_ROOT'

[perezju, 2015/09/10 09:45:48]

nit: it's not clear from the name if this is the _value_ of the SDK_ROOT or, as
it's actually the case, the _key_ used to look up the value within environment
vars.

Also, we probably don't need 'DEVIL' in the name of the constant (it's fine in
the name of the key).

maybe: ANDROID_SDK_ROOT_KEY = 'DEVIL_ANDROID_SDK_ROOT'?

[jbudorick, 2015/09/12 02:31:04]

obsolete

+
+
+class AndroidSdkConfig(env_config.EnvConfig):
+
+  @property
+  def root_path(self):
+    return (self._configured.get('root_path')

[aiolos (Not reviewing), 2015/09/10 12:25:50]

You could use the dependency_manager instead. You would have your default paths
in a BaseConfig (initialized with a json file that just specified the
local_paths without any of the cloud_storage information). And depending on
whether the configured path locations were consistent across projects, you could
either have them proceed the default paths in the local_files list, or have a
(for example) telemetry-specific config that you only use as needed.

(It should already work for finding local paths, but you'd have to depend on
telemetry/catapult_base to use it currently. If that's an issue, I can probably
move it to catapult before Telemetry moves.)

It doesn't have any dependencies in it currently, but you can see the setup for
a global DependencyManager here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

And an example of what the json should look like in the doc string here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/ca...

[jbudorick, 2015/09/12 02:31:05]

Done, and the current dependency on telemetry/catapult_base is fine.

dependency_manager is cool (and is a marked improvement over what I had, imo),
but it seems to have a couple rough edges when it comes to supporting cases like
this:
 - it seems like it's a bit too coupled to binary_manager: I ran into the
platform_architecture assert that seemed like it was a requirement set by
binary_manager but enforced by dependency_manager
 - no support for dynamic configuration: BaseConfig is determined to load
configurations from an existing JSON file, so I had to write a json to a
tempfile after processing our command-line flags.

+            or os.environ.get(DEVIL_ANDROID_SDK_ROOT))

[mikecase (-- gone --), 2015/09/10 00:54:16]

I think this could be a little confusing since these configs are stored as
global variables in env/__init__.py and its unclear where and if they are set
(or even that you have to set them).

Maybe raise an exception if both its not configured and the environment variable
isn't set to anything? Could make it easier to debug, but it might make the code
overly confusing.

[perezju, 2015/09/10 09:45:48]

+1 to raising an exception. Either that or providing some other sensible default
other than |None|.

[jbudorick, 2015/09/12 02:31:05]

need to verify how the new dependency_manager implementation handles this case,
I'll come back to this.

+
+  @property
+  def adb_path(self):
+    return (self._configured.get('adb_path')
+            or os.path.join(self.root_path, 'platform-tools', 'adb'))
+
+  @property
+  def build_tools_path(self):
+    return os.path.join(
+        self.root_path, 'build-tools',
+        self._configured.get('build_tools_version',
+                             DEFAULT_BUILD_TOOLS_VERSION))
+