From 305ca7dadd17bbbf215822e8e8e9c792c182cdaa Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 24 Sep 2026 17:07:21 +0200 Subject: [PATCH] Add missing headers to BUILD.bazel and check it in CI (#5554) * Add missing headers to BUILD.bazel and make its generator reproduce it The "json" cc_library did not list three headers that the library includes: - detail/meta/logic.hpp (added in #5016, included by from_json.hpp) - detail/input/number_parse.hpp (added in #5283, included by lexer.hpp) - detail/input/string_scan.hpp (added in #5283, included by lexer.hpp and serializer.hpp) Bazel's sandbox only exposes declared headers, so any target depending on @nlohmann_json//:json and including failed with "'nlohmann/detail/meta/logic.hpp' file not found". The file could not simply be regenerated, because the generator behind "make BUILD.bazel" was stale: it wrote only the "json" cc_library and dropped the load() statements, the license block, and the "singleheader-json" target that were added by hand in #4584. The generator now emits the complete file, so its output differs from the previous BUILD.bazel only by the three headers. It also resolves the glob against the project root instead of the working directory and sorts the list explicitly. "make BUILD.bazel" is now phony: in a fresh checkout, BUILD.bazel is not older than the headers, so make considered it up to date, and a removed header would never trigger a rebuild. "make check-amalgamation" also checks that BUILD.bazel is up to date. Signed-off-by: Niels Lohmann * Check in CI that BUILD.bazel is up to date The "Check amalgamation" workflow now also regenerates BUILD.bazel, so a pull request that adds, renames, or removes a header without updating the Bazel header list fails, and the attached amalgamation.patch contains the fix. The failure comment and the contribution guidelines mention the new check, and the comment now links to the existing "Amalgamate the source code" section instead of the "Files to change" anchor that was removed in #4560. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- .github/CONTRIBUTING.md | 9 ++++ .github/workflows/check_amalgamation.yml | 7 ++- .../workflows/comment_check_amalgamation.yml | 4 +- BUILD.bazel | 3 ++ FILES.md | 6 ++- Makefile | 12 +++-- cmake/scripts/gen_bazel_build_file.cmake | 44 ++++++++++++++++--- 7 files changed, 72 insertions(+), 13 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 68f82c474..4125b066a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -158,6 +158,15 @@ make amalgamate Running `make amalgamate` will also apply automatic formatting to the source files using [`Artistic Style`](https://astyle.sourceforge.net/). This formatting may modify your source files in-place. Be certain to review and commit any changes to avoid unintended formatting diffs in commits. +If you add, rename, or remove a header in `include/nlohmann`, also regenerate the header list in +[`BUILD.bazel`](https://github.com/nlohmann/json/blob/develop/BUILD.bazel) (requires CMake) by executing: + +```shell +make BUILD.bazel +``` + +The amalgamation check in CI fails if any of these generated files is out of date. + ## Recommended documentation - The library’s [README file](https://github.com/nlohmann/json/blob/master/README.md) is an excellent starting point to diff --git a/.github/workflows/check_amalgamation.yml b/.github/workflows/check_amalgamation.yml index f70ebfba0..f692e434a 100644 --- a/.github/workflows/check_amalgamation.yml +++ b/.github/workflows/check_amalgamation.yml @@ -57,13 +57,16 @@ jobs: python3 -mvenv venv venv/bin/pip3 install -r $MAIN_DIR/tools/astyle/requirements.txt - - name: Regenerate amalgamation and formatting + - name: Regenerate amalgamation, formatting, and BUILD.bazel run: | cd $MAIN_DIR python3 $TOOL_DIR/amalgamate.py -c $TOOL_DIR/config_json.json -s . python3 $TOOL_DIR/amalgamate.py -c $TOOL_DIR/config_json_fwd.json -s . + # the header list of the Bazel "json" target must match the files in include/ + cmake -P cmake/scripts/gen_bazel_build_file.cmake + ${{ github.workspace }}/venv/bin/astyle --project=tools/astyle/.astylerc --suffix=none --quiet \ $INCLUDE_DIR/json.hpp $INCLUDE_DIR/json_fwd.hpp @@ -87,7 +90,7 @@ jobs: mkdir -p ${{ github.workspace }}/patch git diff --patch --no-color > ${{ github.workspace }}/patch/amalgamation.patch if [ -s ${{ github.workspace }}/patch/amalgamation.patch ]; then - echo "The source code has not been amalgamated/formatted correctly. Diff:" + echo "The source code has not been amalgamated/formatted correctly or BUILD.bazel is out of date. Diff:" cat ${{ github.workspace }}/patch/amalgamation.patch echo "has_diff=true" >> "$GITHUB_OUTPUT" else diff --git a/.github/workflows/comment_check_amalgamation.yml b/.github/workflows/comment_check_amalgamation.yml index 788c1b8ce..4667329d2 100644 --- a/.github/workflows/comment_check_amalgamation.yml +++ b/.github/workflows/comment_check_amalgamation.yml @@ -95,13 +95,13 @@ jobs: issue_number: issue_number, owner: context.repo.owner, repo: context.repo.repo, - body: '## 🔴 Amalgamation check failed! 🔴\nThe source code has not been amalgamated and/or formatted correctly.' + body: '## 🔴 Amalgamation check failed! 🔴\nThe source code has not been amalgamated and/or formatted correctly, or `BUILD.bazel` is out of date.' + (hasPatch ? '\n\n📎 A ready-to-apply patch is attached to the [failed workflow run](' + runUrl + ') as the `amalgamation-patch` artifact.' + ' Download it, then apply it locally from the repository root with:' + '\n\n```shell\ngit apply amalgamation.patch\n```\n\n' + 'This does not require installing astyle yourself.' : '') + (first ? '\n\n@' + author + ' Please read and follow the [Contribution Guidelines]' - + '(https://github.com/nlohmann/json/blob/develop/.github/CONTRIBUTING.md#files-to-change).' + + '(https://github.com/nlohmann/json/blob/develop/.github/CONTRIBUTING.md#amalgamate-the-source-code).' : '') }) diff --git a/BUILD.bazel b/BUILD.bazel index de0ff7145..ea8ffae21 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -30,8 +30,10 @@ cc_library( "include/nlohmann/detail/input/input_adapters.hpp", "include/nlohmann/detail/input/json_sax.hpp", "include/nlohmann/detail/input/lexer.hpp", + "include/nlohmann/detail/input/number_parse.hpp", "include/nlohmann/detail/input/parser.hpp", "include/nlohmann/detail/input/position_t.hpp", + "include/nlohmann/detail/input/string_scan.hpp", "include/nlohmann/detail/iterators/internal_iterator.hpp", "include/nlohmann/detail/iterators/iter_impl.hpp", "include/nlohmann/detail/iterators/iteration_proxy.hpp", @@ -49,6 +51,7 @@ cc_library( "include/nlohmann/detail/meta/detected.hpp", "include/nlohmann/detail/meta/identity_tag.hpp", "include/nlohmann/detail/meta/is_sax.hpp", + "include/nlohmann/detail/meta/logic.hpp", "include/nlohmann/detail/meta/std_fs.hpp", "include/nlohmann/detail/meta/type_traits.hpp", "include/nlohmann/detail/meta/void_t.hpp", diff --git a/FILES.md b/FILES.md index b68167336..263647146 100644 --- a/FILES.md +++ b/FILES.md @@ -250,12 +250,16 @@ Further documentation: ### `BUILD.bazel` -The file can be updated by calling +The build definition for [Bazel](https://bazel.build). The file is generated by +`cmake/scripts/gen_bazel_build_file.cmake`, which derives the header list from the files in `include`; change the +script rather than editing the file by hand. The file can be updated by calling ```shell make BUILD.bazel ``` +The "Check amalgamation" workflow fails if the file is out of date. + ### `meson.build` The build definition for the [Meson](https://mesonbuild.com) build system. diff --git a/Makefile b/Makefile index e1a1d2b75..871ea7995 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: pretty clean ChangeLog.md release update_hedley update_hedley_undef +.PHONY: pretty clean ChangeLog.md release update_hedley update_hedley_undef BUILD.bazel ########################################################################## # configuration @@ -30,8 +30,9 @@ AMALGAMATED_FWD_FILE=single_include/nlohmann/json_fwd.hpp # main target all: @echo "amalgamate - amalgamate files single_include/nlohmann/json{,_fwd}.hpp from the include/nlohmann sources" + @echo "BUILD.bazel - regenerate the Bazel BUILD file from the include/nlohmann sources" @echo "ChangeLog.md - generate ChangeLog file" - @echo "check-amalgamation - check whether sources have been amalgamated" + @echo "check-amalgamation - check whether sources have been amalgamated and BUILD.bazel is up to date" @echo "clean - remove built files" @echo "doctest - compile example files and check their output" @echo "fuzz_testing - prepare fuzz testing of the JSON parser" @@ -172,8 +173,13 @@ check-amalgamation: @diff $(AMALGAMATED_FWD_FILE) $(AMALGAMATED_FWD_FILE)~ || (echo "===================================================================\n Amalgamation required! Please read the contribution guidelines\n in file .github/CONTRIBUTING.md.\n===================================================================" ; mv $(AMALGAMATED_FWD_FILE)~ $(AMALGAMATED_FWD_FILE) ; false) @mv $(AMALGAMATED_FILE)~ $(AMALGAMATED_FILE) @mv $(AMALGAMATED_FWD_FILE)~ $(AMALGAMATED_FWD_FILE) + @mv BUILD.bazel BUILD.bazel~ + @$(MAKE) BUILD.bazel + @diff BUILD.bazel BUILD.bazel~ || (echo "===================================================================\n BUILD.bazel is out of date! Please run 'make BUILD.bazel'.\n===================================================================" ; mv BUILD.bazel~ BUILD.bazel ; false) + @mv BUILD.bazel~ BUILD.bazel -BUILD.bazel: $(SRCS) +# generate the Bazel BUILD file; phony, because a removed header would not trigger a rebuild +BUILD.bazel: cmake -P cmake/scripts/gen_bazel_build_file.cmake ########################################################################## diff --git a/cmake/scripts/gen_bazel_build_file.cmake b/cmake/scripts/gen_bazel_build_file.cmake index e754d387d..3c7db9493 100644 --- a/cmake/scripts/gen_bazel_build_file.cmake +++ b/cmake/scripts/gen_bazel_build_file.cmake @@ -1,24 +1,58 @@ # generate Bazel BUILD file +# +# usage: cmake -P cmake/scripts/gen_bazel_build_file.cmake (or: make BUILD.bazel) +# +# The header list of the "json" target is derived from the files in include/. Everything else is fixed text below, +# so edit this script rather than BUILD.bazel. -set(PROJECT_ROOT "${CMAKE_CURRENT_LIST_DIR}/../..") +get_filename_component(PROJECT_ROOT "${CMAKE_CURRENT_LIST_DIR}/../.." ABSOLUTE) set(BUILD_FILE "${PROJECT_ROOT}/BUILD.bazel") -file(GLOB_RECURSE HEADERS LIST_DIRECTORIES false RELATIVE "${PROJECT_ROOT}" "include/*.hpp") +file(GLOB_RECURSE HEADERS LIST_DIRECTORIES false RELATIVE "${PROJECT_ROOT}" "${PROJECT_ROOT}/include/*.hpp") +list(SORT HEADERS) + +set(CONTENT [=[ +load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_license//rules:license.bzl", "license") + +package( + default_applicable_licenses = [":license"], +) + +exports_files([ + "LICENSE.MIT", +]) + +license( + name = "license", + license_kinds = ["@rules_license//licenses/spdx:MIT"], + license_text = "LICENSE.MIT", +) -file(WRITE "${BUILD_FILE}" [=[ cc_library( name = "json", hdrs = [ ]=]) foreach(header ${HEADERS}) - file(APPEND "${BUILD_FILE}" " \"${header}\",\n") + string(APPEND CONTENT " \"${header}\",\n") endforeach() -file(APPEND "${BUILD_FILE}" [=[ +string(APPEND CONTENT [=[ ], includes = ["include"], visibility = ["//visibility:public"], alwayslink = True, ) + +cc_library( + name = "singleheader-json", + hdrs = [ + "single_include/nlohmann/json.hpp", + ], + includes = ["single_include"], + visibility = ["//visibility:public"], +) ]=]) + +file(WRITE "${BUILD_FILE}" "${CONTENT}")