[bazel] Use newer config setting for string_flag (multiple)

The change from config.string [1] to config.string_list [2]
is that the former only supports setting one item as a default,
not multiple.

If, for example, we had wanted to set the default value of
//src/codec/include_decoder to be
['jpeg_decode_codec', 'png_decode_codec']
we could not without switching to string_list.

With Bazel 5.2.0, you could set the flags for string_list
exactly once with a flag (as a comma separated list).
In 5.3.0 they added the repeatable option which allows
us to set it multiple times and have it be aggregated.
As such, we move forward with the 5.3.0rc1 (which also
lets us provide feedback before release if necessary).

This also makes our flag macros align more with Bazel
style (flag_name -> name) and relocates them to a less
hidden spot.

See also http://cl/468514170

[1] https://bazel.build/rules/lib/config#string
[2] https://bazel.build/rules/lib/config#string_list

Change-Id: Ic0ef0de51078acc7212c43e9a58b0a121240b38e
Bug: skia:12541, b/237076898
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/571419
Reviewed-by: Ben Wagner <bungeman@google.com>

.bazelversion

@@ -1 +1 @@
-5.2.0
+5.3.0rc1

bazel/Makefile

@@ -31,7 +31,7 @@ known_good_builds:
 	bazelisk build //tools/skslc
 	bazelisk build //modules/skottie:skottie_tool_cpu
 	bazelisk build //modules/skottie:skottie_tool_gpu
-	bazelisk build //modules/canvaskit:canvaskit_webgl --config=webgl2_release
+	bazelisk build //modules/canvaskit:canvaskit --config=ck_full_webgl2_release
 
 rbe_known_good_builds:
 	bazelisk build //:skia_public --config=for_linux_x64_with_rbe --remote_download_minimal
@@ -48,8 +48,8 @@ rbe_known_good_builds:
 	bazelisk build //modules/skottie:skottie_tool_gpu --config=for_linux_x64_with_rbe --remote_download_minimal
 	## TODO(kjlubick) CanvasKit in release mode (i.e. with Closure) requires
 	## https://github.com/emscripten-core/emscripten/pull/16640 to land
-	bazelisk build //modules/canvaskit:canvaskit_webgl --config=linux_rbe --config=webgl2_debug \
-		--remote_download_minimal
+	bazelisk build //modules/canvaskit:canvaskit --config=linux_rbe \
+		--config=ck_full_webgl2_debug --remote_download_minimal
 
 iwyu_rbe:
 	bazelisk build //:skia_public --config=for_linux_x64_with_rbe --config=enforce_iwyu \

bazel/common_config_settings/BUILD.bazel

@@ -14,8 +14,7 @@ config_setting(
 )
 
 # Apple devices with intel processors released before the shift to the M1 chip
-# will use this config setting. Because M1 chips use the arm cpu instruction set,
-# this flag will not be used.
+# will use this config setting.
 config_setting(
     name = "mac_x64",
     constraint_values = [
@@ -24,6 +23,7 @@ config_setting(
     ],
 )
 
+# M1 Macs (and later) will use this setting.
 config_setting(
     name = "mac_arm64",
     constraint_values = [
@@ -106,8 +106,8 @@ config_setting(
 # //src/pdf:enable_pdf_backend.
 
 string_flag_with_values(
+    name = "fontmgr_factory",
     default = "empty_fontmgr_factory",
-    flag_name = "fontmgr_factory",
     values = [
         # Makes the default SkFontMgr load fonts from a hard-coded directory on disk.
         "custom_directory_fontmgr_factory",
@@ -129,7 +129,7 @@ string_flag_with_values(
 # require setting include_fontmgr to custom_embedded_fontmgr, because those sources and settings
 # will already be compiled in due to the _factory flag.
 string_flag_with_values(
-    flag_name = "include_fontmgr",
+    name = "include_fontmgr",
     multiple = True,
     values = [
         # Allows the construction of an SkFontMgr that loads files from a programmatically
@@ -145,22 +145,22 @@ string_flag_with_values(
 )
 
 bool_flag(
+    name = "enable_effect_serialization",
     default = True,
-    flag_name = "enable_effect_serialization",
 )
 
 bool_flag(
+    name = "enable_tracing",
     # See SkTraceEventCommon.h for more on this type of tracing.
     default = True,
-    flag_name = "enable_tracing",
 )
 
 bool_flag(
+    name = "use_harfbuzz",
     default = False,
-    flag_name = "use_harfbuzz",
 )
 
 bool_flag(
+    name = "use_icu",
     default = False,
-    flag_name = "use_icu",
 )

bazel/common_config_settings/defs.bzl → bazel/flags.bzl

@@ -1,4 +1,6 @@
 """
+THIS IS THE EXTERNAL-ONLY VERSION OF THIS FILE. G3 HAS ITS OWN.
+
 This file contains helpers for defining build flags and options that are used to
 configure the Skia build.
 """
@@ -22,7 +24,8 @@ def _multi_string_impl(ctx):
 
 multi_string_flag = rule(
     implementation = _multi_string_impl,
-    build_setting = config.string(flag = True, allow_multiple = True),
+    # https://bazel.build/rules/lib/config#string_list
+    build_setting = config.string_list(flag = True, repeatable = True),
     attrs = {
         "values": attr.string_list(
             doc = "The list of allowed values for this setting. An error is raised if any other values are given.",
@@ -31,8 +34,7 @@ multi_string_flag = rule(
     doc = "A string-typed build setting that can be set multiple times on the command line",
 )
 
-# buildifier: disable=unnamed-macro
-def string_flag_with_values(flag_name, values, default = "", multiple = False):
+def string_flag_with_values(name, values, default = "", multiple = False):
     """Create a string flag and corresponding config_settings.
 
     string_flag_with_values is a Bazel Macro that defines a flag with the given name and a set
@@ -43,37 +45,29 @@ def string_flag_with_values(flag_name, values, default = "", multiple = False):
     https://docs.bazel.build/versions/main/skylark/macros.html
 
     Args:
-        flag_name: string, the name of the flag to create and use for the config_settings
+        name: string, the name of the flag to create and use for the config_settings
         values: list of strings, the valid values for this flag to be set to.
         default: string, whatever the default value should be if the flag is not set. Can be
             empty string for both a string_flag and a multi_string flag.
         multiple: boolean, True if the flag should be able to be set multiple times on the CLI.
     """
     if multiple:
         multi_string_flag(
-            name = flag_name,
+            name = name,
             # We have to specify a default value, even if that value is empty string.
             # https://docs.bazel.build/versions/main/skylark/config.html#instantiating-build-settings
-            build_setting_default = default,
-            # If empty string is the default, we need to make sure it is in the list
-            # of acceptable values. If the default is not empty string, we don't want
-            # to make empty string a valid value. Having duplicate values in the list
-            # does not cause any issues, so we can just add the default to achieve
-            # this affect.
-            values = values + [default],
+            build_setting_default = [default],
+            # We need to make sure empty string (the default) is in the list of acceptable values.
+            values = values + [""],
         )
     else:
         string_flag(
-            name = flag_name,
+            name = name,
             # We have to specify a default value, even if that value is empty string.
             # https://docs.bazel.build/versions/main/skylark/config.html#instantiating-build-settings
             build_setting_default = default,
-            # If empty string is the default, we need to make sure it is in the list
-            # of acceptable values. If the default is not empty string, we don't want
-            # to make empty string a valid value. Having duplicate values in the list
-            # does not cause any issues, so we can just add the default to achieve
-            # this affect.
-            values = values + [default],
+            # We need to make sure empty string (the default) is in the list of acceptable values.
+            values = values + [""],
         )
 
     # For each of the values given, we define a config_setting. This allows us to use
@@ -83,13 +77,12 @@ def string_flag_with_values(flag_name, values, default = "", multiple = False):
         native.config_setting(
             name = v,
             flag_values = {
-                ":" + flag_name: v,
+                ":" + name: v,
             },
             visibility = ["//:__subpackages__"],
         )
 
-# buildifier: disable=unnamed-macro
-def bool_flag(flag_name, default = True, public = True):
+def bool_flag(name, default, public = True):
     """Create a boolean flag and corresponding config_settings.
 
     bool_flag is a Bazel Macro that defines a boolean flag with the given name two config_settings,
@@ -99,30 +92,31 @@ def bool_flag(flag_name, default = True, public = True):
     Thus it is best to define both an "enabled" alias and a "disabled" alias.
 
     Args:
-        flag_name: string, the name of the flag to create and use for the config_settings
+        name: string, the name of the flag to create and use for the config_settings
         default: boolean, if the flag should default to on or off.
         public: boolean, if the flag should be usable from other packages or if it is meant to be
             combined with some other constraint.
     """
-    skylib_bool_flag(name = flag_name, build_setting_default = default)
+
+    skylib_bool_flag(name = name, build_setting_default = default)
     vis = ["//:__subpackages__"]
     if not public:
         vis = ["//visibility:private"]
 
     native.config_setting(
-        name = flag_name + "_true",
+        name = name + "_true",
         flag_values = {
             # The value must be a string, but it will be parsed to a boolean
             # https://docs.bazel.build/versions/main/skylark/config.html#build-settings-and-select
-            ":" + flag_name: "True",
+            ":" + name: "True",
         },
         visibility = vis,
     )
 
     native.config_setting(
-        name = flag_name + "_false",
+        name = name + "_false",
         flag_values = {
-            ":" + flag_name: "False",
+            ":" + name: "False",
         },
         visibility = vis,
     )

bazel/macros.bzl

@@ -11,7 +11,7 @@ load("@py_deps//:requirements.bzl", _requirement = "requirement")
 load("@bazel_gazelle//:def.bzl", _gazelle = "gazelle")
 load("@emsdk//emscripten_toolchain:wasm_rules.bzl", _wasm_cc_binary = "wasm_cc_binary")
 load("@io_bazel_rules_go//go:def.bzl", _go_binary = "go_binary", _go_library = "go_library")
-load("//bazel/common_config_settings:defs.bzl", _bool_flag = "bool_flag", _string_flag_with_values = "string_flag_with_values")
+load("//bazel/common_config_settings:flags.bzl", _bool_flag = "bool_flag", _string_flag_with_values = "string_flag_with_values")
 load("//bazel:copts.bzl", "DEFAULT_COPTS", "DEFAULT_OBJC_COPTS")
 load("//bazel:linkopts.bzl", "DEFAULT_LINKOPTS")
 

modules/canvaskit/BUILD.bazel

@@ -1,5 +1,4 @@
-load("//bazel:macros.bzl", "exports_files_legacy", "wasm_cc_binary")
-load("//bazel/common_config_settings:defs.bzl", "bool_flag")
+load("//bazel:macros.bzl", "bool_flag", "exports_files_legacy", "wasm_cc_binary")
 load("//bazel:karma_test.bzl", "karma_test")
 load("//bazel:copts.bzl", "DEFAULT_COPTS")
 
@@ -299,43 +298,43 @@ wasm_cc_binary(
 )
 
 bool_flag(
+    name = "enable_canvas_polyfill",
     default = False,
-    flag_name = "enable_canvas_polyfill",
 )
 
 bool_flag(
+    name = "enable_fonts",
     default = False,
-    flag_name = "enable_fonts",
 )
 
 bool_flag(
+    name = "include_embedded_font",
     default = False,
-    flag_name = "include_embedded_font",
 )
 
 bool_flag(
+    name = "include_matrix_js",
     default = False,
-    flag_name = "include_matrix_js",
 )
 
 bool_flag(
+    name = "enable_skottie",
     default = False,
-    flag_name = "enable_skottie",
 )
 
 bool_flag(
+    name = "enable_skp_serialization",
     default = False,
-    flag_name = "enable_skp_serialization",
 )
 
 bool_flag(
+    name = "enable_particles",
     default = False,
-    flag_name = "enable_particles",
 )
 
 bool_flag(
+    name = "enable_runtime_effect",
     default = False,
-    flag_name = "enable_runtime_effect",
 )
 
 karma_test(

src/codec/BUILD.bazel

@@ -143,7 +143,7 @@ split_srcs_and_hdrs(
 )
 
 string_flag_with_values(
-    flag_name = "include_decoder",
+    name = "include_decoder",
     multiple = True,
     values = [
         "avif_decode_codec",

src/gpu/BUILD.bazel

@@ -32,7 +32,7 @@ split_srcs_and_hdrs(
 )
 
 string_flag_with_values(
-    flag_name = "gpu_backend",
+    name = "gpu_backend",
     multiple = True,
     values = [
         "gl_backend",
@@ -42,7 +42,7 @@ string_flag_with_values(
 )
 
 string_flag_with_values(
-    flag_name = "with_gl_standard",
+    name = "with_gl_standard",
     values = [
         "gles_standard",
         "gl_standard",
@@ -70,14 +70,14 @@ selects.config_setting_group(
 )
 
 bool_flag(
+    name = "use_vulkan_memory_allocator",
     default = True,
-    flag_name = "use_vulkan_memory_allocator",
     public = False,  # Users should use :vulkan_with_vma
 )
 
 bool_flag(
+    name = "enable_gpu_test_utils",
     default = False,
-    flag_name = "enable_gpu_test_utils",
 )
 
 filegroup(

src/images/BUILD.bazel

@@ -5,7 +5,7 @@ licenses(["notice"])
 exports_files_legacy()
 
 string_flag_with_values(
-    flag_name = "include_encoder",
+    name = "include_encoder",
     multiple = True,
     values = [
         "jpeg_encode_codec",

src/pdf/BUILD.bazel

@@ -77,13 +77,13 @@ filegroup(
 )
 
 bool_flag(
+    name = "enable_pdf_backend",
     default = False,
-    flag_name = "enable_pdf_backend",
 )
 
 bool_flag(
+    name = "enable_pdf_subset_harfbuzz",
     default = True,  # This defaults it to be on if icu and harfbuzz are on
-    flag_name = "enable_pdf_subset_harfbuzz",
 )
 
 selects.config_setting_group(

src/shaders/BUILD.bazel

@@ -41,8 +41,8 @@ split_srcs_and_hdrs(
 )
 
 bool_flag(
+    name = "legacy_shader_context",
     default = True,
-    flag_name = "legacy_shader_context",
 )
 
 filegroup(

src/sksl/BUILD.bazel

@@ -96,18 +96,18 @@ selects.config_setting_group(
 )
 
 bool_flag(
+    name = "enable_sksl",
     default = False,
-    flag_name = "enable_sksl",
 )
 
 bool_flag(
+    name = "enable_skslc",
     default = False,
-    flag_name = "enable_skslc",
 )
 
 bool_flag(
+    name = "enable_sksl_tracing",
     default = False,
-    flag_name = "enable_sksl_tracing",
 )
 
 filegroup(

src/svg/BUILD.bazel

@@ -5,8 +5,8 @@ licenses(["notice"])
 exports_files_legacy()
 
 bool_flag(
+    name = "enable_svg_canvas",
     default = False,
-    flag_name = "enable_svg_canvas",
 )
 
 filegroup(