Make gyp target_arch setting native to chromium/config.py

Make gyp target_arch setting native to chromium/config.py

R=navabi@chromium.org, stip@chromium.org
BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=236268

[iannucci, 2013-11-08 00:06:21]

PTAL

[navabi, 2013-11-08 19:00:57]

On 2013/11/08 00:06:21, iannucci wrote:
> PTAL

Looks good for the most part. Just a couple questions.

[navabi, 2013-11-08 19:01:13]

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium/config.py (left):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium/config.py:122:
c.gyp_env.GYP_DEFINES['target_arch'] = 'x64'
Was this needed for c.HOST_PLATFORM == win and c.TARGET_BITS == 64? If so it
looks like it is not set in the gyp_arch block below, and if not, what was it
doing here in the first place?

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/gclient/config.py (right):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/gclient/config.py:246: # TODO(iannucci): Youch!
This path is a bit crazy.
What is this about? 

Can you also mention what this is doing in the description of the CL, since it
seems it does not have to do with moving target arch into the chromium config.

[iannucci, 2013-11-08 19:49:56]

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium/config.py (left):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium/config.py:122:
c.gyp_env.GYP_DEFINES['target_arch'] = 'x64'
On 2013/11/08 19:01:15, navabi wrote:
> Was this needed for c.HOST_PLATFORM == win and c.TARGET_BITS == 64? If so it
> looks like it is not set in the gyp_arch block below, and if not, what was it
> doing here in the first place?

It was just a temporary holdover until we did the proper implementation, since
we needed proper support for 64bit windows builds.

Most of the time build/common.gypi 'automatically' detects the proper value for
target_arch. The target_arch actually has nothing to do with HOST_PLATFORM, it
only has to do with TARGET_ARCH and TARGET_BITS.

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/gclient/config.py (right):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/gclient/config.py:246: # TODO(iannucci): Youch!
This path is a bit crazy.
This is an accident. I didn't mean to include it in this cl. Good catch.

[ghost stip (do not use), 2013-11-08 20:11:17]

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium_android/chromium_config.py (left):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:52:
@CONFIG_CTX(includes=['x86_builder', 'try_builder'])
won't this mean x86_try_builder won't get apply_issue?

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium_android/chromium_config.py (right):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:28: raise
recipe_config.BadConf('Android cannot targit %d bits' % c.TARGET_BITS)
target

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:32: def
main_builder(c):
maybe rename this to arm_builder?

[iannucci, 2013-11-12 02:16:34]

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium_android/chromium_config.py (left):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:52:
@CONFIG_CTX(includes=['x86_builder', 'try_builder'])
On 2013/11/08 20:11:18, stip wrote:
> won't this mean x86_try_builder won't get apply_issue?

no, the recipes check to see if 'try' is in the builder name (which it still
is). You can also tell from the expectations that the apply_issue still runs.

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/chromium_android/chromium_config.py (right):

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:28: raise
recipe_config.BadConf('Android cannot targit %d bits' % c.TARGET_BITS)
On 2013/11/08 20:11:18, stip wrote:
> target

Lol. Done.

https://chromiumcodereview.appspot.com/59233015/diff/1/scripts/slave/recipe_m...
scripts/slave/recipe_modules/chromium_android/chromium_config.py:32: def
main_builder(c):
On 2013/11/08 20:11:18, stip wrote:
> maybe rename this to arm_builder?

I'd prefer to leave that to another CL. main_builder is a
chromium-on-androidism.

But I agree, it's not a good name (esp. since we're not namespacing configs
yet).

[iannucci, 2013-11-13 06:11:23]

ping

[navabi, 2013-11-13 17:25:03]

On 2013/11/13 06:11:23, iannucci wrote:
> ping

lgtm.

[navabi1, 2013-11-13 17:25:50]

On 2013/11/13 17:25:03, navabi wrote:
> On 2013/11/13 06:11:23, iannucci wrote:
> > ping
> 
> lgtm.

Also lgtm from Chromium Armand.

[iannucci, 2013-11-13 20:07:24]

On 2013/11/13 17:25:50, navabi1 wrote:
> On 2013/11/13 17:25:03, navabi wrote:
> > On 2013/11/13 06:11:23, iannucci wrote:
> > > ping
> > 
> > lgtm.
> 
> Also lgtm from Chromium Armand.

stip canhaz review?

[ghost stip (do not use), 2013-11-13 20:18:32]

lgtm

[iannucci, 2013-11-14 02:34:12]

+dpranke, kbr, machenbach for expectations lgtm

Basically we will always explicitly set target_arch now.

[Michael Achenbach, 2013-11-14 12:23:48]

lgtm

[iannucci, 2013-11-14 19:30:37]

kbr/dpranke, could I get an ACK? :)

[Ken Russell (switch to Gerrit), 2013-11-14 19:34:17]

Sorry, but the changes to the Linux configuration look wrong.

https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
File scripts/slave/recipes/gpu/build_and_test.expected/linux_debug.json (right):

https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
scripts/slave/recipes/gpu/build_and_test.expected/linux_debug.json:81:
"GYP_DEFINES": "component=shared_library internal_gles2_conform_tests=1
target_arch=ia32",
This does not look good. It looks like the Linux builds are switching from a
default of 64 bits to a default of 32 bits. Is that correct? I don't think this
change is desirable on any of the Linux bots.

https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
File scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json (right):

https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json:81:
"GYP_DEFINES": "clang=1 component=shared_library internal_gles2_conform_tests=1
target_arch=ia32",
What happens when the Mac build switches to 64-bit, which is intended to happen
imminently?

[iannucci, 2013-11-14 22:12:03]

On 2013/11/14 19:34:17, Ken Russell wrote:
> Sorry, but the changes to the Linux configuration look wrong.
> 
>
https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
> File scripts/slave/recipes/gpu/build_and_test.expected/linux_debug.json
(right):
> 
>
https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
> scripts/slave/recipes/gpu/build_and_test.expected/linux_debug.json:81:
> "GYP_DEFINES": "component=shared_library internal_gles2_conform_tests=1
> target_arch=ia32",
> This does not look good. It looks like the Linux builds are switching from a
> default of 64 bits to a default of 32 bits. Is that correct? I don't think
this
> change is desirable on any of the Linux bots.
> 
>
https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
> File scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json (right):
> 
>
https://chromiumcodereview.appspot.com/59233015/diff/180001/scripts/slave/rec...
> scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json:81:
> "GYP_DEFINES": "clang=1 component=shared_library
internal_gles2_conform_tests=1
> target_arch=ia32",
> What happens when the Mac build switches to 64-bit, which is intended to
happen
> imminently?

Ah, good point on both. This is an artifact of the fact that we default the
TARGET_BITS to 32, regardless of host platform. I've now changed it so that
mac/linux default to 'whatever the host is', and windows still defaults to 32.

because windows.

[iannucci, 2013-11-14 22:16:41]

Fix mac/linux bitness. Defaults to 64bit in tests

[Ken Russell (switch to Gerrit), 2013-11-14 23:14:37]

Sorry for holding you up, but still one question.

Not clear to me whether this affects only the tests, but I think the tests
should reflect the actual configurations of the bots.

https://chromiumcodereview.appspot.com/59233015/diff/540001/scripts/slave/rec...
File scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json (right):

https://chromiumcodereview.appspot.com/59233015/diff/540001/scripts/slave/rec...
scripts/slave/recipes/gpu/build_and_test.expected/mac_debug.json:81:
"GYP_DEFINES": "clang=1 component=shared_library internal_gles2_conform_tests=1
target_arch=x64",
Is this correct at this point? I thought the 64-bit Mac transition was in
progress, but am not sure the switch has been flipped in the source tree.

[iannucci, 2013-11-15 00:19:22]

ok, more fixed now.

[Ken Russell (switch to Gerrit), 2013-11-15 00:20:41]

gpu/ changes LGTM.

[Michael Achenbach, 2013-11-20 16:33:33]

Still lgtm.

[commit-bot: I haz the power, 2013-11-20 19:17:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/59233015/640002

[commit-bot: I haz the power, 2013-11-20 19:18:16]

Failed to apply patch for
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
  Hunk #1 FAILED at 26.
  Hunk #2 FAILED at 41.
  2 out of 7 hunks FAILED -- saving rejects to file
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json.rej

Patch:      
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
Index:
scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
diff --git
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
index
011cc94f5b69248cc3c7f859b281bbe739519138..dfd34701eab0012e6484b92a5a82941b877ec684
100644
---
a/build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
+++
b/build/scripts/slave/recipe_modules/chromium/config.expected/Debug-linux.intel.32-to-linux.arm.32.json
@@ -26,7 +26,7 @@
       ]
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "component=shared_library use_ash=0 use_aura=0", 
+      "GYP_DEFINES": "component=shared_library target_arch=arm use_ash=0
use_aura=0", 
       "GYP_GENERATORS": "ninja"
     }
   }, 
@@ -41,7 +41,7 @@
       ]
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "clang=1 component=shared_library use_ash=0 use_aura=0", 
+      "GYP_DEFINES": "clang=1 component=shared_library target_arch=arm
use_ash=0 use_aura=0", 
       "GYP_GENERATORS": "ninja"
     }
   }, 
@@ -57,7 +57,7 @@
       ]
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "component=shared_library", 
+      "GYP_DEFINES": "component=shared_library target_arch=arm", 
       "GYP_GENERATORS": "ninja"
     }
   }, 
@@ -73,7 +73,7 @@
       ]
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "clang=1 component=shared_library", 
+      "GYP_DEFINES": "clang=1 component=shared_library target_arch=arm", 
       "GYP_GENERATORS": "ninja"
     }
   }, 
@@ -127,24 +127,7 @@
       "GYP_GENERATOR_FLAGS": "default_target=All"
     }
   }, 
-  "dartium_builder": {
-    "build_config_fs": "Debug", 
-    "build_dir": "Path('[CHECKOUT]', 'out')", 
-    "compile_py": {
-      "build_tool": "ninja", 
-      "compiler": "goma", 
-      "default_targets": [
-        "chrome_apk", 
-        "content_shell_apk"
-      ]
-    }, 
-    "gyp_env": {
-      "GYP_CROSSCOMPILE": "1", 
-      "GYP_DEFINES": "OS=android component=static_library fastbuild=1
gcc_version=46 host_os=linux order_text_section=Path('[CHECKOUT]', 'orderfiles',
'orderfile.out') target_arch=ia32", 
-      "GYP_GENERATORS": "ninja", 
-      "GYP_GENERATOR_FLAGS": "default_target=All"
-    }
-  }, 
+  "dartium_builder": "config \"dartium_builder\" includes \"x86_builder\", but
[Cannot target x86 with TARGET_ARCH == arm]", 
   "enormous_tests": {
     "build_config_fs": "Debug", 
     "build_dir": "Path('[CHECKOUT]', 'out')", 
@@ -219,7 +202,7 @@
       "build_tool": "ninja"
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "component=shared_library", 
+      "GYP_DEFINES": "component=shared_library target_arch=arm", 
       "GYP_GENERATORS": "ninja"
     }
   }, 
@@ -282,58 +265,11 @@
       ]
     }, 
     "gyp_env": {
-      "GYP_DEFINES": "v8_target_arch=arm"
-    }
-  }, 
-  "x86_builder": {
-    "build_config_fs": "Debug", 
-    "build_dir": "Path('[CHECKOUT]', 'out')", 
-    "compile_py": {
-      "build_tool": "ninja", 
-      "compiler": "goma", 
-      "default_targets": [
-        "All"
-      ]
-    }, 
-    "gyp_env": {
-      "GYP_CROSSCOMPILE": "1", 
-      "GYP_DEFINES": "OS=android component=static_library fastbuild=1
gcc_version=46 host_os=linux order_text_section=Path('[CHECKOUT]', 'orderfiles',
'orderfile.out') target_arch=ia32", 
-      "GYP_GENERATORS": "ninja", 
-      "GYP_GENERATOR_FLAGS": "default_target=All"
-    }
-  }, 
-  "x86_try_builder": {
-    "build_config_fs": "Debug", 
-    "build_dir": "Path('[CHECKOUT]', 'out')", 
-    "compile_py": {
-      "build_tool": "ninja", 
-      "compiler": "goma", 
-      "default_targets": [
-        "All"
-      ]
-    }, 
-    "gyp_env": {
-      "GYP_CROSSCOMPILE": "1", 
-      "GYP_DEFINES": "OS=android component=static_library fastbuild=1
gcc_version=46 host_os=linux order_text_section=Path('[CHECKOUT]', 'orderfiles',
'orderfile.out') target_arch=ia32", 
-      "GYP_GENERATORS": "ninja", 
-      "GYP_GENERATOR_FLAGS": "default_target=All"
-    }
-  }, 
-  "x86_try_instrumentation_tests": {
-    "build_config_fs": "Debug", 
-    "build_dir": "Path('[CHECKOUT]', 'out')", 
-    "compile_py": {
-      "build_tool": "ninja", 
-      "default_targets": [
-        "All"
-      ]
-    }, 
-    "gyp_env": {
-      "GYP_CROSSCOMPILE": "1", 
-      "GYP_DEFINES": "OS=android component=static_library fastbuild=1
gcc_version=46 host_os=linux order_text_section=Path('[CHECKOUT]', 'orderfiles',
'orderfile.out') target_arch=arm", 
-      "GYP_GENERATORS": "ninja", 
-      "GYP_GENERATOR_FLAGS": "default_target=All"
+      "GYP_DEFINES": "target_arch=arm v8_target_arch=arm"
     }
   }, 
+  "x86_builder": "Cannot target x86 with TARGET_ARCH == arm", 
+  "x86_try_builder": "config \"x86_try_builder\" includes \"x86_builder\", but
[Cannot target x86 with TARGET_ARCH == arm]", 
+  "x86_try_instrumentation_tests": "config \"x86_try_instrumentation_tests\"
includes \"x86_builder\", but [Cannot target x86 with TARGET_ARCH == arm]", 
   "xcodebuild": "can not use xcodebuild on \"linux\""
 }
\ No newline at end of file

[commit-bot: I haz the power, 2013-11-20 20:03:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/59233015/1020001

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

Failed to apply patch for
build/scripts/slave/recipes/ozone_ecs.expected/basic.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file build/scripts/slave/recipes/ozone_ecs.expected/basic.json
  Hunk #1 FAILED at 76.
  1 out of 1 hunk FAILED -- saving rejects to file
build/scripts/slave/recipes/ozone_ecs.expected/basic.json.rej

Patch:       build/scripts/slave/recipes/ozone_ecs.expected/basic.json
Index: scripts/slave/recipes/ozone_ecs.expected/basic.json
diff --git build/scripts/slave/recipes/ozone_ecs.expected/basic.json
build/scripts/slave/recipes/ozone_ecs.expected/basic.json
index
06a9a6d08dbf58b4f4a46fd5ff4c538000481ffa..46b61c8e4867aedcbaf1fbbdabb4848d9e946ff2
100644
--- a/build/scripts/slave/recipes/ozone_ecs.expected/basic.json
+++ b/build/scripts/slave/recipes/ozone_ecs.expected/basic.json
@@ -76,7 +76,7 @@
       "runhooks"
     ],
     "env": {
-      "GYP_DEFINES": "chromeos=0 component=static_library toolkit_views=0
use_ash=0 use_aura=1 use_ozone=1 use_pango=0",
+      "GYP_DEFINES": "chromeos=0 component=static_library target_arch=x64
toolkit_views=0 use_ash=0 use_aura=1 use_ozone=1 use_pango=0",
       "GYP_GENERATORS": "ninja"
     },
     "name": "gclient runhooks"

[commit-bot: I haz the power, 2013-11-20 20:10:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iannucci@chromium.org/59233015/1560004

[commit-bot: I haz the power, 2013-11-20 20:13:25]

Change committed as 236268

[Nico, 2014-02-19 02:53:29]

https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium/config.py (right):

https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32): 'mips',
This should be "mipsel", not "mips", right?

https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64): 'mips',
(and this shouldn't do anything yet as it's currently not used – but "mips" is
fairly likely not what we want here once it's going to be used.)

[iannucci, 2014-02-19 03:44:19]

On 2014/02/19 02:53:29, Nico wrote:
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/chromium/config.py (right):
> 
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32): 'mips',
> This should be "mipsel", not "mips", right?
> 
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64): 'mips',
> (and this shouldn't do anything yet as it's currently not used – but "mips" is
> fairly likely not what we want here once it's going to be used.)

yeah probably. if/when we support it for real, this will change. It's "what if"
code right now.

[iannucci, 2014-02-19 03:45:19]

On 2014/02/19 03:44:19, iannucci wrote:
> On 2014/02/19 02:53:29, Nico wrote:
> >
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> > File scripts/slave/recipe_modules/chromium/config.py (right):
> > 
> >
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> > scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32): 'mips',
> > This should be "mipsel", not "mips", right?
> > 
> >
>
https://codereview.chromium.org/59233015/diff/1560004/scripts/slave/recipe_mo...
> > scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64): 'mips',
> > (and this shouldn't do anything yet as it's currently not used – but "mips"
is
> > fairly likely not what we want here once it's going to be used.)
> 
> yeah probably. if/when we support it for real, this will change. It's "what
if"
> code right now.

((and btw, all the recipe_modules/*/config.expected/*.json are gone now))

[Michael Achenbach, 2014-02-19 08:44:32]

V8 changes lgtm

[Nico, 2014-02-19 22:08:45]

On Tue, Feb 18, 2014 at 7:44 PM, <iannucci@chromium.org> wrote:

> On 2014/02/19 02:53:29, Nico wrote:
>
> https://codereview.chromium.org/59233015/diff/1560004/
> scripts/slave/recipe_modules/chromium/config.py
>
>> File scripts/slave/recipe_modules/chromium/config.py (right):
>>
>
>
> https://codereview.chromium.org/59233015/diff/1560004/
> scripts/slave/recipe_modules/chromium/config.py#newcode130
>
>> scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32):
>> 'mips',
>> This should be "mipsel", not "mips", right?
>>
>
As I learned this morning, this recipe is actually used for
http://chromegw/i/clank/builders/mips-builder/builds/232/steps/gclient%20runh...,
and this saying "mips" instead of "mipsel" is why this bot is red. So
this seems a bit more serious than "what if" code :-)


>
>
> https://codereview.chromium.org/59233015/diff/1560004/
> scripts/slave/recipe_modules/chromium/config.py#newcode131
>
>> scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64):
>> 'mips',
>> (and this shouldn't do anything yet as it's currently not used - but
>> "mips" is
>> fairly likely not what we want here once it's going to be used.)
>>
>
> yeah probably. if/when we support it for real, this will change. It's
> "what if"
> code right now.
>
> https://codereview.chromium.org/59233015/
>

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

[iannucci, 2014-02-19 22:13:23]

Neat :(

We should probably fix that then. Looks like V8 uses it too:

->  slave git:(fix_pre) ack --type=json mips
recipes/android/android_builder.expected/mips_builder.json
83:      "--target-arch=mips"
141:      "GYP_DEFINES": "OS=android app_manifest_version_code=10
app_manifest_version_name=some_builder_1234 chrome_build_id=3333
component=static_library fastbuild=1 host_os=linux target_arch=mips",

recipes/v8.expected/mips_target.json
50:      "GYP_DEFINES": "target_arch=mips v8_target_arch=mips"

I'm guessing it's wrong there as well?


On Wed, Feb 19, 2014 at 2:08 PM, Nico Weber <thakis@chromium.org> wrote:

> On Tue, Feb 18, 2014 at 7:44 PM, <iannucci@chromium.org> wrote:
>
>> On 2014/02/19 02:53:29, Nico wrote:
>>
>> https://codereview.chromium.org/59233015/diff/1560004/
>> scripts/slave/recipe_modules/chromium/config.py
>>
>>> File scripts/slave/recipe_modules/chromium/config.py (right):
>>>
>>
>>
>> https://codereview.chromium.org/59233015/diff/1560004/
>> scripts/slave/recipe_modules/chromium/config.py#newcode130
>>
>>> scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32):
>>> 'mips',
>>> This should be "mipsel", not "mips", right?
>>>
>>
> As I learned this morning, this recipe is actually used for
>
http://chromegw/i/clank/builders/mips-builder/builds/232/steps/gclient%20runh...,
and this saying "mips" instead of "mipsel" is why this bot is red. So
> this seems a bit more serious than "what if" code :-)
>
>
>>
>>
>> https://codereview.chromium.org/59233015/diff/1560004/
>> scripts/slave/recipe_modules/chromium/config.py#newcode131
>>
>>> scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64):
>>> 'mips',
>>> (and this shouldn't do anything yet as it's currently not used - but
>>> "mips" is
>>> fairly likely not what we want here once it's going to be used.)
>>>
>>
>> yeah probably. if/when we support it for real, this will change. It's
>> "what if"
>> code right now.
>>
>> https://codereview.chromium.org/59233015/
>>
>
>

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

[Nico, 2014-02-19 22:14:19]

Looks like it:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/toolchain...


On Wed, Feb 19, 2014 at 2:13 PM, Robert Iannucci <iannucci@chromium.org>wrote:

> Neat :(
>
> We should probably fix that then. Looks like V8 uses it too:
>
> ->  slave git:(fix_pre) ack --type=json mips
> recipes/android/android_builder.expected/mips_builder.json
> 83:      "--target-arch=mips"
> 141:      "GYP_DEFINES": "OS=android app_manifest_version_code=10
> app_manifest_version_name=some_builder_1234 chrome_build_id=3333
> component=static_library fastbuild=1 host_os=linux target_arch=mips",
>
> recipes/v8.expected/mips_target.json
> 50:      "GYP_DEFINES": "target_arch=mips v8_target_arch=mips"
>
> I'm guessing it's wrong there as well?
>
>
> On Wed, Feb 19, 2014 at 2:08 PM, Nico Weber <thakis@chromium.org> wrote:
>
>> On Tue, Feb 18, 2014 at 7:44 PM, <iannucci@chromium.org> wrote:
>>
>>> On 2014/02/19 02:53:29, Nico wrote:
>>>
>>> https://codereview.chromium.org/59233015/diff/1560004/
>>> scripts/slave/recipe_modules/chromium/config.py
>>>
>>>> File scripts/slave/recipe_modules/chromium/config.py (right):
>>>>
>>>
>>>
>>> https://codereview.chromium.org/59233015/diff/1560004/
>>> scripts/slave/recipe_modules/chromium/config.py#newcode130
>>>
>>>> scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32):
>>>> 'mips',
>>>> This should be "mipsel", not "mips", right?
>>>>
>>>
>> As I learned this morning, this recipe is actually used for
>>
http://chromegw/i/clank/builders/mips-builder/builds/232/steps/gclient%20runh...,
and this saying "mips" instead of "mipsel" is why this bot is red. So
>> this seems a bit more serious than "what if" code :-)
>>
>>
>>>
>>>
>>> https://codereview.chromium.org/59233015/diff/1560004/
>>> scripts/slave/recipe_modules/chromium/config.py#newcode131
>>>
>>>> scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64):
>>>> 'mips',
>>>> (and this shouldn't do anything yet as it's currently not used - but
>>>> "mips" is
>>>> fairly likely not what we want here once it's going to be used.)
>>>>
>>>
>>> yeah probably. if/when we support it for real, this will change. It's
>>> "what if"
>>> code right now.
>>>
>>> https://codereview.chromium.org/59233015/
>>>
>>
>>
>

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

[Michael Achenbach, 2014-02-20 08:09:29]

On 2014/02/19 22:14:19, Nico wrote:
> Looks like it:
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/build/toolchain...
> 
> 
> On Wed, Feb 19, 2014 at 2:13 PM, Robert Iannucci <iannucci@chromium.org>wrote:
> 
> > Neat :(
> >
> > We should probably fix that then. Looks like V8 uses it too:
> >
> > ->  slave git:(fix_pre) ack --type=json mips
> > recipes/android/android_builder.expected/mips_builder.json
> > 83:      "--target-arch=mips"
> > 141:      "GYP_DEFINES": "OS=android app_manifest_version_code=10
> > app_manifest_version_name=some_builder_1234 chrome_build_id=3333
> > component=static_library fastbuild=1 host_os=linux target_arch=mips",
> >
> > recipes/v8.expected/mips_target.json
> > 50:      "GYP_DEFINES": "target_arch=mips v8_target_arch=mips"
> >
> > I'm guessing it's wrong there as well?
> >
> >
> > On Wed, Feb 19, 2014 at 2:08 PM, Nico Weber <mailto:thakis@chromium.org>
wrote:
> >
> >> On Tue, Feb 18, 2014 at 7:44 PM, <mailto:iannucci@chromium.org> wrote:
> >>
> >>> On 2014/02/19 02:53:29, Nico wrote:
> >>>
> >>> https://codereview.chromium.org/59233015/diff/1560004/
> >>> scripts/slave/recipe_modules/chromium/config.py
> >>>
> >>>> File scripts/slave/recipe_modules/chromium/config.py (right):
> >>>>
> >>>
> >>>
> >>> https://codereview.chromium.org/59233015/diff/1560004/
> >>> scripts/slave/recipe_modules/chromium/config.py#newcode130
> >>>
> >>>> scripts/slave/recipe_modules/chromium/config.py:130: ('mips',  32):
> >>>> 'mips',
> >>>> This should be "mipsel", not "mips", right?
> >>>>
> >>>
> >> As I learned this morning, this recipe is actually used for
> >>
>
http://chromegw/i/clank/builders/mips-builder/builds/232/steps/gclient%2520ru...,
> and this saying "mips" instead of "mipsel" is why this bot is red. So
> >> this seems a bit more serious than "what if" code :-)
> >>
> >>
> >>>
> >>>
> >>> https://codereview.chromium.org/59233015/diff/1560004/
> >>> scripts/slave/recipe_modules/chromium/config.py#newcode131
> >>>
> >>>> scripts/slave/recipe_modules/chromium/config.py:131: ('mips',  64):
> >>>> 'mips',
> >>>> (and this shouldn't do anything yet as it's currently not used - but
> >>>> "mips" is
> >>>> fairly likely not what we want here once it's going to be used.)
> >>>>
> >>>
> >>> yeah probably. if/when we support it for real, this will change. It's
> >>> "what if"
> >>> code right now.
> >>>
> >>> https://codereview.chromium.org/59233015/
> >>>
> >>
> >>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Yes - in the v8 config there was an unused dummy value for mips in the recipe,
which wasn't used on the bots yet. It should be mipsel.