From 773e5eb7d2d78d350b80265da1796068112d56cb Mon Sep 17 00:00:00 2001 From: ArthurSonzogni Date: Mon, 5 May 2025 00:48:53 +0200 Subject: [PATCH] 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 ``` I hoped using a lower version will fix the issue. 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 | 1 + .bcr/presubmit.yml | 13 ++++++--- .github/workflows/build.yaml | 14 ++++++---- BUILD.bazel | 54 ++++++++++++++++++++++++------------ MODULE.bazel | 16 +++++++---- bazel/ftxui.bzl | 19 +++++++++---- tools/test_bazel.sh | 27 ++++++++++++++++++ 7 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 .bazelrc create mode 100755 tools/test_bazel.sh diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 00000000..e2f73a47 --- /dev/null +++ b/.bazelrc @@ -0,0 +1 @@ +build --features=layering_check diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index 632e975d..6ffc6119 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -1,11 +1,16 @@ matrix: + # Note that gcc 9 and earlier aren't supported due to using --std=c++2a + # instead of --std=c++20 platform: - - centos7 - - debian10 - - ubuntu2004 + - debian11 + - ubuntu2024 - macos + - macos-arm64 - windows - bazel: [6.x, 7.x, 8.x] + bazel: + #- 6.x -> Build fails with bazel 6.x + - 7.x + - 8.x tasks: verify_targets: name: Build and test. diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cdecf41d..03e21826 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -20,16 +20,16 @@ jobs: matrix: include: - os: ubuntu-latest - compiler: gcc + compiler: g++ - os: ubuntu-latest - compiler: llvm + compiler: clang++ - os: macos-latest - compiler: llvm + compiler: g++ - os: macos-latest - compiler: gcc + compiler: clang++ - os: windows-latest compiler: cl @@ -40,10 +40,14 @@ jobs: uses: actions/checkout@v3 - name: "Build with Bazel" + env: + CC: ${{ matrix.compiler }} run: bazel build ... - name: "Tests with Bazel" - run: bazel run tests + env: + CC: ${{ matrix.compiler }} + run: bazel test --test_output=all ... test_cmake: name: "CMake, ${{ matrix.compiler }}, ${{ matrix.os }}" diff --git a/BUILD.bazel b/BUILD.bazel index 94bd4389..feb25b7e 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -16,19 +16,16 @@ 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 +57,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 +127,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 +164,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 +186,10 @@ ftxui_cc_library( "include/ftxui/component/task.hpp", ], linkopts = pthread_linkopts(), - deps = [":dom"], + deps = [ + ":dom", + ":screen", + ], ) # FTXUI's tests @@ -205,7 +212,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 +239,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", @@ -243,7 +260,10 @@ cc_test( ], copts = cpp20() + 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..099ba57c 100644 --- a/bazel/ftxui.bzl +++ b/bazel/ftxui.bzl @@ -1,6 +1,7 @@ # ftxui_common.bzl load("@rules_cc//cc:defs.bzl", "cc_library") +load("@rules_cc//cc:defs.bzl", "cc_binary") def cpp17(): return select({ @@ -45,7 +46,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 +73,8 @@ def pthread_linkopts(): def ftxui_cc_library( name, - srcs, - hdrs, + srcs = [], + hdrs = [], linkopts = [], deps = []): @@ -93,7 +95,8 @@ def ftxui_cc_library( ) # 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 +110,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"], + deps = [ + ":component", + ":dom", + ":screen", + ], copts = cpp20() + windows_copts(), ) diff --git a/tools/test_bazel.sh b/tools/test_bazel.sh new file mode 100755 index 00000000..2138cf53 --- /dev/null +++ b/tools/test_bazel.sh @@ -0,0 +1,27 @@ +# This script tests the project with different versions of Bazel and compilers +# locally. This avoids waiting on the CI to run the tests. + +# Version +# ------- +# - Version >= 7 are supported +# - Version <= 6 fail with the error: +# ERROR: Analysis of target '//:component' failed; build aborted: no such +# package '@rules_cc//cc/compiler': BUILD file not found in directory +# 'cc/compiler' of external repository @rules_cc. Add a BUILD file to a +# directory to mark it as a package. +# + +for ver in \ + "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