[zircon][gn] Fix environment_redirect() for special-case selectors lists

When an environment() sets variant_selectors so it doesn't match the
default list used in common environments, things get even hairier for
environment_redirect().  Previously environment() documented that
default_variants must be in the list for environment_redirect() to work
right, but even that was not actually enough.  Instead, remove that
requirement and make environment() always define a dummy environment with
the "default redirect" variant name that environment_redirect() will
choose.  Make that logic choose only from default_variants and line up with
the fallback dummy variant name logic in environment().

Bug: BLD-430
Change-Id: I399a38ba494f4fafc28a3d9e93dece101eab3732

zircon/public/gn/toolchain/environment.gni

@@ -336,10 +336,9 @@ declare_args() {
 #     match anything.  Each `variant` scope can have its own $configs,
 #     $implicit_deps, $globals, $toolchain_args, and $toolchain_vars that
 #     are merged with the environment's (potentially overriding their
-#     individual elements).  Note: environment_redirect() with
-#     `environment_label` set to this environment may not work if this does
-#     not include $default_variants.  Default: $variants +
-#     $default_variants + $standard_variants
+#     individual elements).
+#     - Type: list(scope)
+#     - Default: $variants + $default_variants + $standard_variants
 #
 #   * exclude_variant_tags
 #     - Optional: A list of strings that might appear in the .tags list of a
@@ -620,87 +619,94 @@ template("environment") {
   } else {
     exclude_variant_tags += [ "kernel" ]
   }
-  if (exclude_variant_tags != []) {
-    # Weed out the variants that this environment silently excludes.
-    excluded_variants = []
-    foreach(variant, toolchain_variants) {
-      if (exclude_variant_tags + variant.tags - variant.tags !=
-          exclude_variant_tags) {
-        excluded_variants += [ variant.name ]
-        toolchain_variants -= [ variant ]
-      }
+
+  # Weed out the variants that this environment silently excludes.
+  excluded_variants = []
+  included_variants = []
+  foreach(variant, toolchain_variants) {
+    if (exclude_variant_tags + variant.tags - variant.tags !=
+        exclude_variant_tags) {
+      excluded_variants += [ variant.name ]
+      toolchain_variants -= [ variant ]
+    } else {
+      included_variants += [ variant.name ]
     }
-    if (excluded_variants != []) {
-      # Weed out the selectors that would choose an excluded variant.
-      excluded_selectors = []
-      foreach(selector, variant_selectors) {
-        if (excluded_variants + [ selector.variant ] - [ selector.variant ] !=
-            excluded_variants) {
-          excluded_selectors += [ selector ]
-          excluded_selectors -= [ selector ]
-          excluded_selectors += [ selector ]
-        }
+  }
+  if (excluded_variants != []) {
+    # Weed out the selectors that would choose an excluded variant.
+    excluded_selectors = []
+    foreach(selector, variant_selectors) {
+      if (excluded_variants + [ selector.variant ] - [ selector.variant ] !=
+          excluded_variants) {
+        excluded_selectors += [ selector ]
+        excluded_selectors -= [ selector ]
+        excluded_selectors += [ selector ]
       }
-      variant_selectors -= excluded_selectors
     }
+    variant_selectors -= excluded_selectors
   }
-  if (excluded_variants != []) {
-    # Now excluded_variants lists toolchain names we know we never actually
-    # want to compile anything in.  However, environment_redirect() has to
-    # pick a default to redirect through even though it has no way to see
-    # exclude_variant_tags or the other factors that make this environment
-    # actually exclude a variant.  So, the first likely-looking one in
-    # variants + default_variants has to exist so it can redirect to the
-    # right one.  When the one it would choose is in excluded_variants, we
-    # need to make that toolchain exist as a dummy for those redirects.
-    redirect_default = ""
 
-    # This must be kept in synch with the loop in environment_redirect().
-    foreach(default, variants + default_variants) {
-      if (redirect_default == "") {
-        if (default == "$default") {
-          foreach(shorthand, variant_shorthands) {
-            if (default != "" &&
-                (default == shorthand.name ||
-                 get_path_info(default, "dir") == shorthand.name)) {
-              default = ""
-            }
-          }
-          if (default != "") {
-            default = {
-              variant = default
-            }
+  # Now excluded_variants lists toolchain names we know we never actually
+  # want to compile anything in.  However, environment_redirect() has to
+  # pick a default to redirect through even though it has no way to see
+  # exclude_variant_tags or the other factors that make this environment
+  # actually exclude a variant.  So, the first likely-looking one in
+  # $default_variants has to exist so it can redirect to the right one.
+  # When the one it would choose is in excluded_variants, we need to make
+  # that toolchain exist as a dummy for those redirects.
+  #
+  # Furthermore, in case $variant_selectors for this environment()
+  # doesn't include $default_variants, we need to treat that "first
+  # likely-looking one" the same way so environment_redirect() works.
+  redirect_default = ""
+
+  # **NOTE:** This code must be kept in synch with the corresponding loop
+  # in environment_redirect().
+  foreach(default, default_variants) {
+    if (redirect_default == "") {
+      if (default == "$default") {
+        foreach(shorthand, variant_shorthands) {
+          if (default != "" &&
+              (default == shorthand.name ||
+               get_path_info(default, "dir") == shorthand.name)) {
+            default = ""
           }
         }
-        if (default != "" &&
-            (!defined(default.environment) || default.environment == [] ||
-             default.environment + [ environment ] - [ environment ] !=
-             default.environment ||
-             default.environment + [ base_environment ] -
-             [ base_environment ] != default.environment)) {
-          redirect_default = default.variant
+        if (default != "") {
+          default = {
+            variant = default
+          }
         }
       }
+      if (default != "" &&
+          (!defined(default.environment) || default.environment == [] ||
+           default.environment + [ environment ] - [ environment ] !=
+           default.environment ||
+           default.environment + [ base_environment ] - [ base_environment ] !=
+           default.environment)) {
+        redirect_default = default.variant
+      }
     }
-    if (redirect_default != "" && excluded_variants + [ redirect_default ] -
-                                  [ redirect_default ] == excluded_variants) {
-      # It's not excluded, so we don't have to do anything.
-      redirect_default = ""
-    }
-    if (redirect_default != "") {
-      # Define an additional toolchain under the expected name.  It may wind
-      # up instantiating a lot of targets, which is unfortunate since its
-      # only purpose is to resolve redirect group() targets.  But to make
-      # sure nothing goes awry because the code evaluated is confused by
-      # wrong toolchain definitions, make it a copy in all by name of a
-      # variant where we actually do expect to instantiate and build targets.
-      toolchain_variants += [
-        {
-          forward_variables_from(toolchain_variants[0], "*", [ "name" ])
-          name = redirect_default
-        },
-      ]
-    }
+  }
+  assert(redirect_default != "",
+         "BUG: failed to deduce a default variant!" +
+             " check this code against environment_redirect() ???")
+
+  # If it's included we don't have to do anything.
+  if (included_variants + [ redirect_default ] - [ redirect_default ] ==
+      included_variants) {
+    # Define an additional toolchain under the expected name.  It may wind
+    # up instantiating a lot of targets, which is unfortunate since its
+    # only purpose is to resolve redirect group() targets.  But to make
+    # sure nothing goes awry because the code evaluated is confused by
+    # wrong toolchain definitions, make it a copy in all but name of a
+    # variant where we actually do expect to instantiate and build targets.
+    toolchain_variants += [
+      {
+        forward_variables_from(toolchain_variants[0], "*", [ "name" ])
+        name = redirect_default
+      },
+    ]
   }
 
   # Reuse the selectors local so we can shadow variant_selectors below.
@@ -711,11 +717,15 @@ template("environment") {
     selectors += [
       {
         forward_variables_from(selector, "*")
+        assert(!defined(tags))
         foreach(variant, toolchain_variants) {
           if (variant.name == selector.variant) {
             tags = env_tags + variant.tags
           }
         }
+        assert(defined(tags),
+               "environment(\"$target_name\") variant_selectors element" +
+                   " references undefined variant \"${selector.variant}\"")
       },
     ]
   }

zircon/public/gn/toolchain/environment_redirect.gni

@@ -164,14 +164,30 @@ template("environment_redirect") {
   if (!defined(variant) && (!defined(toolchain.environment_label) ||
                             toolchain.environment_label != environment_label ||
                             current_cpu != cpu || current_os != os)) {
-    # If the caller didn't request a specific variant, then use the default
-    # most likely to be selected for an arbitrary target in the selected
-    # environment.  Then redirect to this target in that toolchain, which
-    # will have enough information to use _variant_target() below to do
-    # the final redirect.
-    # **NOTE:** environment() has special-case code
-    # based on this logic; see the comments there that mention redirects.
-    foreach(default, variants + default_variants) {
+    # If the caller didn't request a specific variant, then use some variant
+    # known to exist.  Then redirect to this target in that toolchain, which
+    # will have enough information to use _variant_target() below to do the
+    # final redirect.  We could try here to find the default most likely to be
+    # selected for an arbitrary target in the selected environment in hopes of
+    # reducing the likelihood that a second redirect will actually be needed
+    # (and thus reducing the number of extra toolchains in which any targets
+    # might need to be instantiated that will never be built).  Most
+    # environments select from `variants + default_variants`, so we could
+    # guess based on that list.  However, we can't be sure that the target
+    # environment actually uses $variants at all, so we could select a variant
+    # from a catch-all selector in $variants that doesn't exist at all in the
+    # target environment (let alone it being the actual correct default for
+    # that environment, which we can't know).  But any such guessing would
+    # never be perfect--if we could determine that correctly with the
+    # information available to us, then we'd never need a second redirect in
+    # the first place.  So rather than using $variants, we just take the first
+    # plausible selector from $default_variants.  In the common environments
+    # and when $variants is not set, this will be the right default.  In case
+    # the target environment didn't even include $default_variants in its
+    # selector list at all, environment() has special-case code to add a dummy
+    # toolchain named for the "plausible" variant this logic will select.
+    # **NOTE:** See the comments there that mention redirects.
+    foreach(default, default_variants) {
       if (!defined(variant)) {
         if (default == "$default") {
           foreach(shorthand, variant_shorthands) {