Skip to content

Add test for FetchContent of UMF on Linux and Windows #1384

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ldorau
Copy link
Contributor

@ldorau ldorau commented Jun 23, 2025

Description

Test FetchContent of UMF on Linux and Windows -
on Windows with the 'Ninja' and 'NMake Makefiles' generators.

Add the fetch_content example based on the basic example.

Nightly CI build: https://github.com/ldorau/unified-memory-framework/actions/runs/16030235338

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau ldorau requested a review from a team as a code owner June 23, 2025 11:23
@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch 4 times, most recently from afd45b5 to cd0640d Compare June 23, 2025 12:10
@ldorau
Copy link
Contributor Author

ldorau commented Jun 23, 2025

@ldorau
Copy link
Contributor Author

ldorau commented Jun 24, 2025

@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from cd0640d to 09fb801 Compare June 24, 2025 11:08
@ldorau
Copy link
Contributor Author

ldorau commented Jun 24, 2025

@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 09fb801 to 8bb3ebc Compare June 27, 2025 09:09
@ldorau
Copy link
Contributor Author

ldorau commented Jun 27, 2025

Rebased

@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 8bb3ebc to 9906125 Compare June 30, 2025 07:09
@ldorau
Copy link
Contributor Author

ldorau commented Jun 30, 2025

Rebased

@ldorau ldorau changed the title Test FetchContent of UMF on Linux and Windows Add tests for FetchContent of UMF on Linux and Windows Jun 30, 2025
@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 9906125 to 4e4194a Compare June 30, 2025 07:51
@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 4e4194a to 64a6257 Compare June 30, 2025 10:16
@ldorau ldorau requested a review from bratpiorka June 30, 2025 10:18
@ldorau ldorau changed the title Add tests for FetchContent of UMF on Linux and Windows Add test for FetchContent of UMF on Linux and Windows Jun 30, 2025
run: ./umf_example_fetch_content

# Build and test the fetch_content example with different CMake generators on Windows
Windows-FetchContent:
Copy link
Contributor

Choose a reason for hiding this comment

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

windows job could be definitely merged into existing nmake/ninja workflows in nigthly; not sure about linux job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,64 @@
# Copyright (C) 2025 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

a general note, not neccessarily needed to be done in this PR - so now, as we test it, perhaps we should add a README section about integration (e.g. like in UR repo: https://github.com/oneapi-src/unified-runtime?tab=readme-ov-file#integration)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can consider that. The question is if it should be the recommended way to integrate UMF.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm yeah, probably we shouldn't say that it's recommended

@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 64a6257 to b28de77 Compare July 2, 2025 13:42
@ldorau
Copy link
Contributor Author

ldorau commented Jul 2, 2025

Test FetchContent of UMF on Linux and Windows in Nightly CI job
- on Windows with the 'Ninja' and 'NMake Makefiles' generators.

Add the fetch_content example based on the basic example.

Signed-off-by: Lukasz Dorau <[email protected]>
@ldorau ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from b28de77 to e2286df Compare July 2, 2025 16:07
@ldorau ldorau requested a review from lukaszstolarczuk July 2, 2025 16:14
Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

in general LGTM

- name: Run the fetch_content example
shell: cmd
working-directory: ${{github.workspace}}/examples/fetch_content/build
run: dir .\umf_example_fetch_content.exe
Copy link
Contributor

Choose a reason for hiding this comment

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

why dir?


- name: Build the fetch_content example
working-directory: ${{github.workspace}}/examples/fetch_content
shell: cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

default powershell should be ok (just %NUMBER_OF_PROCESSORS% -> $env:NUMBER_OF_PROCESSORS)

run: cmake --build ${{github.workspace}}/examples/fetch_content/build --config ${{matrix.build_type}} -j %NUMBER_OF_PROCESSORS%

- name: Run the fetch_content example
shell: cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

default powershell should be ok

@@ -201,6 +238,32 @@ jobs:
${{ matrix.umfd_lib == 'ON' && '--umfd-lib' || ''}}
${{ matrix.static_hwloc == 'ON' && '--hwloc' || '' }}

- name: Set VCPKG_PATH with hwloc for the fetch_content example
Copy link
Contributor

Choose a reason for hiding this comment

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

we already have a step like this, I guess we could just do the same as in that step: add if: matrix.static_hwloc == 'OFF' to the fetch_content example's configure/build/execution steps

# Copyright (C) 2025 Intel Corporation
# Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

Copy link
Contributor

Choose a reason for hiding this comment

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

you could add a short description why do we have this example

@@ -0,0 +1,64 @@
# Copyright (C) 2025 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm yeah, probably we shouldn't say that it's recommended

@@ -107,6 +107,43 @@ jobs:
- name: Run tests under valgrind
run: ${{github.workspace}}/test/test_valgrind.sh ${{github.workspace}} ${{github.workspace}}/build ${{matrix.tool}}

Linux-FetchContent:
Copy link
Contributor

Choose a reason for hiding this comment

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

I can you can drop Linux workflow, if you add this example to standalones execution

in test/CMakeLists.txt
pls add new example in line
set(EXAMPLES ${EXAMPLES} basic custom_file_provider)

// tested on my fork: https://github.com/lukaszstolarczuk/unified-memory-framework/actions/runs/16051712582/job/45295762065#step:11:4294

target_include_directories(${EXAMPLE_NAME} PRIVATE ${LIBUMF_INCLUDE_DIRS})
target_link_directories(${EXAMPLE_NAME} PRIVATE ${LIBHWLOC_LIBRARY_DIRS})
target_link_libraries(${EXAMPLE_NAME} PRIVATE ${LIBUMF_LIBRARIES} hwloc)

Copy link
Contributor

Choose a reason for hiding this comment

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

could you please add printing of PATHs found in this example and used above..?

// e.g. like here: https://github.com/lukaszstolarczuk/unified-memory-framework/actions/runs/16051712582/job/45295762065#step:11:4353

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.

3 participants