From ef7dd8ed06ca0363ecaef28afe5ee328786dcd3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Mon, 20 Apr 2026 19:58:00 +0200 Subject: [PATCH 1/5] Switch inject-browser-sdk to submodule; consolidate deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using FetchContent meant cmake had to clone inject-browser-sdk (a private repo) at configure time, which broke the devcontainer workflow because the container has no GitHub credentials. Converting it to a git submodule reuses the existing mechanism the repo already relies on for dd-trace-cpp and nginx-datadog — CI initializes it via GIT_SUBMODULE_STRATEGY: recursive, and local dev gets a checked-out tree with no extra auth. The submodule is pinned at 49245e1c6ed8, the previous FetchContent GIT_TAG. Since deps/CMakeLists.txt already handles dd-trace-cpp and is now handling inject-browser-sdk too, the top-level CMakeLists.txt's include(cmake/deps/fmt.cmake) and the HTTPD_DATADOG_ENABLE_RUM block move into deps/CMakeLists.txt so all third-party wiring lives in one place. fmt.cmake itself is simplified to FetchContent_MakeAvailable with EXCLUDE_FROM_ALL in Declare: both features require CMake 3.14 and 3.28 respectively, and dd-trace-cpp already forces 3.28 as the effective minimum, so the old manual FetchContent_GetProperties + FetchContent_Populate + add_subdirectory dance is no longer needed. --- .gitmodules | 3 +++ CMakeLists.txt | 17 ----------------- cmake/deps/fmt.cmake | 14 ++++---------- deps/CMakeLists.txt | 16 ++++++++++++++++ deps/inject-browser-sdk | 1 + 5 files changed, 24 insertions(+), 27 deletions(-) create mode 160000 deps/inject-browser-sdk diff --git a/.gitmodules b/.gitmodules index d88c1b83..52a8a84d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -5,3 +5,6 @@ path = deps/nginx-datadog url = ../httpd-datadog.git branch = mirror-nginx-datadog +[submodule "deps/inject-browser-sdk"] + path = deps/inject-browser-sdk + url = ../inject-browser-sdk.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 7f0afadc..59ef4665 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,23 +23,6 @@ if (NOT HTTPD_SRC_DIR) endif () include(cmake/utils.cmake) -include(cmake/deps/fmt.cmake) - -if (HTTPD_DATADOG_ENABLE_RUM) - include(FetchContent) - - set(INJECT_BROWSER_SDK_GIT_REF "49245e1c6ed8275990bb910125187e0aaad09e3d" CACHE STRING - "Git tag/branch for inject-browser-sdk") - - FetchContent_Declare( - InjectBrowserSDK - GIT_REPOSITORY https://github.com/DataDog/inject-browser-sdk.git - GIT_TAG ${INJECT_BROWSER_SDK_GIT_REF} - ) - FetchContent_MakeAvailable(InjectBrowserSDK) - - include(cmake/deps/rapidjson.cmake) -endif () add_library(httpd INTERFACE) target_include_directories( diff --git a/cmake/deps/fmt.cmake b/cmake/deps/fmt.cmake index 829b9aa1..6563674c 100644 --- a/cmake/deps/fmt.cmake +++ b/cmake/deps/fmt.cmake @@ -4,15 +4,9 @@ FetchContent_Declare( fmt GIT_REPOSITORY https://github.com/fmtlib/fmt.git GIT_TAG 10.2.1 + EXCLUDE_FROM_ALL ) -FetchContent_GetProperties(fmt) -if(NOT fmt_POPULATED) - FetchContent_Populate(fmt) - - set(BUILD_SHARED_LIBS OFF) - set(CMAKE_POSITION_INDEPENDENT_CODE ON) - - add_subdirectory(${fmt_SOURCE_DIR} ${fmt_BINARY_DIR} EXCLUDE_FROM_ALL) -endif() - +set(BUILD_SHARED_LIBS OFF) +set(CMAKE_POSITION_INDEPENDENT_CODE ON) +FetchContent_MakeAvailable(fmt) diff --git a/deps/CMakeLists.txt b/deps/CMakeLists.txt index 1181b4dc..42a97c3e 100644 --- a/deps/CMakeLists.txt +++ b/deps/CMakeLists.txt @@ -1,6 +1,22 @@ +include(${CMAKE_SOURCE_DIR}/cmake/deps/fmt.cmake) + add_subdirectory(dd-trace-cpp) # Force POSITION_INDEPENDENT_CODE (PIC) because mod_datadog.so is loaded via # dlopen, but dd-trace-cpp sets this based on BUILD_SHARED_LIBS, whereas we # link with the static library. set_target_properties(dd-trace-cpp-objects PROPERTIES POSITION_INDEPENDENT_CODE ON) + +if (HTTPD_DATADOG_ENABLE_RUM) + if (NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/inject-browser-sdk/CMakeLists.txt) + message(FATAL_ERROR + "deps/inject-browser-sdk submodule is not initialized. " + "Run: git submodule update --init --recursive") + endif () + # mod_datadog/CMakeLists.txt references ${injectbrowsersdk_SOURCE_DIR}, which + # FetchContent used to set; replicate it for the submodule layout. + set(injectbrowsersdk_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/inject-browser-sdk" CACHE INTERNAL "") + add_subdirectory(inject-browser-sdk EXCLUDE_FROM_ALL) + + include(${CMAKE_SOURCE_DIR}/cmake/deps/rapidjson.cmake) +endif () diff --git a/deps/inject-browser-sdk b/deps/inject-browser-sdk new file mode 160000 index 00000000..49245e1c --- /dev/null +++ b/deps/inject-browser-sdk @@ -0,0 +1 @@ +Subproject commit 49245e1c6ed8275990bb910125187e0aaad09e3d From 264167553e0c02f5781dfebcefc2fe817c1b8638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 21 Apr 2026 16:40:30 +0200 Subject: [PATCH 2/5] Address Copilot review on deps consolidation - cmake/deps/fmt.cmake: drop the FetchContent_MakeAvailable refactor and keep main's Populate + add_subdirectory pattern. The refactor used EXCLUDE_FROM_ALL as a FetchContent_Declare keyword, which requires CMake 3.28+, but the project advertises 3.12+ in both CMakeLists.txt and CONTRIBUTING.md. This revert keeps fmt.cmake identical to main and drops an ancillary change that wasn't load-bearing for the submodule work. - deps/CMakeLists.txt: quote the path in the EXISTS check so it survives spaces/semicolons (Windows paths, etc.). --- cmake/deps/fmt.cmake | 13 +++++++++---- deps/CMakeLists.txt | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cmake/deps/fmt.cmake b/cmake/deps/fmt.cmake index 6563674c..38318b96 100644 --- a/cmake/deps/fmt.cmake +++ b/cmake/deps/fmt.cmake @@ -4,9 +4,14 @@ FetchContent_Declare( fmt GIT_REPOSITORY https://github.com/fmtlib/fmt.git GIT_TAG 10.2.1 - EXCLUDE_FROM_ALL ) -set(BUILD_SHARED_LIBS OFF) -set(CMAKE_POSITION_INDEPENDENT_CODE ON) -FetchContent_MakeAvailable(fmt) +FetchContent_GetProperties(fmt) +if(NOT fmt_POPULATED) + FetchContent_Populate(fmt) + + set(BUILD_SHARED_LIBS OFF) + set(CMAKE_POSITION_INDEPENDENT_CODE ON) + + add_subdirectory(${fmt_SOURCE_DIR} ${fmt_BINARY_DIR} EXCLUDE_FROM_ALL) +endif() diff --git a/deps/CMakeLists.txt b/deps/CMakeLists.txt index 42a97c3e..da207da6 100644 --- a/deps/CMakeLists.txt +++ b/deps/CMakeLists.txt @@ -8,7 +8,7 @@ add_subdirectory(dd-trace-cpp) set_target_properties(dd-trace-cpp-objects PROPERTIES POSITION_INDEPENDENT_CODE ON) if (HTTPD_DATADOG_ENABLE_RUM) - if (NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/inject-browser-sdk/CMakeLists.txt) + if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/inject-browser-sdk/CMakeLists.txt") message(FATAL_ERROR "deps/inject-browser-sdk submodule is not initialized. " "Run: git submodule update --init --recursive") From 752bcc6f5e818289955379292f037c07ea3b1ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 21 Apr 2026 19:32:57 +0200 Subject: [PATCH 3/5] ci: init only required submodules in GHA workflows checkout with submodules: true tried to clone inject-browser-sdk, which is a private repo that the default GITHUB_TOKEN can't access, so the build job failed at checkout before cmake ever ran. None of the GitHub Actions presets (ci-dev, ci-release) enable HTTPD_DATADOG_ENABLE_RUM, so inject-browser-sdk is never needed in these workflows. Drop submodules: true and explicitly init the two submodules the build actually consumes: deps/dd-trace-cpp and deps/nginx-datadog. --- .github/workflows/dev.yml | 4 ++-- .github/workflows/release.yml | 4 ++-- .github/workflows/system-tests.yml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 37b5071f..8fcd549e 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -23,10 +23,10 @@ jobs: image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - submodules: true - name: Add cloned repo as safe run: sh -c "git config --global --add safe.directory $PWD" + - name: Init required submodules + run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - name: Configure run: cmake --preset=ci-dev -B build . - name: Build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0d583261..7a7539b1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,10 +9,10 @@ jobs: image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - submodules: true - name: Add cloned repo as safe run: sh -c "git config --global --add safe.directory $PWD" + - name: Init required submodules + run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - name: Configure run: cmake --preset ci-release -B build . - name: Build diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index b9b00519..72414eec 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -23,10 +23,10 @@ jobs: image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - submodules: true - name: Add cloned repo as safe run: sh -c "git config --global --add safe.directory $PWD" + - name: Init required submodules + run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - name: Configure run: cmake --preset=ci-dev -B build . - name: Build From 77a26c57d7f55927065111c57fc599b5f6fa4aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 21 Apr 2026 19:56:07 +0200 Subject: [PATCH 4/5] docs: note private inject-browser-sdk submodule in contributing guide Without access to DataDog/inject-browser-sdk, --init --recursive fails at clone time. Document the two-submodule command that covers standard builds and mention --recursive as the RUM-build opt-in. --- CONTRIBUTING.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0794211f..63d4b270 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,12 +2,14 @@ ## Fork, Clone, Branch and Create your PR -When cloning the repo, you have to initialize the submodules: +When cloning the repo, initialize the submodules you need. For a standard build: ```sh -git submodule update --init --recursive +git submodule update --init deps/dd-trace-cpp deps/nginx-datadog ``` +The `deps/inject-browser-sdk` submodule is a private repo and is only required when building with `-DHTTPD_DATADOG_ENABLE_RUM=ON`. If you have access, use `--recursive` instead to pull it as well. + ### Rules - Follow the pattern of what you already see in the code. - Follow the coding style. From 1330904109305b6ec8d09d83d9a9dab9cc35d091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Tue, 21 Apr 2026 20:26:54 +0200 Subject: [PATCH 5/5] Simplify fmt.cmake to FetchContent_MakeAvailable Matches the PR description: dd-trace-cpp already forces CMake 3.28 as the effective minimum, so the manual FetchContent_GetProperties + FetchContent_Populate + add_subdirectory dance is no longer needed. EXCLUDE_FROM_ALL moves onto the Declare() call where MakeAvailable picks it up in 3.28+. --- cmake/deps/fmt.cmake | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/cmake/deps/fmt.cmake b/cmake/deps/fmt.cmake index 38318b96..6563674c 100644 --- a/cmake/deps/fmt.cmake +++ b/cmake/deps/fmt.cmake @@ -4,14 +4,9 @@ FetchContent_Declare( fmt GIT_REPOSITORY https://github.com/fmtlib/fmt.git GIT_TAG 10.2.1 + EXCLUDE_FROM_ALL ) -FetchContent_GetProperties(fmt) -if(NOT fmt_POPULATED) - FetchContent_Populate(fmt) - - set(BUILD_SHARED_LIBS OFF) - set(CMAKE_POSITION_INDEPENDENT_CODE ON) - - add_subdirectory(${fmt_SOURCE_DIR} ${fmt_BINARY_DIR} EXCLUDE_FROM_ALL) -endif() +set(BUILD_SHARED_LIBS OFF) +set(CMAKE_POSITION_INDEPENDENT_CODE ON) +FetchContent_MakeAvailable(fmt)