Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1517203002:
Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 (Closed)
|
Created:
5 years ago by agrieve Modified:
5 years ago Reviewers:
Roland McGrath CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix nacl_tuple "Undefined identifier" gn gen failure for arm64
BUG=568883
Committed: https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9
Cr-Commit-Position: refs/heads/master@{#365535}
Patch Set 1 #Patch Set 2 : enable_nacl #Messages
Total messages: 15 (5 generated)
Description was changed from ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 ========== to ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 ==========
agrieve@chromium.org changed reviewers: + mcgrathr@chromium.org
On 2015/12/11 11:25:39, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:mcgrathr@chromium.org mcgrathr - please review. This fixes a gn gen failure for arm64 (os=android). Note that I added a _ infront of nacl_tuple to in order to scope the variable to the .gni (hidden feature of GN)
Changing the local variable name is a fine orthogonal change. A think a cleaner fix for the actual bug would be to just put all that code inside 'if (enable_nacl)'. Does that work?
On 2015/12/11 22:57:04, Roland McGrath wrote: > Changing the local variable name is a fine orthogonal change. > A think a cleaner fix for the actual bug would be to just put all that code > inside 'if (enable_nacl)'. Does that work? I like it! Done.
lgtm
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1517203002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1517203002/20001
Message was sent while issue was closed.
Description was changed from ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 ========== to ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 ========== to ========== Fix nacl_tuple "Undefined identifier" gn gen failure for arm64 BUG=568883 Committed: https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9 Cr-Commit-Position: refs/heads/master@{#365535} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9 Cr-Commit-Position: refs/heads/master@{#365535}
Message was sent while issue was closed.
On 2015/12/16 15:37:14, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as > https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9 > Cr-Commit-Position: refs/heads/master@{#365535} This actually breaks the compilation with enable_nacl=false. Here's the error: $ gn gen out/Default ERROR at //build/toolchain/nacl/BUILD.gn:68:24: Undefined identifier rebase_path("${nacl_toolchain_dir}/${toolchain_package}/bin/pnacl-", ^----------------- See //build/toolchain/nacl/BUILD.gn:89:1: whence it was called. pnacl_toolchain("newlib_pnacl") { ^--------------------------------
Message was sent while issue was closed.
On 2015/12/16 19:48:09, aizatsky wrote: > On 2015/12/16 15:37:14, commit-bot: I haz the power wrote: > > Patchset 2 (id:??) landed as > > https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9 > > Cr-Commit-Position: refs/heads/master@{#365535} > > This actually breaks the compilation with enable_nacl=false. > > Here's the error: > > $ gn gen out/Default > ERROR at //build/toolchain/nacl/BUILD.gn:68:24: Undefined identifier > rebase_path("${nacl_toolchain_dir}/${toolchain_package}/bin/pnacl-", > ^----------------- > See //build/toolchain/nacl/BUILD.gn:89:1: whence it was called. > pnacl_toolchain("newlib_pnacl") { > ^-------------------------------- Moving nacl_toolchain_dir out of the if() scope results in a different error: $ gn gen out/Debug ERROR at //ppapi/native_client/BUILD.gn:67:18: Undefined identifier objcopy = "${nacl_toolprefix}objcopy" ^-------------- I ask this CL to be rolled back.
Message was sent while issue was closed.
On 2015/12/16 19:50:40, aizatsky wrote: > On 2015/12/16 19:48:09, aizatsky wrote: > > On 2015/12/16 15:37:14, commit-bot: I haz the power wrote: > > > Patchset 2 (id:??) landed as > > > https://crrev.com/40cdc388ef3b07937f4726d85636e74e447e64c9 > > > Cr-Commit-Position: refs/heads/master@{#365535} > > > > This actually breaks the compilation with enable_nacl=false. > > > > Here's the error: > > > > $ gn gen out/Default > > ERROR at //build/toolchain/nacl/BUILD.gn:68:24: Undefined identifier > > rebase_path("${nacl_toolchain_dir}/${toolchain_package}/bin/pnacl-", > > ^----------------- > > See //build/toolchain/nacl/BUILD.gn:89:1: whence it was called. > > pnacl_toolchain("newlib_pnacl") { > > ^-------------------------------- > > Moving nacl_toolchain_dir out of the if() scope results in a different error: > > $ gn gen out/Debug > ERROR at //ppapi/native_client/BUILD.gn:67:18: Undefined identifier > objcopy = "${nacl_toolprefix}objcopy" > ^-------------- > > I ask this CL to be rolled back. This won't happen if you explicitly set enable_pnacl=false as well as enable_nacl=false. I'll look into a change so that enable_pnacl defaults to false when enable_nacl=false is used explicitly. I think it's best to fix things so that none of the nacl-related GN rules are run at all when enable_nacl=false. |
