Skip to content

ament_cmake_vendor: also generate powershell hooks#626

Draft
mjcarroll wants to merge 2 commits into
rollingfrom
mjcarroll/vendor_powershell
Draft

ament_cmake_vendor: also generate powershell hooks#626
mjcarroll wants to merge 2 commits into
rollingfrom
mjcarroll/vendor_powershell

Conversation

@mjcarroll

@mjcarroll mjcarroll commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

When generating hooks for vendor packages, also generate hooks for powershell (because CMD is kind of miserable).

Closes #611

Assisted-by: Antigravity antigravity@google.com

Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
@mjcarroll
mjcarroll requested a review from cottsay June 4, 2026 14:59
@mjcarroll mjcarroll self-assigned this Jun 4, 2026
@mjcarroll
mjcarroll marked this pull request as draft June 4, 2026 15:10
Add a test script and target to verify the environment variables prepended
by the generated powershell scripts.

Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
Comment on lines +142 to +151
# 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}"
)

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.

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.

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

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.

Comment on lines +67 to +72
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
)

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?

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.

ament_cmake_vendor_package should have powershell (ps1) templates

2 participants