Skip to content

use --install in installation tests on Windows #1310

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.

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

Merged
merged 1 commit into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 45 additions & 3 deletions .github/workflows/reusable_basic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ jobs:
name: Windows
env:
VCPKG_PATH: "${{github.workspace}}/build/vcpkg/packages/hwloc_x64-windows;${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows"
VCPKG_PATH_BIN: "${{github.workspace}}/build/vcpkg/packages/hwloc_x64-windows/bin;${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows/bin;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows/bin"
strategy:
matrix:
os: ['windows-2019', 'windows-2022']
Expand All @@ -247,6 +248,7 @@ jobs:
shared_library: ['ON', 'OFF']
level_zero_provider: ['ON']
cuda_provider: ['ON']
cmake_ver: ['default']
include:
- os: 'windows-2019'
# clang build fails on Windows 2022
Expand All @@ -256,19 +258,22 @@ jobs:
level_zero_provider: 'ON'
cuda_provider: 'ON'
toolset: "-T ClangCL"
cmake_ver: '3.14.0-win64-x64'
- os: 'windows-2022'
build_type: Release
compiler: {c: cl, cxx: cl}
shared_library: 'ON'
level_zero_provider: 'ON'
cuda_provider: 'ON'
umfd_lib: 'ON'
cmake_ver: '3.28.0-windows-x86_64'
- os: 'windows-2022'
build_type: Release
compiler: {c: cl, cxx: cl}
shared_library: 'ON'
level_zero_provider: 'OFF'
cuda_provider: 'OFF'
cmake_ver: 'default'

runs-on: ${{matrix.os}}

Expand All @@ -278,15 +283,39 @@ jobs:
with:
fetch-depth: 0

- name: Install cmake (non-default version)
if: matrix.cmake_ver != 'default'
run: |
$ErrorActionPreference = "Stop"
$cmakePath = "C:\Program Files\CMake"
if (Test-Path -Path $cmakePath) {
Write-Host "Removing existing CMake installation..."
Remove-Item -Recurse -Force -Path $cmakePath
}
$cmakeInstaller = "cmake-${{matrix.cmake_ver}}.msi"
$cmakeInstallerParts = $cmakeInstaller -split '-|\.'
$cmakeMajorMinorPatch = "$($cmakeInstallerParts[1]).$($cmakeInstallerParts[2]).$($cmakeInstallerParts[3])"
$cmakeUrl = "https://github.com/Kitware/CMake/releases/download/v$cmakeMajorMinorPatch/$cmakeInstaller"
Write-Host "Downloading CMake version ${{matrix.cmake_ver}}..."
Invoke-WebRequest -Uri $cmakeUrl -OutFile $cmakeInstaller -TimeoutSec 360
Write-Host "Installing CMake version ${{matrix.cmake_ver}}..."
Start-Process msiexec.exe -ArgumentList "/i $cmakeInstaller /quiet /norestart" -Wait
cmake --version

- name: Initialize vcpkg
uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5
with:
vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025
vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg
vcpkgJsonGlob: '**/vcpkg.json'

# Install the dependencies and add the bin folders to the PATH for older
# versions of CMake to correctly locate the libraries
- name: Install dependencies
run: vcpkg install --triplet x64-windows
run: |
vcpkg install --triplet x64-windows
$env:Path = "${{env.VCPKG_PATH_BIN}};$env:Path"
echo "PATH=$env:Path" >> $env:GITHUB_ENV
shell: pwsh # Specifies PowerShell as the shell for running the script.

- name: Get UMF version
Expand Down Expand Up @@ -318,10 +347,23 @@ jobs:

- name: Run tests
working-directory: ${{env.BUILD_DIR}}
run: ctest -C ${{matrix.build_type}} --output-on-failure --test-dir test
# For CMake versions < 3.22 we have to add the build directory to the PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

that's actually super crazy, as I can see the same issue would be with examples... so perhaps we should handle this in CMake...? (warn users? fix the paths already in CMake...? enforce newer versions if test/examples are enabled...?)

// no need to do this in this PR - I can see you marked this as TODO in CMake 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the examples - they are just regular executables, so the user needs to take care of the correct PATH, as usual. This issue is related only with ctest, as it contains some 'magic' that sets up paths and runs the tests. Anyway, if we encounter more problems here, I will create a separate PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we use "ENVIRONMENT_MODIFICATION" in examples' CMake, but well... we don't execute them here, so we should be good for now

# manually
run: |
$m = [regex]::Matches((cmake --version), "cmake version (\d+)\.(\d+)\.(\d+)")
if ($m) {
$major = [int]$m.groups[1].Value
$minor = [int]$m.groups[2].Value
if ($major -lt 3 -or ($major -eq 3 -and $minor -lt 22)) {
$env:Path = "${{env.BUILD_DIR}}/bin/${{matrix.build_type}};${{env.BUILD_DIR}}/src/proxy_lib/${{matrix.build_type}};$env:Path"
}
}
ctest -C ${{matrix.build_type}} --output-on-failure --test-dir test
shell: pwsh

- name: Test UMF installation and uninstallation
# The '--shared-library' parameter is added to the installation test when the UMF is built as a shared library
# The '--shared-library' parameter is added to the installation test when
# the UMF is built as a shared library
run: >
python3 ${{github.workspace}}/test/test_installation.py
--build-dir ${{env.BUILD_DIR}}
Expand Down
4 changes: 3 additions & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ function(add_umf_test)
"${DLL_PATH_LIST};PATH=path_list_append:${CMAKE_BINARY_DIR}/bin/;PATH=path_list_append:${CMAKE_BINARY_DIR}/bin/$<CONFIG>/"
)

# append PATH to DLLs
# append PATH to DLLs NOTE: this would work only for the CMake ver >= #
# 3.22. For the older versions, the PATH variable should be set in the
# test script)
set_property(TEST ${TEST_NAME} PROPERTY ENVIRONMENT_MODIFICATION
"${DLL_PATH_LIST}")
endif()
Expand Down
Loading
Loading