Skip to content

Add BUILD_EXE as cmake option#717

Merged
gonuke merged 10 commits into
svalinn:developfrom
bam241:build_exe
Feb 15, 2021
Merged

Add BUILD_EXE as cmake option#717
gonuke merged 10 commits into
svalinn:developfrom
bam241:build_exe

Conversation

@bam241

@bam241 bam241 commented Dec 21, 2020

Copy link
Copy Markdown
Member

This should allow to build dagmc libs (shared and.or static) without the exe.

Comment thread cmake/DAGMC_macros.cmake Outdated
# Install a unit test
macro (dagmc_install_test test_name ext)
message(STATUS "Building unit tests: ${test_name}")
if (NOT BUILD_EXEC)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be BUILD_EXE not BUILD_EXEC

@bam241

bam241 commented Dec 21, 2020

Copy link
Copy Markdown
Member Author

I am now missing the includes :( in the install folder....

@ljacobson64

Copy link
Copy Markdown
Member

I think that's a problem in the dagmc_install_library macro, not something you did. The shared library section starts like this:

  if (BUILD_SHARED_LIBS)
    add_library(${lib_name}-shared SHARED ${SRC_FILES})
      set_target_properties(${lib_name}-shared
        PROPERTIES OUTPUT_NAME ${lib_name}
        PUBLIC_HEADER "${PUB_HEADERS}")

while the static library section starts like this:

  if (BUILD_STATIC_LIBS)
    add_library(${lib_name}-static STATIC ${SRC_FILES})
    set_target_properties(${lib_name}-static
      PROPERTIES OUTPUT_NAME ${lib_name})

The static library section doesn't set the public headers as part of the target properties.

@bam241

bam241 commented Dec 21, 2020

Copy link
Copy Markdown
Member Author

included the static header install fix in this PR....

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a few small items from me here.

Comment thread cmake/DAGMC_macros.cmake Outdated
option(BUILD_SHARED_LIBS "Build shared libraries" ON)
option(BUILD_STATIC_LIBS "Build static libraries" ON)

option(BUILD_EXE "Build DAGMC executables" ON)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's indent the comment string to match the other options.

Comment thread cmake/DAGMC_macros.cmake Outdated
Comment thread news/PR-0717.rst
Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>
@bam241

bam241 commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

@pshriwise is there some still some undressed comments in here ?

@ljacobson64

Copy link
Copy Markdown
Member

@bam241 can you remind me what the use case is for building without executables?

@bam241

bam241 commented Jan 21, 2021

Copy link
Copy Markdown
Member Author

@bam241 can you remind me what the use case is for building without executables?

The plugin.

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All my comments were addressed. Looks good @bam241!

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - thanks @bam241

@gonuke gonuke merged commit 80c2a0e into svalinn:develop Feb 15, 2021
@bam241 bam241 deleted the build_exe branch November 28, 2024 14:40
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.

4 participants