Fix Discovery CLI Tool in Windows (No privileges) (backport #5493) (#…

…5510) * Fix Discovery CLI Tool in Windows (#5493) * 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 * Fix conflicts Signed-off-by: Javier Gil Aviles <[email protected]> --------- Signed-off-by: Javier Gil Aviles <[email protected]> Co-authored-by: Carlos Ferreira González <[email protected]> Co-authored-by: Javier Gil Aviles <[email protected]>
eProsima · Feb 4, 2025 · 1d354e3 · 1d354e3
1 parent 42b67a0
commit 1d354e3
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 22 deletions.
diff --git a/.github/workflows/reusable-mac-ci.yml b/.github/workflows/reusable-mac-ci.yml
@@ -77,7 +77,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

diff --git a/.github/workflows/reusable-ubuntu-ci.yml b/.github/workflows/reusable-ubuntu-ci.yml
@@ -200,7 +200,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

diff --git a/.github/workflows/reusable-windows-ci.yml b/.github/workflows/reusable-windows-ci.yml
@@ -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') }}

diff --git a/test/system/tools/fastdds/CMakeLists.txt b/test/system/tools/fastdds/CMakeLists.txt
@@ -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

diff --git a/test/system/tools/fastdds/tests.py b/test/system/tools/fastdds/tests.py
@@ -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():
@@ -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'
@@ -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),

diff --git a/tools/fastdds/discovery/parser.py b/tools/fastdds/discovery/parser.py
@@ -54,6 +54,13 @@ def __init__(self, argv):
                     (len(argv) == 1 and argv[0] == '--help')
                ):
                 print(self.__edit_tool_help(result.stdout))
+            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)
             else:
                 # Call the tool
                 result = subprocess.run([tool_path] + argv)
@@ -87,9 +94,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')

diff --git a/tools/fds/CMakeLists.txt b/tools/fds/CMakeLists.txt
@@ -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

diff --git a/tools/fds/fast-discovery-server.bat.in b/tools/fds/fast-discovery-server.bat.in