Skip to content

ament_cmake_vendor: also generate powershell hooks #626

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

Draft
mjcarroll wants to merge 2 commits into
base: rolling
Choose a base branch
from
Draft

ament_cmake_vendor: also generate powershell hooks #626

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
233911d
ament_cmake_vendor: also generate powershell hooks
mjcarroll Jun 4, 2026
27d9782
ament_cmake_vendor: add powershell tests
mjcarroll Jun 4, 2026
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
13 changes: 13 additions & 0 deletions ament_cmake_vendor_package/cmake/ament_vendor.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ macro(ament_vendor TARGET_NAME)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package_cmake_prefix.bat.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package_cmake_prefix.dsv.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package_cmake_prefix.sh.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package_cmake_prefix.ps1.in)
else()
list(APPEND ${PROJECT_NAME}_CONFIG_EXTRAS ${ament_cmake_vendor_package_DIR}/templates/vendor_package_cmake_prefix.cmake.in)
endif()
Expand All @@ -136,6 +137,18 @@ macro(ament_vendor TARGET_NAME)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package.bat.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package.dsv.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package.sh.in)
ament_environment_hooks(${ament_cmake_vendor_package_DIR}/templates/vendor_package.ps1.in)

# Generate and install local_setup.ps1 for PowerShell support on Windows
configure_file(
"${ament_cmake_vendor_package_DIR}/templates/local_setup.ps1.in"
"${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_vendor_package/local_setup.ps1"
@ONLY
)
install(
FILES "${CMAKE_CURRENT_BINARY_DIR}/ament_cmake_vendor_package/local_setup.ps1"
DESTINATION "share/${PROJECT_NAME}"
)
Comment on lines +142 to +151

@cottsay cottsay Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the only section I'm hung up on.

As I understand it, for colcon builds, the colcon-powershell shell extension generates package-level setup scripts today. Supporting Powershell in plain ament builds (like we use for deb and RPM packages) means that ament_cmake should be generating the hooks, either directly as a supported shell or by exposing an extension point for a new ament_cmake_powershell package to implement.

Adding it here, while it makes the test being added pass, doesn't make the other custom hooks in ROS start working as well without a similar change, and I don't think we want to be implicitly creating package-level Powershell setup scripts all over the place.

@cottsay cottsay Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In short, I'm worried that this is a "whack-a-mole" approach to making powershell work better.


# Resource index marker
ament_index_register_resource("vendor_packages" CONTENT "opt/${PROJECT_NAME}")
Expand Down
14 changes: 14 additions & 0 deletions ament_cmake_vendor_package/cmake/templates/local_setup.ps1.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# generated from ament_cmake_vendor_package/cmake/templates/local_setup.ps1.in

# the current prefix is two levels up from the package specific share folder
$env:AMENT_CURRENT_PREFIX=(Get-Item $PSCommandPath).Directory.Parent.Parent.FullName

# source all environment hooks of this package
if (Test-Path "$env:AMENT_CURRENT_PREFIX/share/@PROJECT_NAME@/environment/vendor_package.ps1") {
. "$env:AMENT_CURRENT_PREFIX/share/@PROJECT_NAME@/environment/vendor_package.ps1"
}
if (Test-Path "$env:AMENT_CURRENT_PREFIX/share/@PROJECT_NAME@/environment/vendor_package_cmake_prefix.ps1") {
. "$env:AMENT_CURRENT_PREFIX/share/@PROJECT_NAME@/environment/vendor_package_cmake_prefix.ps1"
}

Remove-Item Env:\AMENT_CURRENT_PREFIX
36 changes: 36 additions & 0 deletions ament_cmake_vendor_package/cmake/templates/vendor_package.ps1.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# generated from ament_cmake_vendor_package/cmake/templates/vendor_package.ps1.in

function ament_vendor_prepend_unique_value {
param (
$_listname,
$_value
)
if (Test-Path Env:$_listname) {
$_values=(Get-Item env:$_listname).Value
} else {
$_values=""
}
$_all_values="$_value"
if ($_values) {
$_values.Split(";") | ForEach-Object {
if ($_) {
if ($_ -ne $_value) {
$_all_values="${_all_values};$_"
}
}
}
}
Set-Item env:\$_listname -Value "$_all_values"
}

if (Test-Path "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/bin") {
ament_vendor_prepend_unique_value PATH "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/bin"
}

if (Test-Path "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/lib") {
ament_vendor_prepend_unique_value @_LIBDIR_VAR_NAME@ "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/lib"
}

if (Test-Path "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/lib64") {
ament_vendor_prepend_unique_value @_LIBDIR_VAR_NAME@ "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@/lib64"
}
28 changes: 28 additions & 0 deletions ament_cmake_vendor_package/cmake/templates/vendor_package_cmake_prefix.ps1.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# generated from ament_cmake_vendor_package/cmake/templates/vendor_package_cmake_prefix.ps1.in

function ament_vendor_prepend_unique_value {
param (
$_listname,
$_value
)
if (Test-Path Env:$_listname) {
$_values=(Get-Item env:$_listname).Value
} else {
$_values=""
}
$_all_values="$_value"
if ($_values) {
$_values.Split(";") | ForEach-Object {
if ($_) {
if ($_ -ne $_value) {
$_all_values="${_all_values};$_"
}
}
}
}
Set-Item env:\$_listname -Value "$_all_values"
}

if (Test-Path "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@") {
ament_vendor_prepend_unique_value CMAKE_PREFIX_PATH "$env:AMENT_CURRENT_PREFIX/opt/@PROJECT_NAME@"
}
22 changes: 22 additions & 0 deletions ament_cmake_vendor_package/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ ament_vendor(depender
SKIP_INSTALL
)

ament_vendor(exlib_install
VCS_URL exlib_good
VCS_TYPE path
GLOBAL_HOOK
)
Comment on lines +29 to +33

@cottsay cottsay Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will result in the call to ament_vendor actually installing the project as part of ament_cmake_vendor_package's installation, which we certainly don't want.

It also means that a call to make install is necessary to run the tests, which is not a standard practice.


externalproject_add_stepdependencies(depender configure exlib_bad exlib_good)

if(WIN32)
Expand All @@ -50,3 +56,19 @@ ament_add_test(layering_bad
COMMAND "${depender_BINDIR}/depender" "should-fail"
GENERATE_RESULT_FOR_RETURN_CODE_ZERO
APPEND_LIBRARY_DIRS "${exlib_bad_LIBDIR}" "${exlib_good_LIBDIR}")

if(WIN32)
configure_file(
test_powershell_setup.ps1
${CMAKE_CURRENT_BINARY_DIR}/test_powershell_setup.ps1
COPYONLY
)

ament_add_test(test_powershell_setup
COMMAND powershell.exe -NonInteractive -ExecutionPolicy Bypass -File
"${CMAKE_CURRENT_BINARY_DIR}/test_powershell_setup.ps1"
"${CMAKE_INSTALL_PREFIX}"
GENERATE_RESULT_FOR_RETURN_CODE_ZERO
)
Comment on lines +67 to +72

@cottsay cottsay Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This assumes that powershell.exe is always available on Windows. Is that true?

endif()

31 changes: 31 additions & 0 deletions ament_cmake_vendor_package/test/test_powershell_setup.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
param (
[string]$InstallPrefix
)

$env:PATH = ""
$env:CMAKE_PREFIX_PATH = ""

$setup_file = "$InstallPrefix/share/ament_cmake_vendor_package_test/local_setup.ps1"
if (-not (Test-Path $setup_file)) {
Write-Error "Could not find local_setup.ps1 at $setup_file"
exit 1
}

# Source the setup file
. $setup_file

Write-Host "PATH: $env:PATH"
Write-Host "CMAKE_PREFIX_PATH: $env:CMAKE_PREFIX_PATH"

if (-not "$env:PATH".Contains("ament_cmake_vendor_package_test")) {
Write-Error "PATH was not updated by local_setup.ps1"
exit 1
}

if (-not "$env:CMAKE_PREFIX_PATH".Contains("ament_cmake_vendor_package_test")) {
Write-Error "CMAKE_PREFIX_PATH was not updated by local_setup.ps1"
exit 1
}

Write-Host "PowerShell local_setup.ps1 verified successfully!"
exit 0