From b307a175ed8e2c343b49eea554984d39e779f2b1 Mon Sep 17 00:00:00 2001 From: Arthur Sonzogni Date: Wed, 7 May 2025 22:41:17 +0200 Subject: [PATCH] Bazel: general improvements. (#1043) * Bazel: general improvements. Improve the Bazel build. Attempt to fix previous errors recorded while trying to publish ftxui in the Bazel Central Registry: - https://github.com/bazelbuild/bazel-central-registry/pull/4485 - https://buildkite.com/bazel/bcr-presubmit/builds/13601#01968b61-f5b2-4d16-94d0-c87a03a1a23b Test against "recent" platforms ------------------------------- Previously, I got the error: ``` gcc: error: unrecognized command line option '-std-c++20'; did you mean '-std-c++2a'? ``` This was due to using old distribution like ubuntu 2004. Test against newer platforms only to avoid GCC version<-9.x.y Downgrade gtest version. ------------------------ I suspect this caused the Bazel Central Registry error: ``` file:///workdir/modules/googletest/1.15.2/MODULE.bazel:68:20: name 'use_repo_rule' is not defined ``` Specifying using bazelmod fixes the issue. Thanks @robinlinden Tag gtest as dev_dependency --------------------------- Presumably, this should avoid dependants to fetch it? Enable --features-layering_check -------------------------------- Aka clang `-Wprivate-header`. Fix the encountered errors. Use clang in the CI ------------------- The CI was defining clang/gcc in the matrix, but was not using it. Fix the bug. --- .bazelrc | 7 +++++ .bcr/presubmit.yml | 43 ++++++++++++++++++--------- .github/workflows/build.yaml | 25 +++++++++++----- BUILD.bazel | 57 ++++++++++++++++++++++++------------ MODULE.bazel | 16 ++++++---- bazel/ftxui.bzl | 41 ++++++++++---------------- tools/test_bazel.sh | 18 ++++++++++++ 7 files changed, 135 insertions(+), 72 deletions(-) create mode 100644 .bazelrc create mode 100755 tools/test_bazel.sh diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 00000000..bd86ad56 --- /dev/null +++ b/.bazelrc @@ -0,0 +1,7 @@ +build --features=layering_check +build --enable_bzlmod + +build --enable_platform_specific_config +build:linux --cxxopt=-std=c++20 +build:macos --cxxopt=-std=c++20 +build:windows --cxxopt=-std:c++20 diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index 632e975d..6e655431 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -1,21 +1,36 @@ matrix: - platform: - - centos7 - - debian10 - - ubuntu2004 - - macos - - windows - bazel: [6.x, 7.x, 8.x] + bazel: + - 7.x + - 8.x + - rolling + unix_platform: + - debian11 + - ubuntu2204 + - macos + - macos_arm64 + win_platform: + - windows + tasks: - verify_targets: - name: Build and test. - platform: ${{ platform }} + + unix_test: + name: Verify build targets on Unix + platform: ${{ unix_platform }} bazel: ${{ bazel }} + build_flags: + - --cxxopt=-std=c++20 build_targets: - - '@ftxui//:ftxui' - - '@ftxui//:screen' - '@ftxui//:dom' - '@ftxui//:component' - test_targets: - - '@ftxui//:tests' + - '@ftxui//:screen' + windows_test: + name: Verify build targets + platform: ${{ win_platform }} + bazel: ${{ bazel }} + build_flags: + - --cxxopt=/std:c++20 + build_targets: + - '@ftxui//:dom' + - '@ftxui//:component' + - '@ftxui//:screen' diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cdecf41d..accc4091 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -14,25 +14,30 @@ on: jobs: test_bazel: - name: "Bazel, ${{ matrix.compiler }}, ${{ matrix.os }}" + name: "Bazel, ${{ matrix.cxx }}, ${{ matrix.os }}" strategy: fail-fast: false matrix: include: - os: ubuntu-latest - compiler: gcc + cxx: g++ + cc: gcc - os: ubuntu-latest - compiler: llvm + cxx: clang++ + cc: clang - os: macos-latest - compiler: llvm + cxx: g++ + cc: gcc - os: macos-latest - compiler: gcc + cxx: clang++ + cc: clang - os: windows-latest - compiler: cl + cxx: cl + cc: cl runs-on: ${{ matrix.os }} steps: @@ -40,10 +45,16 @@ jobs: uses: actions/checkout@v3 - name: "Build with Bazel" + env: + CC: ${{ matrix.cc }} + CXX: ${{ matrix.cxx }} run: bazel build ... - name: "Tests with Bazel" - run: bazel run tests + env: + CC: ${{ matrix.cc }} + CXX: ${{ matrix.cxx }} + run: bazel test --test_output=all ... test_cmake: name: "CMake, ${{ matrix.compiler }}, ${{ matrix.os }}" diff --git a/BUILD.bazel b/BUILD.bazel index 94bd4389..5dfb8c13 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -12,23 +12,19 @@ load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") load(":bazel/ftxui.bzl", "ftxui_cc_library") load(":bazel/ftxui.bzl", "generate_examples") -load(":bazel/ftxui.bzl", "cpp20") load(":bazel/ftxui.bzl", "windows_copts") load(":bazel/ftxui.bzl", "pthread_linkopts") -package(default_visibility = ["//visibility:public"]) +# A meta target depending on all of the ftxui submodules. +# Note that component depends on dom and screen, so ftxui is just an alias for +# component. +# ┌component──┐ +# │┌dom──────┐│ +# ││┌screen─┐││ +# └┴┴───────┴┴┘ +alias(name = "ftxui", actual = ":component") -# A meta target that depends on all the ftxui sub modules. -alias( - name = "ftxui", - # Note that :component depends on :dom, which depends on :screen. Bazel - # doesn't really support "public" and "private" dependencies. They are all - # public. This is equivalent to depending on all the submodules. - actual = ":component", - visibility = ["//visibility:public"], -) - -# ftxui:screen is a module that provides a screen buffer and color management +# @ftxui:screen is a module that provides a screen buffer and color management # for terminal applications. A screen is a 2D array of cells, each cell can # contain a glyph, a color, and other attributes. The library also provides # functions to manipulate the screen. @@ -60,7 +56,7 @@ ftxui_cc_library( ], ) -# ftxui:dom is a library that provides a way to create and manipulate a +# @ftxui:dom is a library that provides a way to create and manipulate a # "document" that can be rendered to a screen. The document is a tree of nodes. # Nodes can be text, layouts, or various decorators. Users needs to compose # nodes to create a document. A document is responsive to the size of the @@ -130,7 +126,7 @@ ftxui_cc_library( deps = [":screen"], ) -# ftxui:component is a library to create "dynamic" component renderering and +# @ftxui:component is a library to create "dynamic" component renderering and # updating a ftxui::dom document on the screen. It is a higher level API than # ftxui:dom. # @@ -167,6 +163,13 @@ ftxui_cc_library( "src/ftxui/component/terminal_input_parser.hpp", "src/ftxui/component/util.cpp", "src/ftxui/component/window.cpp", + + # Private header from ftxui:dom. + "src/ftxui/dom/node_decorator.hpp", + + # Private header from ftxui:screen. + "src/ftxui/screen/string_internal.hpp", + "src/ftxui/screen/util.hpp", ], hdrs = [ "include/ftxui/component/animation.hpp", @@ -182,7 +185,10 @@ ftxui_cc_library( "include/ftxui/component/task.hpp", ], linkopts = pthread_linkopts(), - deps = [":dom"], + deps = [ + ":dom", + ":screen", + ], ) # FTXUI's tests @@ -205,7 +211,6 @@ cc_test( "src/ftxui/component/resizable_split_test.cpp", "src/ftxui/component/slider_test.cpp", "src/ftxui/component/terminal_input_parser_test.cpp", - "src/ftxui/component/terminal_input_parser_test_fuzzer.cpp", "src/ftxui/component/toggle_test.cpp", "src/ftxui/dom/blink_test.cpp", "src/ftxui/dom/bold_test.cpp", @@ -233,6 +238,17 @@ cc_test( "src/ftxui/screen/string_test.cpp", "src/ftxui/util/ref_test.cpp", + # Private header from ftxui:screen for string_test.cpp. + "src/ftxui/screen/string_internal.hpp", + + # Private header from ftxui::component for + # terminal_input_parser_test.cpp. + "src/ftxui/component/terminal_input_parser.hpp", + + # Private header from ftxui::dom for + # flexbox_helper_test.cpp. + "src/ftxui/dom/flexbox_helper.hpp", + # TODO: Enable the two tests timing out with Bazel: # - "src/ftxui/component/screen_interactive_test.cpp", # - "src/ftxui/dom/selection_test.cpp", @@ -241,9 +257,12 @@ cc_test( "include", "src", ], - copts = cpp20() + windows_copts(), + copts = windows_copts(), deps = [ - "//:ftxui", + ":screen", + ":dom", + ":component", + "@googletest//:gtest", "@googletest//:gtest_main", ], ) diff --git a/MODULE.bazel b/MODULE.bazel index e625075b..b00e258c 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -1,9 +1,13 @@ -# FTXUI Module. -module(name = "ftxui", version = "6.1.8", compatibility_level = 6) +# FTXUI module. +module( + name = "ftxui", + version = "6.1.8", + compatibility_level = 6, +) -# Build deps. +# Build dependencies. bazel_dep(name = "rules_cc", version = "0.1.1") -bazel_dep(name = "platforms", version = "0.0.11") +bazel_dep(name = "platforms", version = "0.0.10") -# Test deps. -bazel_dep(name = "googletest", version = "1.16.0.bcr.1") +# Test dependencies. +bazel_dep(name = "googletest", version = "1.14.0.bcr.1", dev_dependency = True) diff --git a/bazel/ftxui.bzl b/bazel/ftxui.bzl index c96cf5b6..e0de2e81 100644 --- a/bazel/ftxui.bzl +++ b/bazel/ftxui.bzl @@ -1,24 +1,7 @@ # ftxui_common.bzl load("@rules_cc//cc:defs.bzl", "cc_library") - -def cpp17(): - return select({ - "@rules_cc//cc/compiler:msvc-cl": ["/std:c++17"], - "@rules_cc//cc/compiler:clang-cl": ["/std:c++17"], - "@rules_cc//cc/compiler:clang": ["-std=c++17"], - "@rules_cc//cc/compiler:gcc": ["-std=c++17"], - "//conditions:default": ["-std=c++17"], - }) - -def cpp20(): - return select({ - "@rules_cc//cc/compiler:msvc-cl": ["/std:c++20"], - "@rules_cc//cc/compiler:clang-cl": ["/std:c++20"], - "@rules_cc//cc/compiler:clang": ["-std=c++20"], - "@rules_cc//cc/compiler:gcc": ["-std=c++20"], - "//conditions:default": ["-std=c++20"], - }) +load("@rules_cc//cc:defs.bzl", "cc_binary") # Microsoft terminal is a bit buggy ¯\_(ツ)_/¯ and MSVC uses bad defaults. def windows_copts(): @@ -45,7 +28,8 @@ def windows_copts(): # This # - Replace missing font symbols by others. # - Reduce screen position pooling frequency to deals against a Microsoft - # race condition. This was fixed in 2020, but clients never not updated. + # race condition. This was fixed in 2020, but clients are still using + # old versions. # - https://github.com/microsoft/terminal/pull/7583 # - https://github.com/ArthurSonzogni/FTXUI/issues/136 "-DFTXUI_MICROSOFT_TERMINAL_FALLBACK", @@ -71,8 +55,8 @@ def pthread_linkopts(): def ftxui_cc_library( name, - srcs, - hdrs, + srcs = [], + hdrs = [], linkopts = [], deps = []): @@ -88,12 +72,13 @@ def ftxui_cc_library( "include", "src", ], - copts = cpp17() + windows_copts(), + copts = windows_copts(), visibility = ["//visibility:public"], ) # Compile all the examples in the examples/ directory. -# This is useful to check the Bazel is synchronized with CMake definitions. +# This is useful to check the Bazel is always synchronized against CMake +# definitions. def generate_examples(): cpp_files = native.glob(["examples/**/*.cpp"]) @@ -107,9 +92,13 @@ def generate_examples(): # Turn "examples/component/button.cpp" → "example_component_button" name = src.replace("/", "_").replace(".cpp", "") - native.cc_binary( + cc_binary( name = name, srcs = [src], - deps = ["//:component"], - copts = cpp20() + windows_copts(), + deps = [ + ":component", + ":dom", + ":screen", + ], + copts = windows_copts(), ) diff --git a/tools/test_bazel.sh b/tools/test_bazel.sh new file mode 100755 index 00000000..67d12cdb --- /dev/null +++ b/tools/test_bazel.sh @@ -0,0 +1,18 @@ +# This script tests the project with different versions of Bazel and compilers +# locally. This avoids waiting on the CI to run the tests. + +for ver in \ + "6.0.0" \ + "7.0.0" \ + "8.0.0" +do + for cc in \ + "gcc" \ + "clang" + do + echo "=== Testing with Bazel ${ver} with ${cc} ===" + USE_BAZEL_VERSION=${ver} CC=${cc} bazel clean --expunge + USE_BAZEL_VERSION=${ver} CC=${cc} bazel build //... + USE_BAZEL_VERSION=${ver} CC=${cc} bazel test //... + done + done