Add the ability to define toolchains that are compiled locally

[Ryang20718]

When building make toolchain on remote execution, autoconf has a sanity check which complains due to system clock check. Given that our remote executors are FUSE based, the way to resolve this would be to build the make toolchain which cmake rule is dependent upon to run locally.

ERROR: /home/ryang/.cache/bazel/_bazel_ryang/d74806228f9a8a972e76dd6f55d51b26/external/rules_foreign_cc/toolchains/BUILD.bazel:137:10: BootstrapGNUMake external/rules_foreign_cc/toolchains/make [for tool] failed: (Exit 1): bash failed: error executing command (from target @rules_foreign_cc//toolchains:make_tool)

....
rules_foreign_cc: Build failed!
rules_foreign_cc: Keeping temp build directory and dependencies directory for debug.
rules_foreign_cc: Please note that the directories inside a sandbox are still cleaned unless you specify --sandbox_debug Bazel command line flag.
rules_foreign_cc: Printing build logs:
_____ BEGIN BUILD LOGS _____
+ AR=/usr/bin/ar
+ ARFLAGS=rcsD
+ CC=/usr/bin/gcc
+ CFLAGS='-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 -D_FORTIFY_SOURCE=1 -DNDEBUG -ffunction-sections -fdata-sections -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__=redacted -D__TIMESTAMP__=redacted -D__TIME__=redacted -Werror'
+ LD=/usr/bin/gcc
+ LDFLAGS='-fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lstdc++ -lm -Wl,--gc-sections'
+ ./configure --without-guile --with-guile=no --disable-dependency-tracking --prefix=/worker/build/009becc197ab3312/root/bazel-out/k8-opt-exec-BA56BB51/bin/external/rules_foreign_cc/toolchains/make
checking for a BSD-compatible install... /bin/install -c
checking whether build environment is sane... configure: error: newly created file is older than distributed files!
Check your system clock
_____ END BUILD LOGS _____
rules_foreign_cc: Build wrapper script location: bazel-out/k8-opt-exec-BA56BB51/bin/external/rules_foreign_cc/toolchains/make_tool_foreign_cc/wrapper_build_script.sh
rules_foreign_cc: Build script location: bazel-out/k8-opt-exec-BA56BB51/bin/external/rules_foreign_cc/toolchains/make_tool_foreign_cc/build_script.sh
rules_foreign_cc: Build log location: bazel-out/k8-opt-exec-BA56BB51/bin/external/rules_foreign_cc/toolchains/make_tool_foreign_cc/BootstrapGNUMake.log

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Ryang20718]

Hi @jsharpe, wondering if this would be the right approach to selectively build toolchains no-remote or if you'd recommend another alternative?

(Hoping to not have to maintain a fork and rather ship upstream)

[jsharpe]

I think the way to do this would be via a string_flag (https://docs-legacy.aspect.build/bazelbuild/bazel-skylib/1.0.3/docs/common_settings.html#string_flag) that then conditionally adds the tag from the flag into the existing toolchain targets?

[Ryang20718]

appreciate the suggestion @jsharpe! However, I had initially tried that (link to original change) and it seems to be a limitation of bazel that I'm not able to add to "no-remote" to the tag in the _make_tool_impl rule. Wondering if there's any other better way to commit this change upstream?

EXECUTOR_TYPE = provider("allows users to specify execution type for a tag. e.g no-remote-exec", fields = ["execution_type"])

def _executor_type_impl(ctx):
    return EXECUTOR_TYPE(execution_type = ctx.build_setting_value)

executor_type = rule(
    implementation = _executor_type_impl,
    build_setting = config.string(flag = True),
)

def _make_tool_impl(ctx):
...
# (doesn't work) Error in append: trying to mutate a frozen list value
ctx.attr.tags.append(ctx.attr._executor_type[EXECUTOR_TYPE].execution_type)

# (doesn't work) Error: struct value does not support field assignment
ctx.attr.tags = [ctx.attr._executor_type[EXECUTOR_TYPE].execution_type] + ctx.attr.tags

[Ryang20718]

I know you're a busy person, but wanted to bump this @jsharpe 😅

[jsharpe]

I think you should be able to do this via the following diff:

diff --git a/toolchains/BUILD.bazel b/toolchains/BUILD.bazel
index cf8d4f8..e8800dc 100644
--- a/toolchains/BUILD.bazel
+++ b/toolchains/BUILD.bazel
@@ -134,10 +134,23 @@ native_tool_toolchain(
     path = "nmake.exe",
 )

+constraint_setting(name = "make_remote_build")
+
+constraint_value(
+    name = "make_no_remote_exec",
+    constraint_setting = "make_remote_build",
+)
+
 make_tool(
     name = "make_tool",
     srcs = "@gnumake_src//:all_srcs",
-    tags = ["manual"],
+    tags = select({
+        ":make_no_remote_exec": [
+            "manual",
+            "no-remote-exec",
+        ],
+        "//conditions:default": ["manual"],
+    }),
 )

 native_tool_toolchain(

Then to use it all you need to pass is --define make_remote_build=make_no_remote_exec. @Ryang20718 can you give this a go and update this PR and add some docs for this then I'm happy to merge this PR.

[Ryang20718]

@jsharpe Appreciate the diff, but after applying this diff, I received the error @rules_foreign_cc//toolchains:make_tool: attribute "tags" is not configurable

I looked into it and it seems this select for tags doesn't seem to be possible currently?

https://github.com/bazelbuild/bazel/issues/2971

[Ryang20718]

following up here 😅 . Given that we're unable to add dynamically to toolchains, wondering if you'd had any other suggestions or to perhaps go with original changes @jsharpe ?

[jsharpe]

My opinion is that this probably doesn't really belong here - its a workaround for a broken RBE FUSE implementation that doesn't preserve the posix semantics of the files that are input to the build. You are free to register your own toolchains for the make toolchain in your own workspace and all the bzl macros / rules you need to load are public. @UebelAndre or @jheaff1 do you have any opinions on this?

[jheaff1]

I’m not really familiar with remote execution so can’t be of much use here, I’m afraid 😔