Skip to content

Fix cmake bugs#25

Merged
hariharan-devarajan merged 1 commit into
llnl:developfrom
JaeseungYeom:cmake_fix
May 17, 2026
Merged

Fix cmake bugs#25
hariharan-devarajan merged 1 commit into
llnl:developfrom
JaeseungYeom:cmake_fix

Conversation

@JaeseungYeom
Copy link
Copy Markdown
Contributor

@JaeseungYeom JaeseungYeom commented May 14, 2026

Addressing issue #24

  • Fix CMAKE_BINARY_DIR to CMAKE_CURRENT_BINARY_DIR
    When FetchContent builds a dependency package, the variable needed in this CMakeLists.txt is CMAKE_CURRENT_BINARY_DIR because CMAKE_BINARY_DIR points to that of the upper project.
  • Remove redundant export

Copy link
Copy Markdown
Contributor

@rayandrew rayandrew left a comment

Choose a reason for hiding this comment

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

looks good to me, thank you @JaeseungYeom !

@hariharan-devarajan
Copy link
Copy Markdown
Member

@JaeseungYeom that is by design to point to parent. The reason it when I have nested project more than two and added a submodules it will mess up build paths.

What is the issue this creates can you elaborate?

@JaeseungYeom
Copy link
Copy Markdown
Contributor Author

JaeseungYeom commented May 16, 2026

I assume the reason you chose that is to make sure subprojects are found regardless of hierarchy of projects. That was necessary with ExternalProject_Add() which runs at build time. With ExternalProject_Add, since targets are not available at configure time, the only way to find built artifacts was to use well-known paths. external projects also had to be built in separate build steps. It is no longer necessary with FetchContent that runs at configure time. cmake processes all subprojects during configuration and their targets are immediately available. No need for a shared install location. The entire project hierarchy is built in one step with proper dependency ordering automatically handled by cmake. The benefit is that now each subproject is isolated in its own binary directory and does not pollute the parent's build directory. The updated cpp-logger builds no problem under brahma with or without dftracer and with or without dyad.
Honestly, I still do not know why pydyad was failing to build and install dftracer under spack environment. It fails with brahma. I have neither of gotcha, cpp-logger, brahma or dftracer built with spack. And still just having an activate spack environment makes it fail. So, I gave this a try and now it builds successfully.

@hariharan-devarajan
Copy link
Copy Markdown
Member

@rayandrew can u test it a bit. The goal is all projects like DFTracer, Brahma DFTracer-utils build nicely with this change.

@rayandrew
Copy link
Copy Markdown
Contributor

@JaeseungYeom @hariharan-devarajan I tested locally and it works fine.
This PR will also remove Jae-Seung's cpp-logger-fix-export.patch in this PR if merged hariharan-devarajan/brahma#52

@rayandrew
Copy link
Copy Markdown
Contributor

hi @JaeseungYeom can you remove the version bumping in this PR since this is build-related issues?

Thank you!

@hariharan-devarajan hariharan-devarajan merged commit 6e451bb into llnl:develop May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants