Replace create_exec_properties_dict with create_rbe_exec_properties_dict (bazelbuild#763)

* Replace create_exec_properties_dict with create_rbe_exec_properties_dict

And also replace merge_dicts with dicts.add

* Change initialization order.

In repositories/repositories.bzl in the function repositories(), I reversed the order of handling io_bazel_rules_docker" and "bazel_skylib"

* Add bazel_toolchains_repositories to bazel_toolchains_client WORKSPACE

* Remove bazel symlinks that were unintentionally introduced in prev commit.

Also add them to .gitignore.

* Modify gitignore

* Add repositories to tests/rbe_repo/examples/toolchain_config_host/WORKSPACE as well.

* Remove dependency from rules/rbe_repo/build_gen.bzl to skylib

Despite the fact that we are still deprecating merge_dicts and recommending that users use dict.add from skylib, we decided not to add that dependency from rules/rbe_repo/build_gen.bzl and instead use a locally defined _merge_dicts.

The reason for doing this is that it complicated the setup for any WORKSAPCE that only want to use rbe_autconfig. We don't want these simple WORKSPACEs to need to call respositories() in order not to break.

Author: eytankidron

.gitignore

@@ -12,6 +12,8 @@
 # Ignore all bazel-* symlinks. There is no full list since this can change
 # based on the name of the directory bazel is cloned into.
 /bazel-*
+/tests/rbe_repo/examples/bazel_toolchains_client/bazel-*
+/tests/rbe_repo/examples/toolchain_config_host/bazel-*
 /rbe-test-output/
 # Ignore outputs generated during Bazel bootstrapping.
 /output/

WORKSPACE

@@ -564,7 +564,7 @@ rbe_autoconfig(
 rbe_autoconfig_root(name = "rbe_autoconfig_root")
 
 # Define several exec property repo rules to be used in testing.
-load("//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict", "custom_exec_properties", "rbe_exec_properties")
+load("//rules/exec_properties:exec_properties.bzl", "create_rbe_exec_properties_dict", "custom_exec_properties", "rbe_exec_properties")
 
 # A standard RBE execution property set repo rule.
 rbe_exec_properties(
@@ -575,15 +575,15 @@ rbe_exec_properties(
 rbe_exec_properties(
     name = "exec_properties_with_override",
     override_constants = {
-        "NETWORK_ON": create_exec_properties_dict(docker_network = "off"),
+        "NETWORK_ON": create_rbe_exec_properties_dict(docker_network = "off"),
     },
 )
 
 # A custom execution property set repo rule defining its own name for the network on property set.
 custom_exec_properties(
     name = "network_on_exec_properties",
     constants = {
-        "BESPOKE_NETWORK_ON": create_exec_properties_dict(docker_network = "standard"),
+        "BESPOKE_NETWORK_ON": create_rbe_exec_properties_dict(docker_network = "standard"),
     },
 )
 
@@ -601,14 +601,14 @@ rbe_autoconfig(
     use_legacy_platform_definition = False,
 )
 
-load("@bazel_toolchains//rules/exec_properties:exec_properties.bzl", "merge_dicts")
+load("@bazel_skylib//lib:dicts.bzl", "dicts")
 
 # Use in the BazelCI.
 rbe_autoconfig(
     name = "buildkite_config",
     base_container_digest = "sha256:4bfd33aa9ce73e28718385b8c01608a79bc6546906f01cf9329311cace1766a1",
     digest = "sha256:c20046852a2d7910c55d76e0ec9c182b37532a9f0360d22dd5c9a1451b7c3a15",
-    exec_properties = merge_dicts(DOCKER_SIBLINGS_CONTAINERS, NETWORK_ON),
+    exec_properties = dicts.add(DOCKER_SIBLINGS_CONTAINERS, NETWORK_ON),
     registry = "marketplace.gcr.io",
     repository = "google/bazel",
     use_legacy_platform_definition = False,

rules/exec_properties/README.md

@@ -3,8 +3,7 @@
 exec_properties.bzl contains the following Starlark macros:
 * rbe_exec_properties
 * custom_exec_properties
-* create_exec_properties_dict
-* merge_dicts
+* create_rbe_exec_properties_dict
 
 ## rbe_exec_properties
 
@@ -27,16 +26,11 @@ It is highly recommended to use a globally unique name for this repo rule (and
 definitely not `exec_properties`) for reasons that are discussed in more details
 [below](#anti-patterns).
 
-## create_exec_properties_dict
+## create_rbe_exec_properties_dict
 
-`create_exec_properties_dict` is a Starlark macro that creates a dictionary of
-remote execution properties. `create_exec_properties_dict` ensures that the
-created dictionary is compatible with what RBE supports.
-
-## merge_dicts
-
-`merge_dicts` is a Starlark macro that merges dictionaries of remote execution
-properties.
+`create_rbe_exec_properties_dict` is a Starlark macro that creates a dictionary
+of remote execution properties. `create_rbe_exec_properties_dict` ensures that
+the created dictionary is compatible with what RBE supports.
 
 ## Use cases
 
@@ -101,7 +95,7 @@ In the `WORKSPACE` file, call
 rbe_exec_properties(
     name = "exec_properties",
     override_constants = {
-        "NETWORK_ON": create_exec_properties_dict(docker_network = "off"),
+        "NETWORK_ON": create_rbe_exec_properties_dict(docker_network = "off"),
     },
 )
 ```
@@ -131,7 +125,7 @@ In the `WORKSPACE` file, call:
 custom_exec_properties(
     name = "proj_a_prefix_high_mem_machine_exec_property",
     constants = {
-        "HIGH_MEM_MACHINE": create_exec_properties_dict(gce_machine_type = "n1-highmem-8"),
+        "HIGH_MEM_MACHINE": create_rbe_exec_properties_dict(gce_machine_type = "n1-highmem-8"),
     },
 )
 ```
@@ -174,27 +168,27 @@ avoid.
 }
 ```
 
-Instead, always prefer using `create_exec_properties_dict` like so:
+Instead, always prefer using `create_rbe_exec_properties_dict` like so:
 ```
-create_exec_properties_dict(
+create_rbe_exec_properties_dict(
     gce_machine_type = "n1-highmem-8",
     docker_privileged = True,
     docker_sibling_containers = True,
 )
 ```
 
-`create_exec_properties_dict` is better because typos in key names will be
+`create_rbe_exec_properties_dict` is better because typos in key names will be
 caught early, while parsing the Bazel code, instead of having RBE just ignore
 keys that it doesn't recognize and having the developer spend more time that is
 necessary trying to figure out what went wrong. Furthermore,
-`create_exec_properties_dict` will perform some validation about the values.
+`create_rbe_exec_properties_dict` will perform some validation about the values.
 
-### Anti-pattern 2 - Do not call create_exec_properties_dict from a target.
+### Anti-pattern 2 - Do not call create_rbe_exec_properties_dict from a target.
 
-Instead `create_exec_properties_dict` should be called from the `WORKSPACE` in
-the context of creating a local repo, typically using `custom_exec_properties`.
-It can also be called from a `BUILD` file in the context of defining a
-platform.
+Instead `create_rbe_exec_properties_dict` should be called from the `WORKSPACE`
+in the context of creating a local repo, typically using
+`custom_exec_properties`. It can also be called from a `BUILD` file in the
+context of defining a platform.
 
 Here is what might go wrong if it is called directly when populating the
 `exec_properties` field of a target.
@@ -208,7 +202,7 @@ executed remotely on RBE, should run on a high memory machine such as
 foo_library(
    name="my_lib",
    ...
-   exec_properties = create_exec_properties_dict(gce_machine_type = "n1-highmem-8"),
+   exec_properties = create_rbe_exec_properties_dict(gce_machine_type = "n1-highmem-8"),
 )
 ```
 

rules/exec_properties/exec_properties.bzl

@@ -200,6 +200,9 @@ PARAMS = {
 }
 
 def create_exec_properties_dict(**kwargs):
+    fail("create_exec_properties_dict is deprecated. Please use create_rbe_exec_properties_dict instead.")
+
+def create_rbe_exec_properties_dict(**kwargs):
     """Return a dict with exec_properties that are supported by RBE.
 
     Args:
@@ -236,19 +239,7 @@ def create_exec_properties_dict(**kwargs):
     return dict
 
 def merge_dicts(*dict_args):
-    """Merge any number of dicts into a new dict.
-
-    Args:
-      *dict_args: A list of zero or more dicts.
-
-    Returns:
-      A merge of the input dicts. Precedence goes to key value pairs in latter dicts.
-    """
-    result = {}
-    for dictionary in dict_args:
-        if dictionary:
-            result.update(dictionary)
-    return result
+    fail("merge_dicts is deprecated. Please use dicts.add() instead. See https://github.com/bazelbuild/bazel-skylib/blob/master/docs/dicts_doc.md")
 
 def _exec_property_sets_repository_impl(repository_ctx):
     repository_ctx.file(
@@ -325,21 +316,21 @@ def custom_exec_properties(name, constants):
     )
 
 # STANDARD_PROPERTY_SETS is the SoT for the list of constants that rbe_exec_properties defines.
-# For more information about what each parameter of create_exec_properties_dict() means, see
+# For more information about what each parameter of create_rbe_exec_properties_dict() means, see
 # https://cloud.google.com/remote-build-execution/docs/remote-execution-properties.
 STANDARD_PROPERTY_SETS = {
-    "NETWORK_ON": create_exec_properties_dict(docker_network = "standard"),
-    "NETWORK_OFF": create_exec_properties_dict(docker_network = "off"),
-    "DOCKER_PRIVILEGED": create_exec_properties_dict(docker_privileged = True),
-    "NOT_DOCKER_PRIVILEGED": create_exec_properties_dict(docker_privileged = False),
-    "DOCKER_RUN_AS_ROOT": create_exec_properties_dict(docker_run_as_root = True),
-    "NOT_DOCKER_RUN_AS_ROOT": create_exec_properties_dict(docker_run_as_root = False),
-    "DOCKER_SIBLINGS_CONTAINERS": create_exec_properties_dict(docker_sibling_containers = True),
-    "NOT_DOCKER_SIBLINGS_CONTAINERS": create_exec_properties_dict(docker_sibling_containers = False),
-    "DOCKER_USE_URANDOM": create_exec_properties_dict(docker_use_urandom = True),
-    "NOT_DOCKER_USE_URANDOM": create_exec_properties_dict(docker_use_urandom = False),
-    "LINUX": create_exec_properties_dict(os_family = "Linux"),
-    "WINDOWS": create_exec_properties_dict(os_family = "Windows"),
+    "NETWORK_ON": create_rbe_exec_properties_dict(docker_network = "standard"),
+    "NETWORK_OFF": create_rbe_exec_properties_dict(docker_network = "off"),
+    "DOCKER_PRIVILEGED": create_rbe_exec_properties_dict(docker_privileged = True),
+    "NOT_DOCKER_PRIVILEGED": create_rbe_exec_properties_dict(docker_privileged = False),
+    "DOCKER_RUN_AS_ROOT": create_rbe_exec_properties_dict(docker_run_as_root = True),
+    "NOT_DOCKER_RUN_AS_ROOT": create_rbe_exec_properties_dict(docker_run_as_root = False),
+    "DOCKER_SIBLINGS_CONTAINERS": create_rbe_exec_properties_dict(docker_sibling_containers = True),
+    "NOT_DOCKER_SIBLINGS_CONTAINERS": create_rbe_exec_properties_dict(docker_sibling_containers = False),
+    "DOCKER_USE_URANDOM": create_rbe_exec_properties_dict(docker_use_urandom = True),
+    "NOT_DOCKER_USE_URANDOM": create_rbe_exec_properties_dict(docker_use_urandom = False),
+    "LINUX": create_rbe_exec_properties_dict(os_family = "Linux"),
+    "WINDOWS": create_rbe_exec_properties_dict(os_family = "Windows"),
 }
 
 def rbe_exec_properties(name, override_constants = None):

rules/rbe_repo.bzl

@@ -258,17 +258,17 @@ Here is an example of an rbe_autoconfig that configures its underlying platform
 to set the size of the shared memory partition for the docker container to 128
 megabytes.
 
-load("@bazel_toolchains//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict")
+load("@bazel_toolchains//rules/exec_properties:exec_properties.bzl", "create_rbe_exec_properties_dict")
 
 rbe_autoconfig(
     name = "rbe_default",
     use_legacy_platform_definition = False,
-    exec_properties = create_exec_properties_dict(docker_shm_size = "128m"),
+    exec_properties = create_rbe_exec_properties_dict(docker_shm_size = "128m"),
 )
 
-Note the use of create_exec_properties_dict. This is a Bazel macro that makes
-it convenient to create the dicts used in exec_properties. You should always
-prefer to use it over composing the dict manually.
+Note the use of create_rbe_exec_properties_dict. This is a Bazel macro that
+makes it convenient to create the dicts used in exec_properties. You should
+always prefer to use it over composing the dict manually.
 
 Additionally, there are standard execution property dicts that you may want to
 use. These standard dicts should always be preferred if defined. These standard
@@ -290,8 +290,8 @@ rbe_autoconfig(
     exec_properties = NETWORK_ON,
 )
 
-For more information on create_exec_properties_dict, rbe_exec_properties and
-other related Bazel macros, see https://github.com/bazelbuild/bazel-toolchains/tree/master/rules/exec_properties
+For more information on create_rbe_exec_properties_dict, rbe_exec_properties
+and other related Bazel macros, see https://github.com/bazelbuild/bazel-toolchains/tree/master/rules/exec_properties
 
 NOTES:
 

rules/rbe_repo/build_gen.bzl

@@ -19,10 +19,18 @@ load(
     "JAVA_CONFIG_DIR",
     "PLATFORM_DIR",
 )
-load("//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict", "merge_dicts")
+load("//rules/exec_properties:exec_properties.bzl", "create_rbe_exec_properties_dict")
 
 _CC_TOOLCHAIN = ":cc-compiler-k8"
 
+# Defining a local version of dicts.add in order not to create a dependency on bazel_skylib.
+def _merge_dicts(*dict_args):
+    result = {}
+    for dictionary in dict_args:
+        if dictionary:
+            result.update(dictionary)
+    return result
+
 def create_config_aliases(ctx, toolchain_config_spec_name):
     """Produces BUILD files with alias for the C++ and Java toolchain targets.
 
@@ -160,11 +168,11 @@ def _create_platform(ctx, exec_properties, image_name, name, cc_toolchain_target
                               ("\",\n        \"").join(ctx.attr.target_compatible_with) +
                               "\",")
 
-    platform_exec_properties = create_exec_properties_dict(
+    platform_exec_properties = create_rbe_exec_properties_dict(
         container_image = "docker://%s" % image_name,
         os_family = "Linux",
     )
-    platform_exec_properties = merge_dicts(platform_exec_properties, exec_properties)
+    platform_exec_properties = _merge_dicts(platform_exec_properties, exec_properties)
 
     ctx.template(
         PLATFORM_DIR + "/BUILD",

tests/rules/exec_properties/BUILD

@@ -13,17 +13,17 @@
 # limitations under the License.
 
 load(":compare.bzl", "compare_dicts_test")
-load("//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict")
+load("//rules/exec_properties:exec_properties.bzl", "create_rbe_exec_properties_dict")
 
 compare_dicts_test(
     name = "docker_network_compare_test",
-    actual = create_exec_properties_dict(docker_network = "standard"),
+    actual = create_rbe_exec_properties_dict(docker_network = "standard"),
     expected = {"dockerNetwork": "standard"},
 )
 
 compare_dicts_test(
     name = "multiple_properties_compare_test",
-    actual = create_exec_properties_dict(
+    actual = create_rbe_exec_properties_dict(
         docker_privileged = True,
         os_family = "Windows",
     ),
@@ -35,7 +35,7 @@ compare_dicts_test(
 
 compare_dicts_test(
     name = "labels_compare_test",
-    actual = create_exec_properties_dict(labels = {
+    actual = create_rbe_exec_properties_dict(labels = {
         "abc": "123",
         "def": "456",
     }),

tests/rules/exec_properties/compare.bzl

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""This file contains a test rule compare_dicts_test for unit testing create_exec_properties_dict.
+"""This file contains a test rule compare_dicts_test for unit testing create_rbe_exec_properties_dict.
 
 """