Skip to content

update boost install to include natvis files #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

update boost install to include natvis files #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4af4306
update boost install to include natvis files
cmazakas Mar 25, 2025
a5956e5
update boost_install to install extra/ separately
cmazakas Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions include/BoostInstall.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,31 @@ function(__boost_install_update_include_directory lib incdir prop)

endfunction()

function(__boost_install_update_natvis lib extradir)

if(lib MATCHES "^boost_(.*)$")

get_target_property(sources ${lib} INTERFACE_SOURCES)

foreach(src IN LISTS sources)

set(natvis_file "${extradir}/${lib}.natvis")
Copy link

Choose a reason for hiding this comment

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

Given that we're only looking for natvis files that match this exact name, could we emit a warning if we encounter a natvis file that doesn't match? It would be nice to mitigate against the contributor/maintainer making this mistake

Or maybe some other Boost tool should lint the library repos to maintain consistency. Then this suggestion would be worthwhile in that place


if("${src}" STREQUAL "${natvis_file}" OR "${src}" STREQUAL "$<BUILD_INTERFACE:${natvis_file}>")

message("successfully patching natvis for: ${lib}, with: ${natvis_file}")
Copy link

Choose a reason for hiding this comment

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

Is this print just meant for development? It doesn't match the style of the other message calls in this file, and I'm not sure it's worth keeping

set_target_properties(${lib} PROPERTIES INTERFACE_SOURCES "$<BUILD_INTERFACE:${natvis_file}>;$<INSTALL_INTERFACE:${CMAKE_INSTALL_DATADIR}/${lib}.natvis>")
Copy link

Choose a reason for hiding this comment

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

If you go with the change to the install directory, then you should probably forward along the version argument to this function, and use ${CMAKE_INSTALL_DATADIR}/${lib}-${version}/${lib}.natvis


endif()

endforeach()

endif()

endfunction()

# Installs a single target
# boost_install_target(TARGET target VERSION version [HEADER_DIRECTORY directory])
# boost_install_target(TARGET target VERSION version [HEADER_DIRECTORY directory] [EXTRA_DIRECTORY directory])
Copy link

Choose a reason for hiding this comment

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

The cmake_parse_arguments call below is missing EXTRA_DIRECTORY


function(boost_install_target)

Expand Down Expand Up @@ -316,6 +339,10 @@ function(boost_install_target)
if(TYPE STREQUAL "STATIC_LIBRARY" AND NOT CMAKE_VERSION VERSION_LESS 3.15)
install(FILES "$<TARGET_FILE_DIR:${LIB}>/$<TARGET_FILE_PREFIX:${LIB}>$<TARGET_FILE_BASE_NAME:${LIB}>.pdb" DESTINATION ${CMAKE_INSTALL_LIBDIR} OPTIONAL)
endif()

if(__EXTRA_DIRECTORY)
__boost_install_update_natvis(${LIB} ${__EXTRA_DIRECTORY})
endif()
endif()

install(EXPORT ${LIB}-targets DESTINATION "${CONFIG_INSTALL_DIR}" NAMESPACE Boost:: FILE ${LIB}-targets.cmake)
Expand Down Expand Up @@ -508,11 +535,11 @@ function(boost_install_target)

endfunction()

# boost_install([VERSION version] [TARGETS targets...] [HEADER_DIRECTORY directory])
# boost_install([VERSION version] [TARGETS targets...] [HEADER_DIRECTORY directory] [EXTRA_DIRECTORY directory])

function(boost_install)

cmake_parse_arguments(_ "" "VERSION;HEADER_DIRECTORY" "TARGETS" ${ARGN})
cmake_parse_arguments(_ "" "VERSION;HEADER_DIRECTORY;EXTRA_DIRECTORY" "TARGETS" ${ARGN})

if(NOT __VERSION)

Expand Down Expand Up @@ -543,9 +570,16 @@ function(boost_install)

endif()

if(__EXTRA_DIRECTORY AND NOT BOOST_SKIP_INSTALL_RULES AND NOT CMAKE_SKIP_INSTALL_RULES)

get_filename_component(__EXTRA_DIRECTORY "${__EXTRA_DIRECTORY}" ABSOLUTE)
install(DIRECTORY "${__EXTRA_DIRECTORY}/" DESTINATION "${CMAKE_INSTALL_DATADIR}")
Copy link

Choose a reason for hiding this comment

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

I'm also concerned with dumping the "extra" directory directly into CMAKE_INSTALL_DATADIR. I'd suggest changing this current if statement to only normalize __EXTRA_DIRECTORY

if(__EXTRA_DIRECTORY)
  get_filename_component(__EXTRA_DIRECTORY "${__EXTRA_DIRECTORY}" ABSOLUTE)
end()

Then inside the foreach below, adding the install call right before the call to boost_install_target

if(__EXTRA_DIRECTORY AND NOT BOOST_SKIP_INSTALL_RULES AND NOT CMAKE_SKIP_INSTALL_RULES)
  install(DIRECTORY "${__EXTRA_DIRECTORY}/" DESTINATION "${CMAKE_INSTALL_DATADIR}/${target}-${__VERSION}")
endif()
boost_install_target(...)


endif()

foreach(target IN LISTS __TARGETS)

boost_install_target(TARGET ${target} VERSION ${__VERSION} HEADER_DIRECTORY ${__HEADER_DIRECTORY})
boost_install_target(TARGET ${target} VERSION ${__VERSION} HEADER_DIRECTORY ${__HEADER_DIRECTORY} EXTRA_DIRECTORY ${__EXTRA_DIRECTORY})

endforeach()

Expand Down
7 changes: 6 additions & 1 deletion include/BoostRoot.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,13 @@ function(__boost_auto_install __boost_lib)

if("${__boost_lib_incdir}" STREQUAL "${incdir}" OR "${__boost_lib_incdir}" STREQUAL "$<BUILD_INTERFACE:${incdir}>")

set(extradir "${BOOST_SUPERPROJECT_SOURCE_DIR}/libs/${__boost_lib}/extra")
if(NOT EXISTS "${extradir}")
set(extradir "")
endif()

boost_message(DEBUG "Enabling installation for '${__boost_lib}'")
boost_install(TARGETS "boost_${__boost_lib_target}" VERSION "${BOOST_SUPERPROJECT_VERSION}" HEADER_DIRECTORY "${incdir}")
boost_install(TARGETS "boost_${__boost_lib_target}" VERSION "${BOOST_SUPERPROJECT_VERSION}" HEADER_DIRECTORY "${incdir}" EXTRA_DIRECTORY "${extradir}")

else()
boost_message(DEBUG "Not enabling installation for '${__boost_lib}'; interface include directory '${__boost_lib_incdir}' does not equal '${incdir}' or '$<BUILD_INTERFACE:${incdir}>'")
Expand Down
Loading