base: remove -latomic in CrOS chroot

base/BUILD.gn

Index: base/BUILD.gn
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 78953bef001f4cec586d935a9680e804755f31d3..d4d600596c39497d03863839aa8ce9e76d12a68c 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -1129,7 +1129,9 @@ component("base") {
 
   # Needed for <atomic> if using newer C++ library than sysroot
   if (!use_sysroot && (is_android || (is_linux && !is_chromecast))) {

[gurchetansingh, 2017/04/20 21:43:13]

You can simplify with an !is_chromeos check here.

[richard.townsend, 2017/04/21 11:31:52]

That's what I thought, but surprisingly it doesn't work. Adding a quick
print(target_os, is_chromeos, is_linux, host_toolchain, current_toolchain)
before the comment yields:

  chromeos true true //build/toolchain/cros:host //build/toolchain/cros:target
  chromeos false true //build/toolchain/cros:host //build/toolchain/cros:host
  chromeos false false //build/toolchain/cros:host
//build/toolchain/nacl:newlib_pnacl_nonsfi
  chromeos false false //build/toolchain/cros:host
//build/toolchain/nacl:irt_x64
  chromeos false false //build/toolchain/cros:host
//build/toolchain/nacl:newlib_pnacl

So when building tools for the host with the compiled_action target, is_chromeos
is false, which means the error doesn't go away.

[Dirk Pranke, 2017/04/24 21:12:22]

If I understand this correctly, basically the problem is that the cros build is
"like" a sysroot build but isn't a sysroot build, in the sense that the
chroot/ebuild also doesn't have a newer c++ library and so doesn't need
-latomic. Is that right?

Unfortunately, checking just for target_os != 'chromeos' won't work, because we
also will want -latomic for the cases where we're building the "desktop
chromeos" config, like on the
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...
builder.

I think instead you should change this to look at the current_toolchain and
compare it to '//build/toolschain/cros:target' or '//build/toolchain/cros:host'
and add a comment similar to what I wrote in the first paragraph. I think that
might make it a bit clearer what's going on. 
Checking against the toolchain is the "easiest" way of telling if we're in an
ebuild or simplechrome build that we have at the moment.

[richard.townsend, 2017/04/25 14:09:34]

I did some further investigation on the native Linux/ChromeOS configuration on
my Ubuntu 14.04 and 16.04 systems, just building the affected host tool (i.e.
revert 5e671fffc45441c5dd3458871a8aa8f4a3f2e632,  'ninja -C out/ChromeOS
net/http:generate_transport_security_state'), with these gn args:

 is_component_build = false
 is_debug = false
 target_os = "chromeos"

Here's a summary of what I found.

* Outside CrOS sysroot, target_os="chromeos", use_sysroot=true,  prior to patch
= no -latomic, compiles
* Outside CrOS sysroot, target_os="chromeos", use_sysroot=true,  with patch    
= no -latomic, compiles
* Outside CrOS sysroot, target_os="chromeos", use_sysroot=false, prior to patch
=    -latomic, compiles
* Outside CrOS sysroot, target_os="chromeos", use_sysroot=false, with patch    
= no -latomic, compiles
* Inside CrOS sysroot, without patch, use_sysroot=false =    -latomic, errors
* Inside CrOS sysroot, without patch, use_sysroot=true  = no -latomic, compiles
* Inside CrOS sysroot, with patch   , use_sysroot=false = no -latomic, compiles
* Inside CrOS sysroot, with patch   , use_sysroot=true  = no -latomic, compiles

The fact that you don't need '-latomic' to compile on the native system is
surprising to me. Reading through the original commit at
http://crrev.com/f3f51116a0689, it seems the author had problems compiling with
GCC 5.x, but a quick gn arg is_clang=false on my Ubuntu 16.04 system (with GCC
5.4), again, compiles fine, with and without -latomic. My only remaining thought
is that there must have been something wrong with the committer's environment:
perhaps a custom-compiled GCC.

So to summarize my summary, it shouldn't break this particular job, but the
investigation throws further doubt on why this flag is needed.

 

-    libs = [ "atomic" ]
+    if (target_os != "chromeos") {
+      libs = [ "atomic" ]
+    }
   }
 
   if (use_experimental_allocator_shim) {