Skip to content

Migrate public headers to include/sbgraph and enforce <sbgraph/...> consumption - #143

Merged
Kalashnikovni merged 7 commits into
iss-135-define-include-directoryfrom
copilot/iss-135-define-include-directory
Sep 21, 2026
Merged

Kalashnikovni merged 7 commits into
iss-135-define-include-directoryfrom
copilot/iss-135-define-include-directory

Conversation

Copilot AI commented Sep 21, 2026

Copy link
Copy Markdown

This PR migrates sb-graph’s public header surface to a namespaced include layout so downstream users consume headers via <sbgraph/...> instead of ambiguous top-level paths. It also updates CMake export/install wiring so SBG::sbgraph exposes only the prefixed public include tree.

  • Public API include layout

    • Added include/sbgraph/ tree and placed the required public API headers under include/sbgraph/<original path>.
    • Included additional transitive headers in the same tree so the published public headers are self-contained for downstream compile.
  • Include-path migration across project sources

    • Updated includes in headers/sources/tests/tools from old project-root paths (algorithms/..., eval/..., sbg/..., util/..., etc.) to <sbgraph/...>.
    • This makes in-repo builds validate the installed-style API contract instead of relying on source-root include leakage.
  • CMake interface + install behavior

    • SBG::sbgraph now uses:
      • BUILD_INTERFACE: ${PROJECT_SOURCE_DIR}/include
      • INSTALL_INTERFACE: ${CMAKE_INSTALL_INCLUDEDIR}
    • Install rules now install only include/sbgraph to ${CMAKE_INSTALL_INCLUDEDIR}.
    • Preserved package export/config flow (SBGraphConfig.cmake, SBGraphTargets.cmake, SBG::sbgraph).
  • Consumer validation target

    • Added a small compile-only consumer target under tests that includes public headers via <sbgraph/...> and links through SBG::sbgraph.
target_include_directories(sbgraph
  PUBLIC
    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

install(DIRECTORY include/sbgraph
  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
  FILES_MATCHING PATTERN "*.hpp")
#include <sbgraph/eval/file_evaluator.hpp>
#include <sbgraph/sbg/sbg.hpp>
  • CI/build note tied to issue scope
    • The referenced CI failure investigated from Actions logs was make: *** No rule to make target 'test'. Stop.

Copilot AI and others added 2 commits September 21, 2026 01:47
Co-authored-by: Kalashnikovni <19693048+Kalashnikovni@users.noreply.github.com>
Co-authored-by: Kalashnikovni <19693048+Kalashnikovni@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement public-header include layout migration for sb-graph Migrate public headers to include/sbgraph and enforce <sbgraph/...> consumption Sep 21, 2026
Copilot AI requested a review from Kalashnikovni September 21, 2026 01:52
@Kalashnikovni Kalashnikovni added the enhancement New feature or request label Sep 21, 2026
@Kalashnikovni Kalashnikovni added this to the SB-Graph 5.0.0 milestone Sep 21, 2026
@Kalashnikovni Kalashnikovni linked an issue Sep 21, 2026 that may be closed by this pull request

@Kalashnikovni Kalashnikovni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build failed, corrections are needed. @copilot is mandatory that the project builds and passes tests.

@Kalashnikovni Kalashnikovni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build failed, corrections are needed. @copilot is mandatory that the project builds and passes tests.

Co-authored-by: Kalashnikovni <19693048+Kalashnikovni@users.noreply.github.com>

Copilot AI commented Sep 21, 2026

Copy link
Copy Markdown
Author

Build failed, corrections are needed. @copilot is mandatory that the project builds and passes tests.

Addressed in commit ddc641c. I fixed the CI compile failure by adding ${PROJECT_SOURCE_DIR} to test/performance benchmark target include directories so internal benchmark headers like test/performance/bm_exec.hpp resolve correctly during build.

Co-authored-by: Kalashnikovni <19693048+Kalashnikovni@users.noreply.github.com>

@Kalashnikovni Kalashnikovni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Build still not complete, fails with the error:

[100%] Building CXX object test/performance/CMakeFiles/sbg-benchmark.dir/utils.cpp.o
/home/runner/work/sb-graph/sb-graph/test/performance/utils.cpp: In function ‘bool SBG::perf::detail::updateN(const std::string&, int)’:
/home/runner/work/sb-graph/sb-graph/test/performance/utils.cpp:65:11: error: ‘ERROR’ is not a member of ‘SBG::Util’
   65 |     Util::ERROR("Unable to open file ", filename, "\n");
      |           ^~~~~
make[2]: *** [test/performance/CMakeFiles/sbg-benchmark.dir/build.make:219: test/performance/CMakeFiles/sbg-benchmark.dir/utils.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1363: test/performance/CMakeFiles/sbg-benchmark.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

Co-authored-by: Kalashnikovni <19693048+Kalashnikovni@users.noreply.github.com>

Copilot AI commented Sep 21, 2026

Copy link
Copy Markdown
Author

@copilot Build still not complete, fails with the error:

[100%] Building CXX object test/performance/CMakeFiles/sbg-benchmark.dir/utils.cpp.o...

Fixed in commit 05be7ee. I added #include <sbgraph/util/debug.hpp> in test/performance/utils.cpp, which provides SBG::Util::ERROR and resolves the reported compile error.

Copilot AI requested a review from Kalashnikovni September 21, 2026 02:27
@Kalashnikovni
Kalashnikovni marked this pull request as ready for review September 21, 2026 02:34
@Kalashnikovni
Kalashnikovni merged commit bd2bb88 into iss-135-define-include-directory Sep 21, 2026
1 check passed
@Kalashnikovni
Kalashnikovni deleted the copilot/iss-135-define-include-directory branch September 21, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Development

Successfully merging this pull request may close these issues.

Define include directory

2 participants