Skip to content

Commit

Permalink
Fix Discovery CLI Tool in Windows (#5493)
Browse files Browse the repository at this point in the history
* Refs #22463: Add test to check proper link between python and cpp files

Signed-off-by: cferreiragonz <[email protected]>

* Refs #22463: Remove .bat.in and call .exe from python

Signed-off-by: cferreiragonz <[email protected]>

* Refs #22463: Apply suggestions

Signed-off-by: cferreiragonz <[email protected]>

---------

Signed-off-by: cferreiragonz <[email protected]>
(cherry picked from commit 2cdc2d9)

# Conflicts:
#	tools/fastdds/discovery/parser.py
  • Loading branch information
cferreiragonz authored and mergify[bot] committed Dec 18, 2024
1 parent 7f6cf12 commit 08812bc
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/reusable-mac-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
- name: Install Python dependencies
uses: eProsima/eProsima-CI/multiplatform/install_python_packages@v0
with:
packages: vcstool xmlschema
packages: vcstool xmlschema psutil
upgrade: false

- name: Setup CCache
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/reusable-ubuntu-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ jobs:
- name: Install Python dependencies
uses: eProsima/eProsima-CI/ubuntu/install_python_packages@v0
with:
packages: vcstool xmlschema
packages: vcstool xmlschema psutil

- name: Setup CCache
uses: eProsima/eProsima-CI/external/setup-ccache-action@v0
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/reusable-windows-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ jobs:
- name: Install Python dependencies
uses: eProsima/eProsima-CI/windows/install_python_packages@v0
with:
packages: vcstool xmlschema
packages: vcstool xmlschema psutil

- name: Update known hosts file for DNS resolver testing
if: ${{ !contains(github.event.pull_request.labels.*.name, 'no-test') }}
Expand Down
1 change: 1 addition & 0 deletions test/system/tools/fastdds/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ if(Python3_Interpreter_FOUND)
set(TESTS
test_fastdds_installed
test_fastdds_discovery
test_fastdds_discovery_run
test_ros_discovery
test_fastdds_shm
test_fastdds_xml_validate
Expand Down
44 changes: 44 additions & 0 deletions test/system/tools/fastdds/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,28 @@
Available tests:
test_fastdds_installed
test_fastdds_version
test_fastdds_discovery
test_fastdds_discovery_run
test_fastdds_shm
test_fastdds_xml_validate
test_ros_discovery
"""

import argparse
import os
import subprocess
import signal
import sys
from pathlib import Path
try: # Try catch for new python dependency
import psutil
except ImportError:
print(
'psutil module not found. '
'Try to install running "pip install psutil"')
sys.exit(1)


def setup_script_name():
Expand Down Expand Up @@ -121,6 +133,36 @@ def test_fastdds_discovery(install_path, setup_script_path):
sys.exit(ret)


def test_fastdds_discovery_run(install_path, setup_script_path):
"""Test that discovery command runs."""
args = ' discovery -l 127.0.0.1'
try:
test_timeout = 10
process = subprocess.Popen(
cmd(install_path=install_path,
setup_script_path=setup_script_path,
args=args),
shell=True)
ret = process.wait(timeout=test_timeout)
# Manually set the error return code because we need the process to timeout
ret = 3
except subprocess.TimeoutExpired:
print(f'Timeout {test_timeout} expired. Test successful')
try:
# Need to kill all child processes to properly end the test
parent = psutil.Process(process.pid)
for child in parent.children(recursive=True):
child.terminate()
parent.terminate()
ret = 0
except Exception as e:
print(f"Error while ending child processes: {e}")

if 0 != ret:
print('test_fastdds_discovery_run FAILED')
sys.exit(ret)


def test_ros_discovery(install_path, setup_script_path):
"""Test that discovery command runs."""
args = ' -h'
Expand Down Expand Up @@ -200,6 +242,8 @@ def get_paths(install_path):
lambda: test_fastdds_installed(fastdds_tool_path),
'test_fastdds_discovery': lambda: test_fastdds_discovery(
fastdds_tool_path, setup_script_path),
'test_fastdds_discovery_run': lambda: test_fastdds_discovery_run(
fastdds_tool_path, setup_script_path),
'test_ros_discovery':
lambda: test_ros_discovery(ros_disc_tool_path, setup_script_path),
'test_fastdds_shm': lambda: test_fastdds_shm(fastdds_tool_path),
Expand Down
20 changes: 17 additions & 3 deletions tools/fastdds/discovery/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ def __init__(self, argv):
(len(argv) == 1 and argv[0] == '--help')
):
print(self.__edit_tool_help(result.stdout))
<<<<<<< HEAD
=======
elif (
(len(argv) == 1 and argv[0] == '-v') or
(len(argv) == 1 and argv[0] == '--version')
):
result = subprocess.run([tool_path, '-v'])
if result.returncode != 0:
sys.exit(result.returncode)
>>>>>>> 2cdc2d96 (Fix Discovery CLI Tool in Windows (#5493))
else:
# Call the tool
result = subprocess.run([tool_path] + argv)
Expand Down Expand Up @@ -87,9 +97,13 @@ def __find_tool_path(self):
elif os.name == 'nt':
ret = tool_path / 'fast-discovery-server.exe'
if not os.path.exists(ret):
ret = tool_path / 'fast-discovery-server.bat'
if not os.path.exists(ret):
print('fast-discovery-server tool not installed')
exe_files = [f for f in tool_path.glob('*.exe') if re.match(r'fast-discovery-server.*\.exe$', f.name)]
if len(exe_files) == 0:
print("Unable to find fast-discovery-server tool. Check installation")
elif len(exe_files) == 1:
ret = exe_files[0]
else:
print('Multiple candidates for fast-discovery-server.exe. Check installation')
sys.exit(1)
else:
print(f'{os.name} not supported')
Expand Down
2 changes: 1 addition & 1 deletion tools/fds/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ install(EXPORT ${PROJECT_NAME}-targets
if( WIN32 )
# Use powershell to generate the link
install(
CODE "execute_process( COMMAND PowerShell -Command \"if( test-path ${PROJECT_NAME}.exe -PathType Leaf ) { rm ${PROJECT_NAME}.exe } ; New-Item -ItemType SymbolicLink -Target $<TARGET_FILE_NAME:${PROJECT_NAME}> -Path ${PROJECT_NAME}.exe \" ERROR_QUIET RESULTS_VARIABLE SYMLINK_FAILED WORKING_DIRECTORY \"${CMAKE_INSTALL_PREFIX}/${BIN_INSTALL_DIR}\") \n if( SYMLINK_FAILED ) \n message(STATUS \"Windows requires admin installation rights to create symlinks. A bat script will be provided instead.\") \n set(FAST_SERVER_BINARY_NAME $<TARGET_FILE_NAME:${PROJECT_NAME}>) \n configure_file(${CMAKE_CURRENT_LIST_DIR}/fast-discovery-server.bat.in ${CMAKE_INSTALL_PREFIX}/${BIN_INSTALL_DIR}${MSVCARCH_DIR_EXTENSION}/${PROJECT_NAME}.bat @ONLY) \n endif()"
CODE "execute_process( COMMAND PowerShell -Command \"if( test-path ${PROJECT_NAME}.exe -PathType Leaf ) { rm ${PROJECT_NAME}.exe } ; New-Item -ItemType SymbolicLink -Target $<TARGET_FILE_NAME:${PROJECT_NAME}> -Path ${PROJECT_NAME}.exe \" ERROR_QUIET RESULTS_VARIABLE SYMLINK_FAILED WORKING_DIRECTORY \"${CMAKE_INSTALL_PREFIX}/${BIN_INSTALL_DIR}\") \n if( SYMLINK_FAILED ) \n message(STATUS \"Windows requires admin installation rights to create symlinks. Build again with privileges to create symlink. Tool will try to find executable if no symlink is found.\") \n endif()"
COMPONENT discovery)
else()
# Use ln to create the symbolic link. We remove the version from the file name but keep the debug suffix
Expand Down
15 changes: 0 additions & 15 deletions tools/fds/fast-discovery-server.bat.in

This file was deleted.

0 comments on commit 08812bc

Please sign in to comment.