android envsetup: Stop honoring --target-arch parameter.

tools/cr/cr/base/android.py

Index: tools/cr/cr/base/android.py
diff --git a/tools/cr/cr/base/android.py b/tools/cr/cr/base/android.py
index 45fdc51fe064d4d071361bebd14c2628d701de0c..752061358c0dfbb7c6eeccd7f9ef4f1b23546e88 100644
--- a/tools/cr/cr/base/android.py
+++ b/tools/cr/cr/base/android.py
@@ -59,8 +59,10 @@ class AndroidPlatform(cr.Platform):
       if not self._env_ready:
         # See what the env would be without env setup
         before = context.exported
-        # Run env setup and capture/parse it's output
-        envsetup = 'source {CR_ENVSETUP} --target-arch={CR_ENVSETUP_ARCH}'
+        before['GYP_DEFINES'] = before.get(
+            'GYP_DEFINES', '') + ' target_arch={CR_ENVSETUP_ARCH}'

[iancottrell, 2014/02/19 09:58:21]

This feels like the wrong place to do this.
Is there any reason this needs to be android specific?
It might be better to have arch directly modify GYP_DEFINES for all platforms.
GYP_DEFINES in general has some problems, mostly because ramming a map of
settings into a single string is a vile and horrible practice to start with :) 
I have considered adding more magic to help with it, for instance auto merging
all variables that start with a GYP_SET_ prefix or something.
This would allow arch to have GYP_SET_target_arch set on it, and it would just
work. It would also make it easy to override and view all the other gyp options
on a per output basis.
If it is going to stay in this file, it should probably move down to the
GYP_DEFINES specific modification part below this.

[Nico, 2014/02/19 14:04:06]

Yes: envsetup is android-specific, and I'm changing how envsetup works.
You can pass -D for every single define to gyp instead if you think that's
nicer.
Which part? The `if key == 'GYP_DEFINES'` bit? That only runs if envsetup
changes GYP_DEFINES, right? It'll stop doing that soon, so that line will be
dead soon.

(I have no opinion how the cr bit should work. Tell me what to do, and I'll
gladly do it.)

[iancottrell, 2014/02/19 14:18:24]

I mean that setting the arch for gyp does not need to be specific to android, it
would be cleaner for it not to be in this file at all.
I am happy for you to commit it here and file a bug against me to make it more
general though.
Except the code you are adding is faking envsetup modifying GYP_DEFINES :)
The reason it would be better down there is that up here it is modifying
context.exported which is not safe, and will also get dropped if the line below
is removed
Down there, it would get added to the new override added to context.

+        # Run env setup and capture/parse its output
+        envsetup = 'source {CR_ENVSETUP}'
         output = cr.Host.CaptureShell(context, envsetup + ' > /dev/null && env')
         env_setup = cr.Config('envsetup', literal=True, export=True)
         for line in output.split('\n'):