google / flatbuffers

FlatBuffers: Memory Efficient Serialization Library
https://flatbuffers.dev/
Apache License 2.0
22.52k stars 3.19k forks source link

Bazel build on windows fails with error from swift #8301

Closed peakschris closed 1 month ago

peakschris commented 1 month ago

I'm not sure if it's supposed to work, but the bazel build of latest on windows is broken:

git clone flatbuffers
cd flatbuffers
bazel build //...
INFO: Repository build_bazel_rules_swift_local_config instantiated at:
  D:/workdir/flatbuffers/WORKSPACE:53:25: in <toplevel>
  external/build_bazel_rules_swift/swift/repositories.bzl:114:11: in swift_rules_dependencies
  build_bazel_rules_swift/swift/repositories.bzl:32:18: in _maybe
Repository rule swift_autoconfiguration defined at:
  external/build_bazel_rules_swift/swift/internal/swift_autoconfiguration.bzl:302:42: in <toplevel>
ERROR: An error occurred during the fetch of repository 'build_bazel_rules_swift_local_config':
   Traceback (most recent call last):
        File "external/build_bazel_rules_swift/swift/internal/swift_autoconfiguration.bzl", line 300, column 32, in _swift_autoconfiguration_impl
                _create_linux_toolchain(repository_ctx)
        File "external/build_bazel_rules_swift/swift/internal/swift_autoconfiguration.bzl", line 212, column 13, in _create_linux_toolchain
                fail("ERROR: rules_swift uses Bazel's CROSSTOOL to link, but Swift " +
Error in fail: ERROR: rules_swift uses Bazel's CROSSTOOL to link, but Swift requires that the driver used is clang. Please set `CC=clang` in your environment before invoking Bazel.

We have a requirement to compile the flatbuffers library with msvc, so switching to clang is a non-starter for us. Is swift a real prerequisite for C++?

mustiikhalil commented 1 month ago

Hej, thanks for opening the issue.

No, Swift isn't a prerequisite to building with Bazel. I would say it's mainly for macOS and Linux.

Unfortunately, my knowledge with Bazel is none-existing to solve this actually.

peakschris commented 1 month ago

This can be fixed with these two changes:

build_grpc_with_cxx14.patch:

diff --git a/bazel/copts.bzl b/bazel/copts.bzl
index 10be944f25..879518b92f 100644
--- a/bazel/copts.bzl
+++ b/bazel/copts.bzl
@@ -59,4 +59,7 @@ GRPC_LLVM_WARNING_FLAGS = [
 GRPC_DEFAULT_COPTS = select({
     "//:use_strict_warning": GRPC_LLVM_WARNING_FLAGS + ["-DUSE_STRICT_WARNING=1"],
     "//conditions:default": [],
-})
+}) + select({
+        "@bazel_tools//src/conditions:windows": ["/std:c++14"],
+        "//conditions:default": ["-std=c++14"],
+})

.bazelrc:

# As of bazel 6.3.0, common supports all options
common --deleted_packages=tests/bazel_repository_test_dir,tests/ts/bazel_repository_test_dir
# Workaround https://github.com/bazelbuild/bazel/issues/21712 until bazel 7.2.0 is out
common --experimental_worker_for_repo_fetching=off
# Point tools such as coursier (used in rules_jvm_external) to Bazel's internal JDK
# suggested in https://github.com/bazelbuild/rules_jvm_external/issues/445
common --repo_env=JAVA_HOME=../bazel_tools/jdk
common --action_env=JAVA_HOME=../bazel_tools/jdk
# Workaround "Error: need --enable_runfiles on Windows for to support rules_js"
common:windows --enable_runfiles
# Swift is not required on Windows
common:windows --deleted_packages=swift

The windows build command is then: bazel build //... --config=windows

peakschris commented 1 month ago

Would you be able to upstream this?

mustiikhalil commented 1 month ago

Well, not anytime this week, but I can look at it on Monday. Would that be fine?

cc: @dbaileychess

peakschris commented 1 month ago

Amazing! Thanks :-)

I'm also looking at whether we can add the bazel extensions to the conan package, so people using bazel can consume flatbuffers from conan. I'll followup on that one separately if successful.

mustiikhalil commented 1 month ago

@peakschris I wonder what would this code below fix:

diff --git a/bazel/copts.bzl b/bazel/copts.bzl
index 10be944f25..879518b92f 100644
--- a/bazel/copts.bzl
+++ b/bazel/copts.bzl
@@ -59,4 +59,7 @@ GRPC_LLVM_WARNING_FLAGS = [
 GRPC_DEFAULT_COPTS = select({
     "//:use_strict_warning": GRPC_LLVM_WARNING_FLAGS + ["-DUSE_STRICT_WARNING=1"],
     "//conditions:default": [],
-})
+}) + select({
+        "@bazel_tools//src/conditions:windows": ["/std:c++14"],
+        "//conditions:default": ["-std=c++14"],
+})

Since I know we are having an issue building with Xcode 15.0 as well with gRPC that was fixed by limiting Xcode to 14.2 which isn't optimal commit. CI issue https://buildkite.com/bazel/flatbuffers/builds/9316#018e2ff5-e5ca-46e5-afc1-74d96b161517

peakschris commented 1 month ago

All I've changed in that patch file is to use /std:c++14 on windows, whilst the default option used is -std=c++14.

Original patch file:

+}) + ["-std=c++14"]

Modified patch file:

+}) + select({
+        "@bazel_tools//src/conditions:windows": ["/std:c++14"],
+        "//conditions:default": ["-std=c++14"],
+})

It looks like a different issue to the Xcode one.