Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote execution from windows host to linux executor #19587

Open
UebelAndre opened this issue Sep 21, 2023 · 10 comments
Open

Remote execution from windows host to linux executor #19587

UebelAndre opened this issue Sep 21, 2023 · 10 comments
Labels
area-Windows Windows-specific issues and feature requests P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@UebelAndre
Copy link
Contributor

Description of the feature request:

I would like windows users to be able to build and test changes on linux via remote execution. The desired workflow is that there's a remote config in the repository .bazelrc file that provides all the necessary flags to enable remote execution and describe that the desired execution and target platforms are linux (despite the client being on windows).

Which category does this issue belong to?

No response

What underlying problem are you trying to solve with this feature?

Large organizations that have a primarily linux software stack but have less-technical contributors that run on windows should be able to use Bazel to empower people to to work on the machine they're familiar with and run tests in a remote execution cluster on a supported platform.

I tried setting this up myself and I ran into the following before concluding that supporting this was not something I could do with any changes to my repository and that Bazel would need to be updated to support this:

  1. bazel_tools is platform specific. When I run on windows, I get a bazel_tools repository that has a ton of .exe files in it and is missing files needed for linux
  2. In bazel_tools there's some old reliance on --host_cpu that seems to select windows branches. These should simply be @platform targets.
  3. Bazel assumes if the client is windows that the java runtime must be bin/java.exe. This is wrong and the execution should instead be checked. https://github.com/bazelbuild/bazel/blob/6.3.2/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java#L46 is likely the root cause.

Which operating system are you running Bazel on?

windows, macos

What is the output of bazel info release?

6.3.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@UebelAndre
Copy link
Contributor Author

A potential solution to the first two bullets may be

diff --git a/tools/def_parser/BUILD.tools b/tools/def_parser/BUILD.tools
index fb4f524cec..efa8c05572 100644
--- a/tools/def_parser/BUILD.tools
+++ b/tools/def_parser/BUILD.tools
@@ -11,7 +11,7 @@ filegroup(
 filegroup(
     name = "def_parser",
     srcs = select({
-        "//src/conditions:host_windows": ["def_parser_windows"],
+        "@platforms//os:windows": ["def_parser_windows"],
         "//conditions:default": [
             "no_op.bat",
         ],
diff --git a/tools/launcher/BUILD.tools b/tools/launcher/BUILD.tools
index 36465665bf..486b7940c1 100644
--- a/tools/launcher/BUILD.tools
+++ b/tools/launcher/BUILD.tools
@@ -11,7 +11,7 @@ filegroup(
 filegroup(
     name = "launcher",
     srcs = select({
-        "//src/conditions:host_windows": [":launcher_windows"],
+        "@platforms//os:windows": [":launcher_windows"],
         "//conditions:default": [
             "//src/tools/launcher:launcher",
         ],
diff --git a/tools/test/BUILD b/tools/test/BUILD
index 6f39dd99b2..6cc4ddf80a 100644
--- a/tools/test/BUILD
+++ b/tools/test/BUILD
@@ -116,14 +116,13 @@ filegroup(
         "extensions.bzl",
         "generate-xml.sh",
         "test-setup.sh",
+        "dummy.sh",
     ] + select({
         "@bazel_tools//src/conditions:windows": [
             "tw",
             "xml",
         ],
-        "//conditions:default": [
-            "dummy.sh",
-        ],
+        "//conditions:default": [],
     }),
     visibility = ["//tools:__pkg__"],
 )

@fmeum
Copy link
Collaborator

fmeum commented Sep 21, 2023

Cc @tjgq

@tjgq
Copy link
Contributor

tjgq commented Sep 21, 2023

The proposed changes to BUILD.tools files alone won't work, because some of the tools in @bazel_tools are precompiled binaries targeting the OS Bazel was compiled for. We need to also do one of the following:

  1. Fall back to compiling all of the embedded tools from source when host platform != execution platform (there might be a bootstrapping problem and/or a performance penalty here)
  2. Bundle precompiled binaries of the embedded tools for every OS (entails a prohibitive Bazel binary size cost)
  3. Fetch the appropriate precompiled binaries for the execution platform on demand (from a well-known per-platform repository) instead of getting them from @bazel_tools.

I suspect 3 is the only real option. See also #19209 where I describe a similar issue.

@iancha1992 iancha1992 added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Sep 21, 2023
@tjgq tjgq added the area-Windows Windows-specific issues and feature requests label Sep 21, 2023
@UebelAndre
Copy link
Contributor Author

3 seems like the best option to me.

@fmeum
Copy link
Collaborator

fmeum commented Sep 22, 2023

Bazel assumes if the client is windows that the java runtime must be bin/java.exe. This is wrong and the execution should instead be checked. https://github.com/bazelbuild/bazel/blob/6.3.2/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java#L46 is likely the root cause.

@UebelAndre It looks like this constant is only used for java_runtimes without the java attribute specified. I think that this should (and could) instead be fixed by setting the java attribute in rules_java's repository rules, which is more direct than guessing the extension based on the platform the runtime is executed on.

fmeum added a commit to fmeum/rules_java that referenced this issue Sep 22, 2023
If the `java` attribute on `java_runtime` isn't set, Bazel guesses it
based on the host platform, which can be different from the execution
platform.

https://github.com/bazelbuild/bazel/blob/04a05677e4714434ec196c45a4ccbd8e0ef2f0ff/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java#L46

Work towards bazelbuild/bazel#19587
fmeum added a commit to fmeum/rules_java that referenced this issue Sep 22, 2023
If the `java` attribute on `java_runtime` isn't set, Bazel guesses it
based on the host platform, which can be different from the execution
platform.

https://github.com/bazelbuild/bazel/blob/04a05677e4714434ec196c45a4ccbd8e0ef2f0ff/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java#L46

Work towards bazelbuild/bazel#19587
@UebelAndre
Copy link
Contributor Author

I also found an issue in the python rules I think stems from https://github.com/bazelbuild/bazel/blob/7.0.0-pre.20230917.3/src/main/starlark/builtins_bzl/common/python/py_executable.bzl#L193-L197

cc @rickeylev would you be able to point out what code is actually in use? This starlark? Something else in Java? rules_python? It'd be ideal if rules_python started using the starlark implementation of the python rules there so this could be fixed quickly and easily.

@rickeylev
Copy link
Contributor

The Starlark code in builtins_bzl is what is active in Bazel at head. There's a couple Java helpers (internal helpers and flags), but all the rule and provider code is in builtins_bzl Starlark.

fwiw, that particular line was directly ported from the Java implementation. And yes, it's problematic and fundamentally wrong, but I was just trying to get things ported over at the time :).

I just started moving the code into rules_python a few days ago, but it'll probably be a few weeks before it's functional, plus it requires Bazel at head. You can follow issue 1069 in rules_python for updates on that.

@UebelAndre
Copy link
Contributor Author

UebelAndre commented Sep 23, 2023

The Starlark code in builtins_bzl is what is active in Bazel at head. There's a couple Java helpers (internal helpers and flags), but all the rule and provider code is in builtins_bzl Starlark.

fwiw, that particular line was directly ported from the Java implementation. And yes, it's problematic and fundamentally wrong, but I was just trying to get things ported over at the time :).

I just started moving the code into rules_python a few days ago, but it'll probably be a few weeks before it's functional, plus it requires Bazel at head. You can follow issue 1069 in rules_python for updates on that.

To be clear, I was confused around what code was actually powering python rules, not criticizing the starlark port. I'm on Bazel 6.3.2 and don't think I'm even using the starlark port. I think that effort is going to be incredible when completed! That said though, it would be awesome to rip out any reliance on the Bazel client's OS so that cross-platform remote execution could be supported

Also You're saying bazelbuild/rules_python#1069 requires Bazel at HEAD (so v7+)? I would imagine if the macros a @rules_python//python:defs.bzl pointed to the starlark implementation then it would work with 6.3.2? Or are there parts of the implementation that are not released?

@rickeylev
Copy link
Contributor

I would imagine if the macros a @rules_python//python:defs.bzl pointed to the starlark implementation then it would work with 6.3.2?

Not as-is, no. There's various low-level helper apis it needs that are exposed in Bazel@head that aren't available in 6.3. There might be particular codepaths that 6.3 could weave through to work, but I'm honestly not sure. If you grep for py_internal in rules_python (or py_builtins in bazel), you'll see the various low-level apis and where and how they get used.

Or are there parts of the implementation that are not released?

It's all been released; mostly. All the code for the Python rules themselves is now in rules_python (e.g. bazelbuild/rules_python@3a57e4a). The "mostly" is because I'm finding some bugs as I get it working within rules_python (e.g. a couple low level apis got overlooked for exporting]).


In any case, lets continue discussion about the implementation being within rules_python instead of bazel to bazelbuild/rules_python#1069.

@joeleba joeleba added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Sep 26, 2023
fmeum added a commit to fmeum/rules_java that referenced this issue Oct 6, 2023
If the `java` attribute on `java_runtime` isn't set, Bazel guesses it
based on the host platform, which can be different from the execution
platform.

https://github.com/bazelbuild/bazel/blob/04a05677e4714434ec196c45a4ccbd8e0ef2f0ff/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java#L46

Work towards bazelbuild/bazel#19587
@UebelAndre
Copy link
Contributor Author

@tjgq do you know concretely what is required to move these compiled binaries out of bazel_tools and into external repositories that can be selected via @platforms constraints?

copybara-service bot pushed a commit that referenced this issue Dec 4, 2023
Changes from checking host platform to the target platform.

Work towards:

#20065

#19587

PiperOrigin-RevId: 587638137
Change-Id: Ib6cdcddd9f07bf28f474de9a356b0f94792605dc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants