base: remove -latomic in CrOS chroot

base/BUILD.gn

 # Copyright (c) 2013 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.
 
 # HOW TO WRITE CONDITIONALS IN THIS FILE
 # ======================================
 #
 # In many other places, one would write a conditional that expresses all the
 # cases when a source file is used or unused, and then either add or subtract
 # it from the sources list in that case
@@ skipping 1110 matching lines @@
     "//third_party/modp_b64",
   ]
 
   public_deps = [
     ":base_static",
     ":build_date",
     ":debugging_flags",
   ]
 
   # 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) {
     # TODO(primiano): support other platforms, currently this works only on
     # Linux/CrOS/Android. http://crbug.com/550886 .
     sources += [
       "allocator/allocator_shim.cc",
       "allocator/allocator_shim.h",
       "allocator/allocator_shim_internals.h",
       "allocator/allocator_shim_override_cpp_symbols.h",
@@ skipping 1549 matching lines @@
 }
 
 fuzzer_test("base_json_correctness_fuzzer") {
   sources = [
     "json/correctness_fuzzer.cc",
   ]
   deps = [
     ":base",
   ]
 }