Revert "build_vars.txt: Changed to build_vars.json"

This reverts commit 6e002e1f.

Reason for revert: Official build failure crbug.com/1114609

Original change's description:
> build_vars.txt: Changed to build_vars.json
> 
> And moved helper function into gn_helpers.py
> 
> It was only ever in .properties format so that it
> could be easily parsed by bash, but we no longer need
> that.
> 
> Also fixes default_android_sdk_version to be a string.
> -next builds need it to be a string.
> 
> Bug: None
> Change-Id: I99541f18510de93dae4c0d68b734638564f74ee2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340673
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@google.com>
> Cr-Commit-Position: refs/heads/master@{#796020}

TBR=dpranke@google.com,agrieve@chromium.org,mheikal@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: None
Change-Id: Iad8597f6af00c534245fd1a40d1809d952758f47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2344626Reviewed-by: Ben Mason <benmason@chromium.org>
Commit-Queue: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796387}

build/android/BUILD.gn

@@ -2,7 +2,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import("//build/config/android/build_vars.gni")
 import("//build/config/android/config.gni")
 import("//build/config/android/rules.gni")
 import("//build/config/python.gni")
@@ -22,6 +21,28 @@ if (enable_java_templates) {
       min_sdk_version = default_min_sdk_version
     }
   }
+
+  # Write to a file some GN vars that are useful to scripts that use the output
+  # directory. Format is chosen as easliy importable by both python and bash.
+  _lines = [
+    "android_sdk_build_tools=" +
+        rebase_path(android_sdk_build_tools, root_build_dir),
+    "android_sdk_build_tools_version=$android_sdk_build_tools_version",
+    "android_sdk_root=" + rebase_path(android_sdk_root, root_build_dir),
+    "android_sdk_version=$android_sdk_version",
+    "android_ndk_root=" + rebase_path(android_ndk_root, root_build_dir),
+    "android_tool_prefix=" + rebase_path(android_tool_prefix, root_build_dir),
+    "android_configuration_failure_dir=" +
+        rebase_path(android_configuration_failure_dir, root_build_dir),
+    "final_android_sdk=$final_android_sdk"
+  ]
+  if (defined(android_secondary_abi_cpu)) {
+    _secondary_label_info =
+        get_label_info(":foo($android_secondary_abi_toolchain)", "root_out_dir")
+    _lines += [ "android_secondary_abi_toolchain=" +
+                rebase_path(_secondary_label_info, root_build_dir) ]
+  }
+  write_file(android_build_vars, _lines)
 }
 
 python_library("devil_chromium_py") {
@@ -77,7 +98,7 @@ python_library("resource_sizes_py") {
     "//third_party/catapult/tracing:convert_chart_json",
   ]
   data = [
-    build_vars_file,
+    android_build_vars,
     android_readelf,
   ]
 }
@@ -105,19 +126,3 @@ if (build_with_chromium) {
         [ "//third_party/android_platform/development/scripts:stack_py" ]
   }
 }
-
-# GN evaluates each .gn file once per toolchain, so restricting to default
-# toolchain will ensure write_file() is called only once.
-assert(current_toolchain == default_toolchain)
-
-# NOTE: If other platforms would benefit from exporting variables, we should
-# move this to a more top-level place.
-# It is currently here (instead of //BUILD.gn) to ensure that the file is
-# written even for non-chromium embedders of //build.
-_build_vars_json = {
-  # Underscore prefix so that it appears at the top.
-  _HEADER = "Generated during 'gn gen' by //build/android/BUILD.gn."
-  forward_variables_from(android_build_vars_json, "*")
-}
-
-write_file(build_vars_file, _build_vars_json, "json")

build/android/gradle/generate_gradle.py

@@ -29,9 +29,6 @@ import jinja_template
 from util import build_utils
 from util import resource_utils
 
-sys.path.append(os.path.dirname(_BUILD_ANDROID))
-import gn_helpers
-
 _DEPOT_TOOLS_PATH = os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party',
                                  'depot_tools')
 _DEFAULT_ANDROID_MANIFEST_PATH = os.path.join(
@@ -109,6 +106,11 @@ def _WriteFile(path, data):
     output_file.write(data)
 
 
+def _ReadPropertiesFile(path):
+  with open(path) as f:
+    return dict(l.rstrip().split('=', 1) for l in f if '=' in l)
+
+
 def _RunGnGen(output_dir, args=None):
   cmd = [os.path.join(_DEPOT_TOOLS_PATH, 'gn'), 'gen', output_dir]
   if args:
@@ -823,11 +825,10 @@ def main():
         for t in targets_from_args
     ]
     # Necessary after "gn clean"
-    if not os.path.exists(
-        os.path.join(output_dir, gn_helpers.BUILD_VARS_FILENAME)):
+    if not os.path.exists(os.path.join(output_dir, 'build_vars.txt')):
       _RunGnGen(output_dir)
 
-  build_vars = gn_helpers.ReadBuildVars(output_dir)
+  build_vars = _ReadPropertiesFile(os.path.join(output_dir, 'build_vars.txt'))
   jinja_processor = jinja_template.JinjaProcessor(_FILE_DIR)
   if args.beta:
     channel = 'beta'

build/android/gyp/util/build_utils.py

@@ -83,6 +83,12 @@ def FindInDirectory(directory, filename_filter='*'):
   return files
 
 
+def ReadBuildVars(path):
+  """Parses a build_vars.txt into a dict."""
+  with open(path) as f:
+    return dict(l.rstrip().split('=', 1) for l in f)
+
+
 def ParseGnList(value):
   """Converts a "GN-list" command-line parameter into a list.
 

build/android/resource_sizes.py

@@ -35,17 +35,14 @@ _AAPT_PATH = lazy.WeakConstant(lambda: build_tools.GetPath('aapt'))
 _BUILD_UTILS_PATH = os.path.join(
     host_paths.DIR_SOURCE_ROOT, 'build', 'android', 'gyp')
 
-with host_paths.SysPath(os.path.join(host_paths.DIR_SOURCE_ROOT, 'build')):
-  import gn_helpers  # pylint: disable=import-error
-
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
-  import perf_tests_results_helper  # pylint: disable=import-error
+  import perf_tests_results_helper # pylint: disable=import-error
 
 with host_paths.SysPath(host_paths.TRACING_PATH):
-  from tracing.value import convert_chart_json  # pylint: disable=import-error
+  from tracing.value import convert_chart_json # pylint: disable=import-error
 
 with host_paths.SysPath(_BUILD_UTILS_PATH, 0):
-  from util import build_utils  # pylint: disable=import-error
+  from util import build_utils # pylint: disable=import-error
   from util import zipalign  # pylint: disable=import-error
 
 
@@ -576,7 +573,8 @@ def _ConfigOutDirAndToolsPrefix(out_dir):
       out_dir = constants.GetOutDirectory()
     except Exception:  # pylint: disable=broad-except
       return out_dir, ''
-  build_vars = gn_helpers.ReadBuildVars(out_dir)
+  build_vars = build_utils.ReadBuildVars(
+      os.path.join(out_dir, "build_vars.txt"))
   tool_prefix = os.path.join(out_dir, build_vars['android_tool_prefix'])
   return out_dir, tool_prefix
 

build/config/android/build_vars.gni

-# Copyright 2020 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.
-
-import("//build/config/android/config.gni")
-
-# Contains useful GN variables that may be used by scripts that take
-# --output-directory as an arg.
-build_vars_file = "$root_build_dir/build_vars.json"
-
-android_build_vars_json = {
-  if (enable_java_templates) {
-    android_ndk_root = rebase_path(android_ndk_root, root_build_dir)
-    android_sdk_build_tools =
-        rebase_path(android_sdk_build_tools, root_build_dir)
-    android_sdk_build_tools_version = android_sdk_build_tools_version
-    android_sdk_root = rebase_path(android_sdk_root, root_build_dir)
-    android_sdk_version = android_sdk_version
-    android_tool_prefix = rebase_path(android_tool_prefix, root_build_dir)
-    final_android_sdk = final_android_sdk
-
-    if (defined(android_secondary_abi_cpu)) {
-      android_secondary_abi_toolchain =
-          rebase_path(get_label_info(":foo($android_secondary_abi_toolchain)",
-                                     "root_out_dir"),
-                      root_build_dir)
-    }
-  }
-}

build/config/android/config.gni

@@ -84,7 +84,7 @@ if (is_android || is_chromeos) {
   public_android_sdk_root = "//third_party/android_sdk/public"
   if (android_sdk_release == "r") {
     default_android_sdk_root = public_android_sdk_root
-    default_android_sdk_version = "30"
+    default_android_sdk_version = 30
     default_android_sdk_build_tools_version = "30.0.1"
     public_android_sdk = true
   }
@@ -138,6 +138,9 @@ if (is_android || is_chromeos) {
   assert(defined(default_android_sdk_root),
          "SDK release " + android_sdk_release + " not recognized.")
 
+  # Where to write failed expectations for bots to read.
+  android_configuration_failure_dir = "$root_build_dir/failed_expectations"
+
   declare_args() {
     android_ndk_root = default_android_ndk_root
     android_ndk_version = default_android_ndk_version
@@ -247,6 +250,9 @@ if (is_android || is_chromeos) {
     enable_jdk_library_desugaring = false
   }
 
+  # Path to where selected build variables are written to.
+  android_build_vars = "$root_build_dir/build_vars.txt"
+
   # Host stuff -----------------------------------------------------------------
 
   # Defines the name the Android build gives to the current host CPU

build/config/android/internal_rules.gni

@@ -82,9 +82,6 @@ _desugar_jdk_libs_json = "//third_party/r8/desugar_jdk_libs.json"
 _desugar_jdk_libs_jar = "//third_party/android_deps/libs/com_android_tools_desugar_jdk_libs/desugar_jdk_libs-1.0.5.jar"
 _desugar_runtime_jar = "$root_build_dir/obj/third_party/bazel/desugar/Desugar_runtime.processed.jar"
 
-# Where to write failed expectations for bots to read.
-_expectations_failure_dir = "$root_build_dir/failed_expectations"
-
 _dexdump_path = "$android_sdk_build_tools/dexdump"
 _dexlayout_path = "//third_party/android_build_tools/art/dexlayout"
 _profman_path = "//third_party/android_build_tools/art/profman"
@@ -1265,7 +1262,7 @@ if (enable_java_templates) {
         ]
         _actual_file = "$target_gen_dir/$target_name.proguard_configs"
         _failure_file =
-            "$_expectations_failure_dir/" +
+            "$android_configuration_failure_dir/" +
             string_replace(invoker.expected_proguard_config, "/", "_")
         outputs = [ _actual_file ]
         args = _args + [
@@ -2644,7 +2641,7 @@ if (enable_java_templates) {
       action_with_pydeps(_expectations_target) {
         _actual_file = "${invoker.android_manifest}.normalized"
         _failure_file =
-            "$_expectations_failure_dir/" +
+            "$android_configuration_failure_dir/" +
             string_replace(invoker.expected_android_manifest, "/", "_")
         inputs = [
           invoker.android_manifest,
@@ -2923,7 +2920,7 @@ if (enable_java_templates) {
       action_with_pydeps(_expectations_target) {
         _actual_file = "$target_gen_dir/$target_name.libs_and_assets"
         _failure_file =
-            "$_expectations_failure_dir/" +
+            "$android_configuration_failure_dir/" +
             string_replace(invoker.expected_libs_and_assets, "/", "_")
         inputs = [
           invoker.build_config,
@@ -4143,7 +4140,7 @@ template("create_android_app_bundle_module") {
     _expectations_target = "${invoker.top_target_name}_validate_libs_and_assets"
     action_with_pydeps(_expectations_target) {
       _actual_file = "$target_gen_dir/$target_name.libs_and_assets"
-      _failure_file = "$_expectations_failure_dir/" +
+      _failure_file = "$android_configuration_failure_dir/" +
                       string_replace(invoker.expected_libs_and_assets, "/", "_")
       inputs = [
         invoker.expected_libs_and_assets,

build/gn_helpers.py

@@ -20,7 +20,6 @@ Where the sequence of parameters to join is the relative path from your source
 file to the build directory.
 """
 
-import json
 import os
 import re
 import sys
@@ -28,7 +27,6 @@ import sys
 
 _CHROMIUM_ROOT = os.path.join(os.path.dirname(__file__), os.pardir)
 
-BUILD_VARS_FILENAME = 'build_vars.json'
 IMPORT_RE = re.compile(r'^import\("//(\S+)"\)')
 
 
@@ -498,9 +496,3 @@ class GNValueParser(object):
       self.cur = end
       return True
     return False
-
-
-def ReadBuildVars(output_directory):
-  """Parses $output_directory/build_vars.json into a dict."""
-  with open(os.path.join(output_directory, BUILD_VARS_FILENAME)) as f:
-    return json.load(f)

tools/binary_size/libsupersize/path_util.py

@@ -6,7 +6,6 @@
 
 import abc
 import distutils.spawn
-import json
 import logging
 import os
 
@@ -135,11 +134,11 @@ class ToolPrefixFinder(_PathFinder):
 
 
 def _LoadBuildVars(output_directory):
-  build_vars_path = os.path.join(output_directory, 'build_vars.json')
+  build_vars_path = os.path.join(output_directory, 'build_vars.txt')
   if os.path.exists(build_vars_path):
     with open(build_vars_path) as f:
-      return json.load(f)
-  return {}
+      return dict(l.rstrip().split('=', 1) for l in f if '=' in l)
+  return dict()
 
 
 def GetSrcRootFromOutputDirectory(output_directory):

tools/binary_size/libsupersize/testdata/mock_source_directory/out/Release/build_vars.json

-{
-  "android_sdk_root": "../../../mock_sdk",
-  "android_tool_prefix": "../../../mock_toolchain/"
-}

tools/binary_size/libsupersize/testdata/mock_source_directory/out/Release/build_vars.txt

+android_sdk_root=../../../mock_sdk
+android_tool_prefix=../../../mock_toolchain/