From a8df7e3d1d57e64e0653602ead7c013fb53b51b1 Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sun, 4 May 2025 01:17:51 +0300 Subject: [PATCH 01/13] major refactor to move the installation logic to the ament_package extension Signed-off-by: Nadav Elkabets --- .../ament_cmake_python-extras.cmake | 11 ++ .../cmake/ament_python_install_package.cmake | 145 +++----------- ...t_python_install_registered_packages.cmake | 182 ++++++++++++++++++ 3 files changed, 219 insertions(+), 119 deletions(-) create mode 100644 ament_cmake_python/cmake/ament_python_install_registered_packages.cmake diff --git a/ament_cmake_python/ament_cmake_python-extras.cmake b/ament_cmake_python/ament_cmake_python-extras.cmake index d8825818..bd239c4a 100644 --- a/ament_cmake_python/ament_cmake_python-extras.cmake +++ b/ament_cmake_python/ament_cmake_python-extras.cmake @@ -79,6 +79,17 @@ print(os.path.relpath(sysconfig.get_path('purelib', **kwargs), start='${CMAKE_IN endif() endmacro() +macro(_ament_cmake_python_register_extension_hook) + if(NOT DEFINED AMENT_CMAKE_PYTHON_EXTENSION_REGISTERED) + set(AMENT_CMAKE_PYTHON_EXTENSION_REGISTERED TRUE) + + ament_register_extension( + "ament_package" + "ament_cmake_python" + "ament_python_install_registered_packages.cmake") + endif() +endmacro() + include("${ament_cmake_python_DIR}/ament_python_install_module.cmake") include("${ament_cmake_python_DIR}/ament_python_install_package.cmake") include("${ament_cmake_python_DIR}/ament_get_python_install_dir.cmake") diff --git a/ament_cmake_python/cmake/ament_python_install_package.cmake b/ament_cmake_python/cmake/ament_python_install_package.cmake index 25a08092..cdbe86b7 100644 --- a/ament_cmake_python/cmake/ament_python_install_package.cmake +++ b/ament_cmake_python/cmake/ament_python_install_package.cmake @@ -35,6 +35,7 @@ # :type SKIP_COMPILE: option # macro(ament_python_install_package) + _ament_cmake_python_register_extension_hook() _ament_cmake_python_register_environment_hook() _ament_cmake_python_install_package(${ARGN}) endmacro() @@ -83,129 +84,35 @@ function(_ament_cmake_python_install_package package_name) set(ARG_DESTINATION ${PYTHON_INSTALL_DIR}) endif() - set(build_dir "${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_python/${package_name}") - - string(CONFIGURE "\ -from setuptools import find_packages -from setuptools import setup - -setup( - name='${package_name}', - version='${ARG_VERSION}', - packages=find_packages( - include=('${package_name}', '${package_name}.*')), -) -" setup_py_content) - - file(GENERATE - OUTPUT "${build_dir}/setup.py" - CONTENT "${setup_py_content}" - ) - - if(AMENT_CMAKE_SYMLINK_INSTALL) - add_custom_target( - ament_cmake_python_symlink_${package_name} - COMMAND ${CMAKE_COMMAND} -E create_symlink - "${ARG_PACKAGE_DIR}" "${build_dir}/${package_name}" - ) - set(egg_dependencies ament_cmake_python_symlink_${package_name}) - - if(ARG_SETUP_CFG) - add_custom_target( - ament_cmake_python_symlink_${package_name}_setup - COMMAND ${CMAKE_COMMAND} -E create_symlink - "${ARG_SETUP_CFG}" "${build_dir}/setup.cfg" - ) - list(APPEND egg_dependencies ament_cmake_python_symlink_${package_name}_setup) - endif() + get_property(_pkgs GLOBAL PROPERTY AMENT_CMAKE_PYTHON_PKGS) + list(FIND _pkgs "${package_name}" _idx) + if(_idx EQUAL -1) + set_property(GLOBAL APPEND PROPERTY AMENT_CMAKE_PYTHON_PKGS "${package_name}") else() - add_custom_target( - ament_cmake_python_copy_${package_name} - COMMAND ${CMAKE_COMMAND} -E copy_directory - "${ARG_PACKAGE_DIR}" "${build_dir}/${package_name}" - ) - set(egg_dependencies ament_cmake_python_copy_${package_name}) - - if(ARG_SETUP_CFG) - add_custom_target( - ament_cmake_python_copy_${package_name}_setup - COMMAND ${CMAKE_COMMAND} -E copy - "${ARG_SETUP_CFG}" "${build_dir}/setup.cfg" - ) - list(APPEND egg_dependencies ament_cmake_python_copy_${package_name}_setup) - endif() + message(STATUS "ament_python_install_package: extending '${package_name}'") endif() - # Technically, we should call find_package(Python3) first to ensure that Python3::Interpreter - # is available. But we skip this here because this macro requires ament_cmake, and ament_cmake - # calls find_package(Python3) for us. - get_executable_path(python_interpreter Python3::Interpreter BUILD) - - add_custom_target( - ament_cmake_python_build_${package_name}_egg ALL - COMMAND ${python_interpreter} setup.py egg_info - WORKING_DIRECTORY "${build_dir}" - DEPENDS ${egg_dependencies} - ) - - set(python_version "py${Python3_VERSION_MAJOR}.${Python3_VERSION_MINOR}") - - set(egg_name "${package_name}") - set(egg_install_name "${egg_name}-${ARG_VERSION}") - set(egg_install_name "${egg_install_name}-${python_version}") - - install( - DIRECTORY "${build_dir}/${egg_name}.egg-info/" - DESTINATION "${ARG_DESTINATION}/${egg_install_name}.egg-info" - ) - - if(ARG_SCRIPTS_DESTINATION) - file(MAKE_DIRECTORY "${build_dir}/scripts") # setup.py may or may not create it - - add_custom_target( - ament_cmake_python_build_${package_name}_scripts ALL - COMMAND ${python_interpreter} setup.py install_scripts -d scripts - WORKING_DIRECTORY "${build_dir}" - DEPENDS ${egg_dependencies} - ) - - if(NOT AMENT_CMAKE_SYMLINK_INSTALL) - # Not needed for nor supported by symlink installs - set(_extra_install_args USE_SOURCE_PERMISSIONS) - endif() - - install( - DIRECTORY "${build_dir}/scripts/" - DESTINATION "${ARG_SCRIPTS_DESTINATION}/" - ${_extra_install_args} - ) + get_property(_dirs GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_PACKAGE_DIRS) + list(FIND _dirs "${ARG_PACKAGE_DIR}" _didx) + if(_didx EQUAL -1) + set_property(GLOBAL APPEND PROPERTY AMENT_CMAKE_PYTHON_${package_name}_PACKAGE_DIRS "${ARG_PACKAGE_DIR}") + else() + message(WARNING "duplicate PACKAGE_DIR for '${package_name}', skipping") endif() - install( - DIRECTORY "${ARG_PACKAGE_DIR}/" - DESTINATION "${ARG_DESTINATION}/${package_name}" - PATTERN "*.pyc" EXCLUDE - PATTERN "__pycache__" EXCLUDE - ) - - if(NOT ARG_SKIP_COMPILE) - get_executable_path(python_interpreter_config Python3::Interpreter CONFIGURE) - # compile Python files - install(CODE - "execute_process( - COMMAND - \"${python_interpreter_config}\" \"-m\" \"compileall\" - \"${CMAKE_INSTALL_PREFIX}/${ARG_DESTINATION}/${package_name}\" - )" - ) - endif() + _ament_cmake_python_override(SKIP_COMPILE) + _ament_cmake_python_override(VERSION) + _ament_cmake_python_override(SETUP_CFG) + _ament_cmake_python_override(DESTINATION) + _ament_cmake_python_override(SCRIPTS_DESTINATION) +endfunction() - if(package_name IN_LIST AMENT_CMAKE_PYTHON_INSTALL_INSTALLED_NAMES) - message(FATAL_ERROR - "ament_python_install_package() a Python module file or package with " - "the same name '${package_name}' has been installed before") +macro(_ament_cmake_python_override param) + set(_prop "AMENT_CMAKE_PYTHON_${package_name}_${param}") + set(_val "${ARG_${param}}") + get_property(_old_val GLOBAL PROPERTY ${_prop}) + if(_old_val AND NOT _old_val STREQUAL "${_val}") + message(WARNING "${param} for '${package_name}' changed from '${_old_val}' to '${_val}'") endif() - list(APPEND AMENT_CMAKE_PYTHON_INSTALL_INSTALLED_NAMES "${package_name}") - set(AMENT_CMAKE_PYTHON_INSTALL_INSTALLED_NAMES - "${AMENT_CMAKE_PYTHON_INSTALL_INSTALLED_NAMES}" PARENT_SCOPE) -endfunction() + set_property(GLOBAL PROPERTY ${_prop} "${_val}") +endmacro() diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake new file mode 100644 index 00000000..9f17af41 --- /dev/null +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -0,0 +1,182 @@ + +function(ament_cmake_python_install_registered_packages) + get_property(_pkgs GLOBAL PROPERTY AMENT_CMAKE_PYTHON_PKGS) + foreach(pkg IN LISTS _pkgs) + _ament_cmake_python_install_package_impl(${pkg}) + endforeach() +endfunction() + +function(_ament_cmake_python_install_package_impl package_name) + foreach(_prop IN ITEMS SKIP_COMPILE VERSION SETUP_CFG DESTINATION SCRIPTS_DESTINATION PACKAGE_DIRS) + get_property(_${_prop} GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_${_prop}) + endforeach() + + _ament_cmake_python_prepare_build(${package_name}) + _ament_cmake_python_copy_or_symlink(${package_name}) + + # Technically, we should call find_package(Python3) first to ensure that Python3::Interpreter + # is available. But we skip this here because this macro requires ament_cmake, and ament_cmake + # calls find_package(Python3) for us. + get_executable_path(python_interpreter Python3::Interpreter BUILD) + + _ament_cmake_python_generate_egg(${package_name}) + + if(_SCRIPTS_DESTINATION) + _ament_cmake_python_install_scripts(${package_name}) + endif() + + _ament_cmake_python_install_sources(${package_name}) + + if(NOT _SKIP_COMPILE) + _ament_cmake_python_byte_compile(${package_name}) + endif() + +endfunction() + +macro(_ament_cmake_python_prepare_build package_name) + set(_build_dir "${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_python/${package_name}") + + string(CONFIGURE "\ +from setuptools import find_packages +from setuptools import setup + +setup( + name='${package_name}', + version='${_VERSION}', + packages=find_packages( + include=('${package_name}', '${package_name}.*')), +) +" setup_py_content) + + file(GENERATE + OUTPUT "${_build_dir}/setup.py" + CONTENT "${setup_py_content}" + ) + +endmacro() + +macro(_ament_cmake_python_copy_or_symlink package_name) + set(_sync_target "ament_cmake_python_sync_${package_name}") + add_custom_target(${_sync_target}) + + foreach(_dir IN LISTS _PACKAGE_DIRS) + file(GLOB_RECURSE _dir_files RELATIVE "${_dir}" "${_dir}/*") + foreach(_rel IN LISTS _dir_files) + set(_src "${_dir}/${_rel}") + set(_dst "${_build_dir}/${package_name}/${_rel}") + + get_filename_component(_dst_parent "${_dst}" DIRECTORY) + file(MAKE_DIRECTORY "${_dst_parent}") + + list(FIND _already_processed "${_dst}" _idx) + if(NOT _idx EQUAL -1) + continue() + endif() + list(APPEND _already_processed "${_dst}") + + if(AMENT_CMAKE_SYMLINK_INSTALL) + add_custom_command( + OUTPUT "${_dst}" + COMMAND ${CMAKE_COMMAND} -E create_symlink "${_src}" "${_dst}" + DEPENDS "${_src}" + COMMENT "Symlinking ${_rel}" + VERBATIM + ) + else() + add_custom_command( + OUTPUT "${_dst}" + COMMAND ${CMAKE_COMMAND} -E copy_if_different "${_src}" "${_dst}" + DEPENDS "${_src}" + COMMENT "Copying ${_rel}" + VERBATIM + ) + endif() + endforeach() + endforeach() + + if(_SETUP_CFG) + set(_cfg_dst "${_build_dir}/setup.cfg") + if(AMENT_CMAKE_SYMLINK_INSTALL) + set(_copy_cmd ${CMAKE_COMMAND} -E create_symlink "${_SETUP_CFG}" "${_cfg_dst}") + else() + set(_copy_cmd ${CMAKE_COMMAND} -E copy_if_different "${_SETUP_CFG}" "${_cfg_dst}") + endif() + + add_custom_command( + OUTPUT "${_cfg_dst}" + COMMAND ${_copy_cmd} + DEPENDS "${_SETUP_CFG}" + COMMENT "Synchronising setup.cfg" + VERBATIM + ) + add_custom_target(phony_${package_name}_cfg DEPENDS "${_cfg_dst}") + add_dependencies(${_sync_target} phony_${package_name}_cfg) + endif() +endmacro() + +macro(_ament_cmake_python_generate_egg package_name) + add_custom_target( + ament_cmake_python_build_${package_name}_egg ALL + COMMAND ${python_interpreter} setup.py egg_info + WORKING_DIRECTORY "${_build_dir}" + DEPENDS ${_sync_target} + ) + + set(python_version "py${Python3_VERSION_MAJOR}.${Python3_VERSION_MINOR}") + + set(egg_name "${package_name}") + set(egg_install_name "${egg_name}-${_VERSION}") + set(egg_install_name "${egg_install_name}-${python_version}") + + install( + DIRECTORY "${_build_dir}/${egg_name}.egg-info/" + DESTINATION "${_DESTINATION}/${egg_install_name}.egg-info" + ) +endmacro() + +macro(_ament_cmake_python_install_scripts package_name) + file(MAKE_DIRECTORY "${_build_dir}/scripts") # setup.py may or may not create it + + add_custom_target( + ament_cmake_python_build_${package_name}_scripts ALL + COMMAND ${python_interpreter} setup.py install_scripts -d scripts + WORKING_DIRECTORY "${_build_dir}" + DEPENDS ${_sync_target} + ) + + if(NOT AMENT_CMAKE_SYMLINK_INSTALL) + # Not needed for nor supported by symlink installs + set(_extra_install_args USE_SOURCE_PERMISSIONS) + endif() + + install( + DIRECTORY "${_build_dir}/scripts/" + DESTINATION "${_SCRIPTS_DESTINATION}/" + ${_extra_install_args} + ) +endmacro() + +macro(_ament_cmake_python_install_sources package_name) + foreach(_dir IN LISTS _PACKAGE_DIRS) + install( + DIRECTORY "${_dir}/" + DESTINATION "${_DESTINATION}/${package_name}" + PATTERN "*.pyc" EXCLUDE + PATTERN "__pycache__" EXCLUDE + ) + endforeach() +endmacro() + +macro(_ament_cmake_python_byte_compile package_name) + get_executable_path(python_interpreter_config Python3::Interpreter CONFIGURE) + # compile Python files + install(CODE + "execute_process( + COMMAND + \"${python_interpreter_config}\" \"-m\" \"compileall\" + \"${CMAKE_INSTALL_PREFIX}/${_DESTINATION}/${package_name}\" + )" + ) +endmacro() + +ament_cmake_python_install_registered_packages() \ No newline at end of file From 00311aa6de0c73763add9cf26b17c19abd160bf4 Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sat, 14 Jun 2025 17:21:46 +0300 Subject: [PATCH 02/13] fix: always override with the latest file Signed-off-by: Nadav Elkabets --- ...t_python_install_registered_packages.cmake | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 9f17af41..7ed54b2e 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -57,29 +57,42 @@ endmacro() macro(_ament_cmake_python_copy_or_symlink package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") - add_custom_target(${_sync_target}) + set(_dsts "") + set(_srcs "") foreach(_dir IN LISTS _PACKAGE_DIRS) file(GLOB_RECURSE _dir_files RELATIVE "${_dir}" "${_dir}/*") foreach(_rel IN LISTS _dir_files) set(_src "${_dir}/${_rel}") set(_dst "${_build_dir}/${package_name}/${_rel}") - get_filename_component(_dst_parent "${_dst}" DIRECTORY) - file(MAKE_DIRECTORY "${_dst_parent}") - - list(FIND _already_processed "${_dst}" _idx) + list(FIND _dsts "${_dst}" _idx) if(NOT _idx EQUAL -1) - continue() + list(REMOVE_AT _dsts ${_idx}) + list(REMOVE_AT _srcs ${_idx}) endif() - list(APPEND _already_processed "${_dst}") + list(APPEND _dsts "${_dst}") + list(APPEND _srcs "${_src}") + endforeach() + endforeach() + + set(_sync_deps "") + list(LENGTH _dsts _len) + if(_len GREATER 0) + math(EXPR _last "${_len} - 1") + foreach(_file_idx RANGE 0 ${_last}) + list(GET _dsts ${_file_idx} _dst) + list(GET _srcs ${_file_idx} _src) + + get_filename_component(_dst_parent "${_dst}" DIRECTORY) + file(MAKE_DIRECTORY "${_dst_parent}") if(AMENT_CMAKE_SYMLINK_INSTALL) add_custom_command( OUTPUT "${_dst}" COMMAND ${CMAKE_COMMAND} -E create_symlink "${_src}" "${_dst}" DEPENDS "${_src}" - COMMENT "Symlinking ${_rel}" + COMMENT "Symlinking ${_dst}" VERBATIM ) else() @@ -87,12 +100,13 @@ macro(_ament_cmake_python_copy_or_symlink package_name) OUTPUT "${_dst}" COMMAND ${CMAKE_COMMAND} -E copy_if_different "${_src}" "${_dst}" DEPENDS "${_src}" - COMMENT "Copying ${_rel}" + COMMENT "Copying ${_dst}" VERBATIM ) endif() + list(APPEND _sync_deps "${_dst}") endforeach() - endforeach() + endif() if(_SETUP_CFG) set(_cfg_dst "${_build_dir}/setup.cfg") @@ -109,9 +123,11 @@ macro(_ament_cmake_python_copy_or_symlink package_name) COMMENT "Synchronising setup.cfg" VERBATIM ) - add_custom_target(phony_${package_name}_cfg DEPENDS "${_cfg_dst}") - add_dependencies(${_sync_target} phony_${package_name}_cfg) + list(APPEND _sync_deps "${_cfg_dst}") endif() + + add_custom_target(${_sync_target} DEPENDS ${_sync_deps}) + endmacro() macro(_ament_cmake_python_generate_egg package_name) From 143a4988d0dd314ded8422009d4f9eefcf8b045b Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sat, 14 Jun 2025 18:55:48 +0300 Subject: [PATCH 03/13] Rerun glob on changes Signed-off-by: Nadav Elkabets --- .../cmake/ament_python_install_registered_packages.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 7ed54b2e..d034e818 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -61,7 +61,7 @@ macro(_ament_cmake_python_copy_or_symlink package_name) set(_dsts "") set(_srcs "") foreach(_dir IN LISTS _PACKAGE_DIRS) - file(GLOB_RECURSE _dir_files RELATIVE "${_dir}" "${_dir}/*") + file(GLOB_RECURSE _dir_files CONFIGURE_DEPENDS RELATIVE "${_dir}" "${_dir}/*") foreach(_rel IN LISTS _dir_files) set(_src "${_dir}/${_rel}") set(_dst "${_build_dir}/${package_name}/${_rel}") From a8419aead5322580e5c31e9d221228a91077a786 Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sat, 14 Jun 2025 19:55:54 +0300 Subject: [PATCH 04/13] Only copy during build Signed-off-by: Nadav Elkabets --- ...t_python_install_registered_packages.cmake | 110 +++++++----------- 1 file changed, 44 insertions(+), 66 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index d034e818..78e76d8e 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -1,3 +1,16 @@ +# Copyright 2025 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. function(ament_cmake_python_install_registered_packages) get_property(_pkgs GLOBAL PROPERTY AMENT_CMAKE_PYTHON_PKGS) @@ -12,7 +25,7 @@ function(_ament_cmake_python_install_package_impl package_name) endforeach() _ament_cmake_python_prepare_build(${package_name}) - _ament_cmake_python_copy_or_symlink(${package_name}) + _ament_cmake_python_copy_build_files(${package_name}) # Technically, we should call find_package(Python3) first to ensure that Python3::Interpreter # is available. But we skip this here because this macro requires ament_cmake, and ament_cmake @@ -55,79 +68,44 @@ setup( endmacro() -macro(_ament_cmake_python_copy_or_symlink package_name) +macro(_ament_cmake_python_copy_build_files package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") + set(_stage_dir "${_build_dir}/${package_name}") + set(_stamp "${_stage_dir}/.sync_stamp") + + set(_cmds "") + + list(APPEND _cmds + COMMAND ${CMAKE_COMMAND} -E remove_directory "${_stage_dir}") - set(_dsts "") - set(_srcs "") foreach(_dir IN LISTS _PACKAGE_DIRS) - file(GLOB_RECURSE _dir_files CONFIGURE_DEPENDS RELATIVE "${_dir}" "${_dir}/*") - foreach(_rel IN LISTS _dir_files) - set(_src "${_dir}/${_rel}") - set(_dst "${_build_dir}/${package_name}/${_rel}") - - list(FIND _dsts "${_dst}" _idx) - if(NOT _idx EQUAL -1) - list(REMOVE_AT _dsts ${_idx}) - list(REMOVE_AT _srcs ${_idx}) - endif() - list(APPEND _dsts "${_dst}") - list(APPEND _srcs "${_src}") - endforeach() + list(APPEND _cmds + COMMAND ${CMAKE_COMMAND} -E copy_directory + "${_dir}" "${_stage_dir}") endforeach() - set(_sync_deps "") - list(LENGTH _dsts _len) - if(_len GREATER 0) - math(EXPR _last "${_len} - 1") - foreach(_file_idx RANGE 0 ${_last}) - list(GET _dsts ${_file_idx} _dst) - list(GET _srcs ${_file_idx} _src) - - get_filename_component(_dst_parent "${_dst}" DIRECTORY) - file(MAKE_DIRECTORY "${_dst_parent}") - - if(AMENT_CMAKE_SYMLINK_INSTALL) - add_custom_command( - OUTPUT "${_dst}" - COMMAND ${CMAKE_COMMAND} -E create_symlink "${_src}" "${_dst}" - DEPENDS "${_src}" - COMMENT "Symlinking ${_dst}" - VERBATIM - ) - else() - add_custom_command( - OUTPUT "${_dst}" - COMMAND ${CMAKE_COMMAND} -E copy_if_different "${_src}" "${_dst}" - DEPENDS "${_src}" - COMMENT "Copying ${_dst}" - VERBATIM - ) - endif() - list(APPEND _sync_deps "${_dst}") - endforeach() - endif() - if(_SETUP_CFG) - set(_cfg_dst "${_build_dir}/setup.cfg") - if(AMENT_CMAKE_SYMLINK_INSTALL) - set(_copy_cmd ${CMAKE_COMMAND} -E create_symlink "${_SETUP_CFG}" "${_cfg_dst}") - else() - set(_copy_cmd ${CMAKE_COMMAND} -E copy_if_different "${_SETUP_CFG}" "${_cfg_dst}") - endif() - - add_custom_command( - OUTPUT "${_cfg_dst}" - COMMAND ${_copy_cmd} - DEPENDS "${_SETUP_CFG}" - COMMENT "Synchronising setup.cfg" - VERBATIM - ) - list(APPEND _sync_deps "${_cfg_dst}") + list(APPEND _cmds + COMMAND ${CMAKE_COMMAND} -E copy_if_different + "${_SETUP_CFG}" "${_build_dir}/setup.cfg") endif() - add_custom_target(${_sync_target} DEPENDS ${_sync_deps}) + foreach(_dir IN LISTS _PACKAGE_DIRS) + list(APPEND _cmds + COMMAND ${CMAKE_COMMAND} -E touch "${_dir}") + endforeach() + + list(APPEND _cmds + COMMAND ${CMAKE_COMMAND} -E touch "${_stamp}") + + add_custom_command( + OUTPUT "${_stamp}" + ${_cmds} + DEPENDS ${_PACKAGE_DIRS} ${_SETUP_CFG} + COMMENT "Synchronising sources for ${package_name} (copy_directory)" + VERBATIM) + add_custom_target(${_sync_target} DEPENDS "${_stamp}") endmacro() macro(_ament_cmake_python_generate_egg package_name) @@ -195,4 +173,4 @@ macro(_ament_cmake_python_byte_compile package_name) ) endmacro() -ament_cmake_python_install_registered_packages() \ No newline at end of file +ament_cmake_python_install_registered_packages() From 4993fa0ce4c42d4321fda131e52a39b92f7be85a Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sat, 5 Jul 2025 19:14:43 +0300 Subject: [PATCH 05/13] Keep existing build behavior Signed-off-by: Nadav Elkabets --- ...t_python_install_registered_packages.cmake | 37 +++++-------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 78e76d8e..379e9321 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -70,42 +70,23 @@ endmacro() macro(_ament_cmake_python_copy_build_files package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") - set(_stage_dir "${_build_dir}/${package_name}") - set(_stamp "${_stage_dir}/.sync_stamp") - - set(_cmds "") - - list(APPEND _cmds - COMMAND ${CMAKE_COMMAND} -E remove_directory "${_stage_dir}") + add_custom_target(${_sync_target} DEPENDS ${_PACKAGE_DIRS} ${_SETUP_CFG}) + foreach(_dir IN LISTS _PACKAGE_DIRS) - list(APPEND _cmds + add_custom_command(TARGET ${_sync_target} COMMAND ${CMAKE_COMMAND} -E copy_directory - "${_dir}" "${_stage_dir}") + "${_dir}" "${_build_dir}/${package_name}" + ) endforeach() if(_SETUP_CFG) - list(APPEND _cmds + add_custom_command(TARGET ${_sync_target} COMMAND ${CMAKE_COMMAND} -E copy_if_different - "${_SETUP_CFG}" "${_build_dir}/setup.cfg") + "${_SETUP_CFG}" "${_build_dir}/setup.cfg" + ) endif() - - foreach(_dir IN LISTS _PACKAGE_DIRS) - list(APPEND _cmds - COMMAND ${CMAKE_COMMAND} -E touch "${_dir}") - endforeach() - - list(APPEND _cmds - COMMAND ${CMAKE_COMMAND} -E touch "${_stamp}") - - add_custom_command( - OUTPUT "${_stamp}" - ${_cmds} - DEPENDS ${_PACKAGE_DIRS} ${_SETUP_CFG} - COMMENT "Synchronising sources for ${package_name} (copy_directory)" - VERBATIM) - - add_custom_target(${_sync_target} DEPENDS "${_stamp}") + endmacro() macro(_ament_cmake_python_generate_egg package_name) From 9f0fd4cb737af9e1a0e0e9c05be0b21d6306ee20 Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sun, 10 Aug 2025 01:39:38 +0300 Subject: [PATCH 06/13] feature: add tests for overlayed packages Signed-off-by: Nadav Elkabets --- ament_cmake_python/CMakeLists.txt | 25 +++++++++++++++++++ .../ament_python_test_package/__init__.py | 0 .../test/ament_python_test_package/main.py | 3 +++ .../__init__.py | 1 + .../subdir/__init__.py | 0 .../subdir/utils.py | 2 ++ .../test/test_ament_python_install_package.py | 24 ++++++++++++++++++ 7 files changed, 55 insertions(+) create mode 100644 ament_cmake_python/test/ament_python_test_package/__init__.py create mode 100644 ament_cmake_python/test/ament_python_test_package/main.py create mode 100644 ament_cmake_python/test/ament_python_test_package_overlay/__init__.py create mode 100644 ament_cmake_python/test/ament_python_test_package_overlay/subdir/__init__.py create mode 100644 ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py create mode 100644 ament_cmake_python/test/test_ament_python_install_package.py diff --git a/ament_cmake_python/CMakeLists.txt b/ament_cmake_python/CMakeLists.txt index 25d8b06c..0e446281 100644 --- a/ament_cmake_python/CMakeLists.txt +++ b/ament_cmake_python/CMakeLists.txt @@ -4,6 +4,31 @@ project(ament_cmake_python NONE) find_package(ament_cmake_core REQUIRED) +if(BUILD_TESTING) + find_package(ament_cmake_pytest REQUIRED) + set(ament_cmake_python_DIR "cmake") + include("ament_cmake_python-extras.cmake") + + ament_python_install_package( + ament_python_test_package + PACKAGE_DIR test/ament_python_test_package + ) + ament_python_install_package( + ament_python_test_package_overlay + PACKAGE_DIR test/ament_python_test_package + ) + ament_python_install_package( + ament_python_test_package_overlay + PACKAGE_DIR test/ament_python_test_package_overlay + ) + + ament_add_pytest_test( + test_ament_python_install_package + test/test_ament_python_install_package.py + ENV TEST_PACKAGE_INSTALL_DIR=${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR} + ) +endif() + ament_package( CONFIG_EXTRAS "ament_cmake_python-extras.cmake" ) diff --git a/ament_cmake_python/test/ament_python_test_package/__init__.py b/ament_cmake_python/test/ament_python_test_package/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ament_cmake_python/test/ament_python_test_package/main.py b/ament_cmake_python/test/ament_python_test_package/main.py new file mode 100644 index 00000000..624ef4fe --- /dev/null +++ b/ament_cmake_python/test/ament_python_test_package/main.py @@ -0,0 +1,3 @@ +class Bar: + def __init__() -> None: + pass diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/__init__.py b/ament_cmake_python/test/ament_python_test_package_overlay/__init__.py new file mode 100644 index 00000000..50fb31a6 --- /dev/null +++ b/ament_cmake_python/test/ament_python_test_package_overlay/__init__.py @@ -0,0 +1 @@ +from subdir.utils import foo diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/subdir/__init__.py b/ament_cmake_python/test/ament_python_test_package_overlay/subdir/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py b/ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py new file mode 100644 index 00000000..256ce6d1 --- /dev/null +++ b/ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py @@ -0,0 +1,2 @@ +def foo() -> None: + pass diff --git a/ament_cmake_python/test/test_ament_python_install_package.py b/ament_cmake_python/test/test_ament_python_install_package.py new file mode 100644 index 00000000..e7ed0683 --- /dev/null +++ b/ament_cmake_python/test/test_ament_python_install_package.py @@ -0,0 +1,24 @@ +from pathlib import Path +import os +import filecmp +import shutil + +INSTALL_DIR = Path(os.environ["TEST_PACKAGE_INSTALL_DIR"]) +TEST_DIR = Path(__file__).parent +AMENT_PYTHON_TEST_PACKAGE = "ament_python_test_package" +AMENT_PYTHON_TEST_PACKAGE_OVERLAY = AMENT_PYTHON_TEST_PACKAGE + "_overlay" + +def test_ament_python_test_package() -> None: + assert not filecmp.dircmp( + TEST_DIR / AMENT_PYTHON_TEST_PACKAGE, + INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE + ).diff_files + +def test_ament_python_test_package_with_overlay(tmpdir) -> None: + shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE, tmpdir, dirs_exist_ok=True) + shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY, tmpdir, dirs_exist_ok=True) + assert not filecmp.dircmp(tmpdir, INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY).diff_files + assert not filecmp.dircmp( + tmpdir / "subdir", + INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "subdir" + ).diff_files From ac34cfd718832f1d80473c7b97168e20f8efb52d Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Sun, 10 Aug 2025 01:45:38 +0300 Subject: [PATCH 07/13] Add ament_cmake_pytest build dependency Signed-off-by: Nadav Elkabets --- ament_cmake_python/package.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/ament_cmake_python/package.xml b/ament_cmake_python/package.xml index 00b98ffc..357555f8 100644 --- a/ament_cmake_python/package.xml +++ b/ament_cmake_python/package.xml @@ -13,6 +13,7 @@ Michel Hidalgo ament_cmake_core + ament_cmake_pytest ament_cmake_core From a80f864e47ef87aa3967d7e3752eb27eba81c35e Mon Sep 17 00:00:00 2001 From: Nadav Elkabets <32939935+nadavelkabets@users.noreply.github.com> Date: Fri, 22 Aug 2025 15:52:40 +0000 Subject: [PATCH 08/13] Add ament_cmake_python_test package and fix failing test Signed-off-by: Nadav Elkabets --- ament_cmake_python/CMakeLists.txt | 25 --------------- ...t_python_install_registered_packages.cmake | 18 ++++++++--- ament_cmake_python/package.xml | 2 -- ament_cmake_python_test/CMakeLists.txt | 31 +++++++++++++++++++ ament_cmake_python_test/package.xml | 21 +++++++++++++ .../ament_python_test_package/__init__.py | 0 .../test/ament_python_test_package/main.py | 0 .../__init__.py | 0 .../subdir/__init__.py | 0 .../subdir/utils.py | 0 .../test/test_ament_python_install_package.py | 6 ++++ 11 files changed, 72 insertions(+), 31 deletions(-) create mode 100644 ament_cmake_python_test/CMakeLists.txt create mode 100644 ament_cmake_python_test/package.xml rename {ament_cmake_python => ament_cmake_python_test}/test/ament_python_test_package/__init__.py (100%) rename {ament_cmake_python => ament_cmake_python_test}/test/ament_python_test_package/main.py (100%) rename {ament_cmake_python => ament_cmake_python_test}/test/ament_python_test_package_overlay/__init__.py (100%) rename {ament_cmake_python => ament_cmake_python_test}/test/ament_python_test_package_overlay/subdir/__init__.py (100%) rename {ament_cmake_python => ament_cmake_python_test}/test/ament_python_test_package_overlay/subdir/utils.py (100%) rename {ament_cmake_python => ament_cmake_python_test}/test/test_ament_python_install_package.py (75%) diff --git a/ament_cmake_python/CMakeLists.txt b/ament_cmake_python/CMakeLists.txt index 0e446281..25d8b06c 100644 --- a/ament_cmake_python/CMakeLists.txt +++ b/ament_cmake_python/CMakeLists.txt @@ -4,31 +4,6 @@ project(ament_cmake_python NONE) find_package(ament_cmake_core REQUIRED) -if(BUILD_TESTING) - find_package(ament_cmake_pytest REQUIRED) - set(ament_cmake_python_DIR "cmake") - include("ament_cmake_python-extras.cmake") - - ament_python_install_package( - ament_python_test_package - PACKAGE_DIR test/ament_python_test_package - ) - ament_python_install_package( - ament_python_test_package_overlay - PACKAGE_DIR test/ament_python_test_package - ) - ament_python_install_package( - ament_python_test_package_overlay - PACKAGE_DIR test/ament_python_test_package_overlay - ) - - ament_add_pytest_test( - test_ament_python_install_package - test/test_ament_python_install_package.py - ENV TEST_PACKAGE_INSTALL_DIR=${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR} - ) -endif() - ament_package( CONFIG_EXTRAS "ament_cmake_python-extras.cmake" ) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 379e9321..5f1cbfe2 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -72,7 +72,7 @@ macro(_ament_cmake_python_copy_build_files package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") add_custom_target(${_sync_target} DEPENDS ${_PACKAGE_DIRS} ${_SETUP_CFG}) - + foreach(_dir IN LISTS _PACKAGE_DIRS) add_custom_command(TARGET ${_sync_target} COMMAND ${CMAKE_COMMAND} -E copy_directory @@ -132,14 +132,24 @@ macro(_ament_cmake_python_install_scripts package_name) endmacro() macro(_ament_cmake_python_install_sources package_name) - foreach(_dir IN LISTS _PACKAGE_DIRS) + set(_DIRS_TO_INSTALL "${_PACKAGE_DIRS}") + list(TRANSFORM _DIRS_TO_INSTALL APPEND "/") + if(AMENT_CMAKE_SYMLINK_INSTALL) install( - DIRECTORY "${_dir}/" + DIRECTORY ${_DIRS_TO_INSTALL} DESTINATION "${_DESTINATION}/${package_name}" PATTERN "*.pyc" EXCLUDE PATTERN "__pycache__" EXCLUDE ) - endforeach() + else() + # we merge during build as cmake doesn't guarantee install sequence + install( + DIRECTORY "${_build_dir}/${package_name}/" + DESTINATION "${_DESTINATION}/${package_name}" + PATTERN "*.pyc" EXCLUDE + PATTERN "__pycache__" EXCLUDE + ) + endif() endmacro() macro(_ament_cmake_python_byte_compile package_name) diff --git a/ament_cmake_python/package.xml b/ament_cmake_python/package.xml index 357555f8..f59c8381 100644 --- a/ament_cmake_python/package.xml +++ b/ament_cmake_python/package.xml @@ -13,8 +13,6 @@ Michel Hidalgo ament_cmake_core - ament_cmake_pytest - ament_cmake_core diff --git a/ament_cmake_python_test/CMakeLists.txt b/ament_cmake_python_test/CMakeLists.txt new file mode 100644 index 00000000..a16cd525 --- /dev/null +++ b/ament_cmake_python_test/CMakeLists.txt @@ -0,0 +1,31 @@ +cmake_minimum_required(VERSION 3.12) + +project(ament_cmake_python_test) + +find_package(ament_cmake_core REQUIRED) + +if(BUILD_TESTING) + find_package(ament_cmake_pytest REQUIRED) + find_package(ament_cmake_python REQUIRED) + + ament_python_install_package( + ament_python_test_package + PACKAGE_DIR test/ament_python_test_package + ) + ament_python_install_package( + ament_python_test_package_overlay + PACKAGE_DIR test/ament_python_test_package + ) + ament_python_install_package( + ament_python_test_package_overlay + PACKAGE_DIR test/ament_python_test_package_overlay + ) + + ament_add_pytest_test( + test_ament_python_install_package + test/test_ament_python_install_package.py + ENV TEST_PACKAGE_INSTALL_DIR=${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR} + ) +endif() + +ament_package() diff --git a/ament_cmake_python_test/package.xml b/ament_cmake_python_test/package.xml new file mode 100644 index 00000000..676473ce --- /dev/null +++ b/ament_cmake_python_test/package.xml @@ -0,0 +1,21 @@ + + + + ament_cmake_python_test + 0.0.0 + Test package for ament_cmake_python. + + Nadav Elkabets + + Apache License 2.0 + + ament_cmake_core + ament_cmake_pytest + ament_cmake_python + + ament_cmake_core + + + ament_cmake + + diff --git a/ament_cmake_python/test/ament_python_test_package/__init__.py b/ament_cmake_python_test/test/ament_python_test_package/__init__.py similarity index 100% rename from ament_cmake_python/test/ament_python_test_package/__init__.py rename to ament_cmake_python_test/test/ament_python_test_package/__init__.py diff --git a/ament_cmake_python/test/ament_python_test_package/main.py b/ament_cmake_python_test/test/ament_python_test_package/main.py similarity index 100% rename from ament_cmake_python/test/ament_python_test_package/main.py rename to ament_cmake_python_test/test/ament_python_test_package/main.py diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/__init__.py b/ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py similarity index 100% rename from ament_cmake_python/test/ament_python_test_package_overlay/__init__.py rename to ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/subdir/__init__.py b/ament_cmake_python_test/test/ament_python_test_package_overlay/subdir/__init__.py similarity index 100% rename from ament_cmake_python/test/ament_python_test_package_overlay/subdir/__init__.py rename to ament_cmake_python_test/test/ament_python_test_package_overlay/subdir/__init__.py diff --git a/ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py b/ament_cmake_python_test/test/ament_python_test_package_overlay/subdir/utils.py similarity index 100% rename from ament_cmake_python/test/ament_python_test_package_overlay/subdir/utils.py rename to ament_cmake_python_test/test/ament_python_test_package_overlay/subdir/utils.py diff --git a/ament_cmake_python/test/test_ament_python_install_package.py b/ament_cmake_python_test/test/test_ament_python_install_package.py similarity index 75% rename from ament_cmake_python/test/test_ament_python_install_package.py rename to ament_cmake_python_test/test/test_ament_python_install_package.py index e7ed0683..44f5260d 100644 --- a/ament_cmake_python/test/test_ament_python_install_package.py +++ b/ament_cmake_python_test/test/test_ament_python_install_package.py @@ -2,6 +2,7 @@ import os import filecmp import shutil +import sys INSTALL_DIR = Path(os.environ["TEST_PACKAGE_INSTALL_DIR"]) TEST_DIR = Path(__file__).parent @@ -17,8 +18,13 @@ def test_ament_python_test_package() -> None: def test_ament_python_test_package_with_overlay(tmpdir) -> None: shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE, tmpdir, dirs_exist_ok=True) shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY, tmpdir, dirs_exist_ok=True) + with open(INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "__init__.py", "r") as f: + d = f.read() + print("CONTENT: " + d, file=sys.stderr) + print("is symlink: " + str(Path(str(INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "__init__.py")).is_symlink()), file=sys.stderr) assert not filecmp.dircmp(tmpdir, INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY).diff_files assert not filecmp.dircmp( tmpdir / "subdir", INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "subdir" ).diff_files + From 51bd23b083d8e277a1817aa3ace023f831a3609a Mon Sep 17 00:00:00 2001 From: Nadav Elkabets Date: Fri, 29 Aug 2025 20:30:58 +0300 Subject: [PATCH 09/13] Fix missing generated messages Signed-off-by: Nadav Elkabets --- ...t_python_install_registered_packages.cmake | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 5f1cbfe2..5fbbd7c4 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -142,12 +142,24 @@ macro(_ament_cmake_python_install_sources package_name) PATTERN "__pycache__" EXCLUDE ) else() - # we merge during build as cmake doesn't guarantee install sequence - install( - DIRECTORY "${_build_dir}/${package_name}/" - DESTINATION "${_DESTINATION}/${package_name}" - PATTERN "*.pyc" EXCLUDE - PATTERN "__pycache__" EXCLUDE + # cmake does not support overwriting a more recently modified file + # we must remove any existing files before installing the new package + install(CODE + "set(_dest \"\${CMAKE_INSTALL_PREFIX}/${_DESTINATION}/${package_name}\") + set(_dirs ${_DIRS_TO_INSTALL}) + foreach(_dir IN LISTS _dirs) + file(GLOB_RECURSE _rel_files RELATIVE \"\${_dir}\" \"\${_dir}/*\") + foreach(_file IN LISTS _rel_files) + file(REMOVE \"\${_dest}/\${_file}\") + endforeach() + file(INSTALL + DESTINATION \"\${_dest}\" + TYPE DIRECTORY + FILES \"\${_dir}\" + PATTERN \"*.pyc\" EXCLUDE + PATTERN \"__pycache__\" EXCLUDE + ) + endforeach()" ) endif() endmacro() From 9ca68ff840cbab793c9029ac5d3879426028125a Mon Sep 17 00:00:00 2001 From: = Date: Mon, 30 Mar 2026 16:40:48 +0300 Subject: [PATCH 10/13] Feature: add DEPENDS flag, tests for command args Signed-off-by: = --- .../cmake/ament_python_install_package.cmake | 35 +++----- ...t_python_install_registered_packages.cmake | 49 +++++----- ament_cmake_python_test/CMakeLists.txt | 23 +++++ .../test/ament_python_test_package/main.py | 2 +- .../__init__.py | 2 +- .../ament_python_test_package_overlay/main.py | 5 ++ .../test/test_ament_python_install_package.py | 90 ++++++++++++++----- 7 files changed, 129 insertions(+), 77 deletions(-) create mode 100644 ament_cmake_python_test/test/ament_python_test_package_overlay/main.py diff --git a/ament_cmake_python/cmake/ament_python_install_package.cmake b/ament_cmake_python/cmake/ament_python_install_package.cmake index cdbe86b7..db640565 100644 --- a/ament_cmake_python/cmake/ament_python_install_package.cmake +++ b/ament_cmake_python/cmake/ament_python_install_package.cmake @@ -33,6 +33,9 @@ # :type SCRIPTS_DESTINATION: string # :param SKIP_COMPILE: if set do not byte-compile the installed package # :type SKIP_COMPILE: option +# :param DEPENDS: build targets that must complete before +# the package files are synced to the build directory +# :type DEPENDS: list of strings # macro(ament_python_install_package) _ament_cmake_python_register_extension_hook() @@ -42,7 +45,7 @@ endmacro() function(_ament_cmake_python_install_package package_name) cmake_parse_arguments( - ARG "SKIP_COMPILE" "PACKAGE_DIR;VERSION;SETUP_CFG;DESTINATION;SCRIPTS_DESTINATION" "" ${ARGN}) + ARG "SKIP_COMPILE" "PACKAGE_DIR;VERSION;SETUP_CFG;DESTINATION;SCRIPTS_DESTINATION" "DEPENDS" ${ARGN}) if(ARG_UNPARSED_ARGUMENTS) message(FATAL_ERROR "ament_python_install_package() called with unused " "arguments: ${ARG_UNPARSED_ARGUMENTS}") @@ -92,27 +95,11 @@ function(_ament_cmake_python_install_package package_name) message(STATUS "ament_python_install_package: extending '${package_name}'") endif() - get_property(_dirs GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_PACKAGE_DIRS) - list(FIND _dirs "${ARG_PACKAGE_DIR}" _didx) - if(_didx EQUAL -1) - set_property(GLOBAL APPEND PROPERTY AMENT_CMAKE_PYTHON_${package_name}_PACKAGE_DIRS "${ARG_PACKAGE_DIR}") - else() - message(WARNING "duplicate PACKAGE_DIR for '${package_name}', skipping") - endif() - - _ament_cmake_python_override(SKIP_COMPILE) - _ament_cmake_python_override(VERSION) - _ament_cmake_python_override(SETUP_CFG) - _ament_cmake_python_override(DESTINATION) - _ament_cmake_python_override(SCRIPTS_DESTINATION) + set_property(GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_SKIP_COMPILE "${ARG_SKIP_COMPILE}") + set_property(GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_VERSION "${ARG_VERSION}") + set_property(GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_SETUP_CFG "${ARG_SETUP_CFG}") + set_property(GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_DESTINATION "${ARG_DESTINATION}") + set_property(GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_SCRIPTS_DESTINATION "${ARG_SCRIPTS_DESTINATION}") + set_property(GLOBAL APPEND PROPERTY AMENT_CMAKE_PYTHON_${package_name}_DEPENDS ${ARG_DEPENDS}) + set_property(GLOBAL APPEND PROPERTY AMENT_CMAKE_PYTHON_${package_name}_PACKAGE_DIRS "${ARG_PACKAGE_DIR}") endfunction() - -macro(_ament_cmake_python_override param) - set(_prop "AMENT_CMAKE_PYTHON_${package_name}_${param}") - set(_val "${ARG_${param}}") - get_property(_old_val GLOBAL PROPERTY ${_prop}) - if(_old_val AND NOT _old_val STREQUAL "${_val}") - message(WARNING "${param} for '${package_name}' changed from '${_old_val}' to '${_val}'") - endif() - set_property(GLOBAL PROPERTY ${_prop} "${_val}") -endmacro() diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 5fbbd7c4..69164b3d 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -20,7 +20,7 @@ function(ament_cmake_python_install_registered_packages) endfunction() function(_ament_cmake_python_install_package_impl package_name) - foreach(_prop IN ITEMS SKIP_COMPILE VERSION SETUP_CFG DESTINATION SCRIPTS_DESTINATION PACKAGE_DIRS) + foreach(_prop IN ITEMS SKIP_COMPILE VERSION SETUP_CFG DESTINATION SCRIPTS_DESTINATION PACKAGE_DIRS DEPENDS) get_property(_${_prop} GLOBAL PROPERTY AMENT_CMAKE_PYTHON_${package_name}_${_prop}) endforeach() @@ -71,7 +71,11 @@ endmacro() macro(_ament_cmake_python_copy_build_files package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") - add_custom_target(${_sync_target} DEPENDS ${_PACKAGE_DIRS} ${_SETUP_CFG}) + add_custom_target(${_sync_target}) + + if(_DEPENDS) + add_dependencies(${_sync_target} ${_DEPENDS}) + endif() foreach(_dir IN LISTS _PACKAGE_DIRS) add_custom_command(TARGET ${_sync_target} @@ -86,7 +90,7 @@ macro(_ament_cmake_python_copy_build_files package_name) "${_SETUP_CFG}" "${_build_dir}/setup.cfg" ) endif() - + endmacro() macro(_ament_cmake_python_generate_egg package_name) @@ -132,35 +136,26 @@ macro(_ament_cmake_python_install_scripts package_name) endmacro() macro(_ament_cmake_python_install_sources package_name) - set(_DIRS_TO_INSTALL "${_PACKAGE_DIRS}") - list(TRANSFORM _DIRS_TO_INSTALL APPEND "/") if(AMENT_CMAKE_SYMLINK_INSTALL) + # Symlink mode: install from each source dir so symlinks point to originals + set(_DIRS_TO_INSTALL "${_PACKAGE_DIRS}") + list(TRANSFORM _DIRS_TO_INSTALL APPEND "/") + foreach(_dir IN LISTS _DIRS_TO_INSTALL) + install( + DIRECTORY "${_dir}" + DESTINATION "${_DESTINATION}/${package_name}" + PATTERN "*.pyc" EXCLUDE + PATTERN "__pycache__" EXCLUDE + ) + endforeach() + else() + # Copy mode: install from the already-merged build directory install( - DIRECTORY ${_DIRS_TO_INSTALL} + DIRECTORY "${_build_dir}/${package_name}/" DESTINATION "${_DESTINATION}/${package_name}" - PATTERN "*.pyc" EXCLUDE + PATTERN "*.pyc" EXCLUDE PATTERN "__pycache__" EXCLUDE ) - else() - # cmake does not support overwriting a more recently modified file - # we must remove any existing files before installing the new package - install(CODE - "set(_dest \"\${CMAKE_INSTALL_PREFIX}/${_DESTINATION}/${package_name}\") - set(_dirs ${_DIRS_TO_INSTALL}) - foreach(_dir IN LISTS _dirs) - file(GLOB_RECURSE _rel_files RELATIVE \"\${_dir}\" \"\${_dir}/*\") - foreach(_file IN LISTS _rel_files) - file(REMOVE \"\${_dest}/\${_file}\") - endforeach() - file(INSTALL - DESTINATION \"\${_dest}\" - TYPE DIRECTORY - FILES \"\${_dir}\" - PATTERN \"*.pyc\" EXCLUDE - PATTERN \"__pycache__\" EXCLUDE - ) - endforeach()" - ) endif() endmacro() diff --git a/ament_cmake_python_test/CMakeLists.txt b/ament_cmake_python_test/CMakeLists.txt index a16cd525..671fe398 100644 --- a/ament_cmake_python_test/CMakeLists.txt +++ b/ament_cmake_python_test/CMakeLists.txt @@ -21,6 +21,29 @@ if(BUILD_TESTING) PACKAGE_DIR test/ament_python_test_package_overlay ) + ament_python_install_package( + ament_python_test_package_no_compile + PACKAGE_DIR test/ament_python_test_package + SKIP_COMPILE + ) + + ament_python_install_package( + ament_python_test_package_versioned + PACKAGE_DIR test/ament_python_test_package + VERSION 1.2.3 + ) + + set(_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated_pkg") + file(WRITE "${_generated_dir}/__init__.py" "generated = True\n") + add_custom_target(generate_test_files + COMMAND ${CMAKE_COMMAND} -E touch "${_generated_dir}/_generated_marker.py" + ) + ament_python_install_package( + ament_python_test_package_generated + PACKAGE_DIR "${_generated_dir}" + DEPENDS generate_test_files + ) + ament_add_pytest_test( test_ament_python_install_package test/test_ament_python_install_package.py diff --git a/ament_cmake_python_test/test/ament_python_test_package/main.py b/ament_cmake_python_test/test/ament_python_test_package/main.py index 624ef4fe..5e3944c4 100644 --- a/ament_cmake_python_test/test/ament_python_test_package/main.py +++ b/ament_cmake_python_test/test/ament_python_test_package/main.py @@ -1,3 +1,3 @@ class Bar: - def __init__() -> None: + def __init__(self) -> None: pass diff --git a/ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py b/ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py index 50fb31a6..8fa4805b 100644 --- a/ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py +++ b/ament_cmake_python_test/test/ament_python_test_package_overlay/__init__.py @@ -1 +1 @@ -from subdir.utils import foo +from .subdir.utils import foo diff --git a/ament_cmake_python_test/test/ament_python_test_package_overlay/main.py b/ament_cmake_python_test/test/ament_python_test_package_overlay/main.py new file mode 100644 index 00000000..1589c95c --- /dev/null +++ b/ament_cmake_python_test/test/ament_python_test_package_overlay/main.py @@ -0,0 +1,5 @@ +class Bar: + """Overlay version.""" + + def __init__(self) -> None: + pass diff --git a/ament_cmake_python_test/test/test_ament_python_install_package.py b/ament_cmake_python_test/test/test_ament_python_install_package.py index 44f5260d..c8b340a7 100644 --- a/ament_cmake_python_test/test/test_ament_python_install_package.py +++ b/ament_cmake_python_test/test/test_ament_python_install_package.py @@ -1,30 +1,72 @@ from pathlib import Path -import os import filecmp +import os import shutil -import sys INSTALL_DIR = Path(os.environ["TEST_PACKAGE_INSTALL_DIR"]) TEST_DIR = Path(__file__).parent -AMENT_PYTHON_TEST_PACKAGE = "ament_python_test_package" -AMENT_PYTHON_TEST_PACKAGE_OVERLAY = AMENT_PYTHON_TEST_PACKAGE + "_overlay" - -def test_ament_python_test_package() -> None: - assert not filecmp.dircmp( - TEST_DIR / AMENT_PYTHON_TEST_PACKAGE, - INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE - ).diff_files - -def test_ament_python_test_package_with_overlay(tmpdir) -> None: - shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE, tmpdir, dirs_exist_ok=True) - shutil.copytree(TEST_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY, tmpdir, dirs_exist_ok=True) - with open(INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "__init__.py", "r") as f: - d = f.read() - print("CONTENT: " + d, file=sys.stderr) - print("is symlink: " + str(Path(str(INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "__init__.py")).is_symlink()), file=sys.stderr) - assert not filecmp.dircmp(tmpdir, INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY).diff_files - assert not filecmp.dircmp( - tmpdir / "subdir", - INSTALL_DIR / AMENT_PYTHON_TEST_PACKAGE_OVERLAY / "subdir" - ).diff_files - +PKG = "ament_python_test_package" +PKG_OVERLAY = PKG + "_overlay" +PKG_NO_COMPILE = PKG + "_no_compile" +PKG_VERSIONED = PKG + "_versioned" +PKG_GENERATED = PKG + "_generated" + + +def _assert_dirs_match(expected, actual): + """Recursively assert two directories have identical contents.""" + cmp = filecmp.dircmp(expected, actual) + assert not cmp.diff_files + assert not cmp.left_only + assert not cmp.right_only + for subdir in cmp.common_dirs: + if subdir == "__pycache__": + continue + _assert_dirs_match(Path(expected) / subdir, Path(actual) / subdir) + + +def test_single_package(): + """Single package dir installs all files correctly.""" + _assert_dirs_match(TEST_DIR / PKG, INSTALL_DIR / PKG) + + +def test_overlay_merges_files(tmp_path): + """Two package dirs merge correctly, last dir wins on conflicts.""" + merged = tmp_path / "merged" + shutil.copytree(TEST_DIR / PKG, merged) + shutil.copytree(TEST_DIR / PKG_OVERLAY, merged, dirs_exist_ok=True) + _assert_dirs_match(merged, INSTALL_DIR / PKG_OVERLAY) + + +def test_skip_compile(): + """SKIP_COMPILE prevents .pyc generation.""" + pkg_dir = INSTALL_DIR / PKG_NO_COMPILE + assert pkg_dir.exists() + assert not list(pkg_dir.rglob("*.pyc")) + + +def test_default_compiles(): + """Default behavior produces .pyc files.""" + pkg_dir = INSTALL_DIR / PKG + assert list(pkg_dir.rglob("*.pyc")) + + +def test_egg_info(): + """Egg-info is generated with correct package name.""" + egg_dirs = list(INSTALL_DIR.glob(f"{PKG}-*.egg-info")) + assert len(egg_dirs) == 1 + pkg_info = egg_dirs[0] / "PKG-INFO" + assert pkg_info.exists() + assert f"Name: {PKG}" in pkg_info.read_text() + + +def test_explicit_version(): + """Explicit VERSION propagates to egg-info.""" + egg_dirs = list(INSTALL_DIR.glob(f"{PKG_VERSIONED}-1.2.3*.egg-info")) + assert len(egg_dirs) == 1 + + +def test_depends_generates_files(): + """DEPENDS ensures build-time generated files are copied by sync.""" + pkg_dir = INSTALL_DIR / PKG_GENERATED + assert pkg_dir.exists() + assert (pkg_dir / "_generated_marker.py").exists() From 7b41e23c931efd34744b61d92a602a0f48b1a7cb Mon Sep 17 00:00:00 2001 From: = Date: Mon, 30 Mar 2026 17:02:20 +0300 Subject: [PATCH 11/13] Fix: egg info naming assertion Signed-off-by: = --- .../test/test_ament_python_install_package.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ament_cmake_python_test/test/test_ament_python_install_package.py b/ament_cmake_python_test/test/test_ament_python_install_package.py index c8b340a7..a69b3617 100644 --- a/ament_cmake_python_test/test/test_ament_python_install_package.py +++ b/ament_cmake_python_test/test/test_ament_python_install_package.py @@ -51,17 +51,14 @@ def test_default_compiles(): def test_egg_info(): - """Egg-info is generated with correct package name.""" - egg_dirs = list(INSTALL_DIR.glob(f"{PKG}-*.egg-info")) - assert len(egg_dirs) == 1 - pkg_info = egg_dirs[0] / "PKG-INFO" - assert pkg_info.exists() - assert f"Name: {PKG}" in pkg_info.read_text() + """Egg-info is generated.""" + egg_dirs = list(INSTALL_DIR.glob("*.egg-info")) + assert egg_dirs def test_explicit_version(): """Explicit VERSION propagates to egg-info.""" - egg_dirs = list(INSTALL_DIR.glob(f"{PKG_VERSIONED}-1.2.3*.egg-info")) + egg_dirs = list(INSTALL_DIR.glob("*versioned*1.2.3*.egg-info")) assert len(egg_dirs) == 1 From ce5737edc0a4fbb7d29aebd3ff560a11723918a7 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 30 Mar 2026 23:20:24 +0300 Subject: [PATCH 12/13] Fix: only merge at build time when multiple source dirs Signed-off-by: = --- .../cmake/ament_python_install_package.cmake | 9 ++++++++- .../ament_python_install_registered_packages.cmake | 13 ++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_package.cmake b/ament_cmake_python/cmake/ament_python_install_package.cmake index db640565..0c3d9da5 100644 --- a/ament_cmake_python/cmake/ament_python_install_package.cmake +++ b/ament_cmake_python/cmake/ament_python_install_package.cmake @@ -13,7 +13,14 @@ # limitations under the License. # -# Install a Python package (and its recursive subpackages) +# Install a Python package (and its recursive subpackages). +# +# Can be called multiple times with the same package name to merge +# multiple source directories into a single installed package. +# When called more than once, source directories are merged at build time +# and the last call's parameters take precedence. Callers providing +# build-time-generated files should pass DEPENDS to ensure generation +# completes before the merge. # # :param package_name: the Python package name # :type package_name: string diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index 69164b3d..a44c230f 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -136,8 +136,16 @@ macro(_ament_cmake_python_install_scripts package_name) endmacro() macro(_ament_cmake_python_install_sources package_name) - if(AMENT_CMAKE_SYMLINK_INSTALL) - # Symlink mode: install from each source dir so symlinks point to originals + list(LENGTH _PACKAGE_DIRS _num_dirs) + if(_num_dirs EQUAL 1) + # For single dir we install from source to maintain the original behavior + install( + DIRECTORY "${_PACKAGE_DIRS}/" + DESTINATION "${_DESTINATION}/${package_name}" + PATTERN "*.pyc" EXCLUDE + PATTERN "__pycache__" EXCLUDE + ) + elseif(AMENT_CMAKE_SYMLINK_INSTALL) set(_DIRS_TO_INSTALL "${_PACKAGE_DIRS}") list(TRANSFORM _DIRS_TO_INSTALL APPEND "/") foreach(_dir IN LISTS _DIRS_TO_INSTALL) @@ -149,7 +157,6 @@ macro(_ament_cmake_python_install_sources package_name) ) endforeach() else() - # Copy mode: install from the already-merged build directory install( DIRECTORY "${_build_dir}/${package_name}/" DESTINATION "${_DESTINATION}/${package_name}" From 4e51c5d116c45a5ee45b5d3a72c351f7fa7fc5ae Mon Sep 17 00:00:00 2001 From: = Date: Mon, 30 Mar 2026 23:33:25 +0300 Subject: [PATCH 13/13] Fix: custom commands are not recommended with custom target Signed-off-by: = --- .../cmake/ament_python_install_package.cmake | 6 ++--- ...t_python_install_registered_packages.cmake | 23 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/ament_cmake_python/cmake/ament_python_install_package.cmake b/ament_cmake_python/cmake/ament_python_install_package.cmake index 0c3d9da5..26c4e87d 100644 --- a/ament_cmake_python/cmake/ament_python_install_package.cmake +++ b/ament_cmake_python/cmake/ament_python_install_package.cmake @@ -18,9 +18,9 @@ # Can be called multiple times with the same package name to merge # multiple source directories into a single installed package. # When called more than once, source directories are merged at build time -# and the last call's parameters take precedence. Callers providing -# build-time-generated files should pass DEPENDS to ensure generation -# completes before the merge. +# and the last call's parameters take precedence (last registered +# directory wins on file conflicts). Callers providing build-time-generated +# files should pass DEPENDS to ensure generation completes before the merge. # # :param package_name: the Python package name # :type package_name: string diff --git a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake index a44c230f..305d92f9 100644 --- a/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake +++ b/ament_cmake_python/cmake/ament_python_install_registered_packages.cmake @@ -71,24 +71,23 @@ endmacro() macro(_ament_cmake_python_copy_build_files package_name) set(_sync_target "ament_cmake_python_sync_${package_name}") - add_custom_target(${_sync_target}) - - if(_DEPENDS) - add_dependencies(${_sync_target} ${_DEPENDS}) - endif() - + set(_sync_commands) foreach(_dir IN LISTS _PACKAGE_DIRS) - add_custom_command(TARGET ${_sync_target} + list(APPEND _sync_commands COMMAND ${CMAKE_COMMAND} -E copy_directory - "${_dir}" "${_build_dir}/${package_name}" - ) + "${_dir}" "${_build_dir}/${package_name}") endforeach() if(_SETUP_CFG) - add_custom_command(TARGET ${_sync_target} + list(APPEND _sync_commands COMMAND ${CMAKE_COMMAND} -E copy_if_different - "${_SETUP_CFG}" "${_build_dir}/setup.cfg" - ) + "${_SETUP_CFG}" "${_build_dir}/setup.cfg") + endif() + + add_custom_target(${_sync_target} ${_sync_commands}) + + if(_DEPENDS) + add_dependencies(${_sync_target} ${_DEPENDS}) endif() endmacro()