New docker UMF #1298

rbanka1 · 2025-05-06T15:26:57Z

Added UMF docker

// workflow executed on fork:
https://github.com/rbanka1/unified-memory-framework/actions/runs/14862004849/job/41730405890

Checklist

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

CI workflows, not executed per PR (e.g. Nightly), execute properly
New tests added, especially if they will fail without my changes
Added/extended example(s) to cover this functionality
Extended the README/documentation
All newly added source files have a license
All newly added source files are referenced in CMake files
Logger (with debug/info/... messages) is used
All API changes are reflected in docs and def/map files, and are tested

KFilipek · 2025-05-07T13:17:07Z

@lukaszstolarczuk
We have https://github.com/bb-umf account on GitHub.

KFilipek · 2025-05-07T13:28:58Z

.github/docker/ubuntu-20.04.Dockerfile


 # Dependencies for tests (optional)
 ARG TEST_DEPS="\
-	libnuma-dev"
+	libnuma-dev \
+	libtbb-dev\


Scalable pool relies on TBB library (src/pool/pool_scalable.c), so this is an UMF dependency.

The same as for libnuma - as long as you don't use that pool it's not required. Hence, I'd leave as test deps.

KFilipek · 2025-05-07T13:30:33Z

.github/docker/ubuntu-20.04.Dockerfile


 # Dependencies for tests (optional)
 ARG TEST_DEPS="\
-	libnuma-dev"
+	libnuma-dev \


It's requirement for OS memory provider, according to README.md

As mentioned in the readme, it's for testing :)

While I agree with you that for runtime this is a requirement, it is not the hard requirement. As long as you don't use that provider it's not really required, hence we only mark this as "for testing" (or, in the user's case - for the application that uses the provider).

I'd personally leave libnuma as test deps.

KFilipek · 2025-05-07T13:35:11Z

.github/workflows/reusable_basic.yml


 env:
  BUILD_DIR : "${{github.workspace}}/build"
-  INSTL_DIR : "${{github.workspace}}/install-dir"
+  INSTL_DIR : "${{github.workspace}}/../install-dir"


I believe going outside the workspace is not the best idea.

…-framework into newDocker

lukaszstolarczuk · 2025-05-14T11:01:59Z

.github/docker/ubuntu-20.04.Dockerfile

@@ -1,10 +1,10 @@
-# Copyright (C) 2024 Intel Corporation
+# Copyright (C) 2024-2025 Intel Corporation


pls order commits and add a proper commit title/message; while we're adding one new docker, the main change is something else 😉

pls also update the title of the PR, and if ready, please un-draft

lukaszstolarczuk · 2025-05-14T11:03:19Z

.github/docker/ubuntu-22.04.Dockerfile

@@ -20,21 +20,24 @@ ENV DEBIAN_FRONTEND noninteractive
 ARG BASE_DEPS="\
 	build-essential \
 	cmake \
-	git"
+	git \
+	libtool"


we don't install hwloc from source on this image, libtool seems unneeded here...?

lukaszstolarczuk · 2025-05-14T11:04:37Z

.github/docker/ubuntu-20.04.Dockerfile

-	libtbb-dev"
+# Hwloc installation dependencies
+ARG HWLOC_DEPS="\
+	dos2unix \


do we need this?

lukaszstolarczuk · 2025-05-14T11:05:16Z

.github/docker/ubuntu-20.04.Dockerfile

+
+# Copy hwloc
+# libhwloc-dev is required - installed via script because hwloc version is to old on this OS
+COPY .github/scripts/install_hwloc.sh /opt/umf/install_hwloc.sh


pls move this COPY step after all ARGs

lukaszstolarczuk · 2025-05-14T11:05:33Z

.github/docker/ubuntu-20.04.Dockerfile

+	libtool"
+
+# Copy hwloc
+# libhwloc-dev is required - installed via script because hwloc version is to old on this OS


is too old

lukaszstolarczuk · 2025-05-14T11:14:46Z

.github/workflows/reusable_basic.yml

-      if: matrix.compiler.cxx == 'g++-7'
-      run: sudo apt-get install -y ${{matrix.compiler.cxx}}
-
-    - name: Get UMF version


why this step is removed?

lukaszstolarczuk · 2025-05-14T11:14:53Z

.github/workflows/reusable_basic.yml

@@ -192,7 +166,6 @@ jobs:
        -DUMF_DISABLE_HWLOC=${{matrix.disable_hwloc}}
        -DUMF_LINK_HWLOC_STATICALLY=${{matrix.link_hwloc_statically}}
        ${{ matrix.build_type == 'Debug' && matrix.compiler.c == 'gcc' && '-DUMF_USE_COVERAGE=ON' || '' }}
-        ${{ matrix.llvm_linker || '' }}


why this is removed?

lukaszstolarczuk · 2025-05-14T11:15:29Z

.github/workflows/reusable_basic.yml

@@ -205,11 +178,14 @@ jobs:
        ${{ matrix.compiler.cxx == 'icpx' && '. /opt/intel/oneapi/setvars.sh' || true }}
        LD_LIBRARY_PATH="${{env.BUILD_DIR}}/lib/:${LD_LIBRARY_PATH}" ctest --output-on-failure

+    - name: Set OS name
+      run: echo "OS_VER=ubuntu-${{ matrix.ubuntu_ver }}" >> $GITHUB_ENV


we set OS and OS_VER already within docker image

lukaszstolarczuk · 2025-05-14T11:19:16Z

.github/workflows/reusable_basic.yml

    - name: Check coverage
      if:  ${{ matrix.build_type == 'Debug' && matrix.compiler.c == 'gcc' }}
      working-directory: ${{env.BUILD_DIR}}
      run: |
-        export COVERAGE_FILE_NAME=${{env.COVERAGE_NAME}}-${{matrix.os}}-shared-${{matrix.shared_library}}-no_hwloc-${{matrix.disable_hwloc}}
+        export COVERAGE_FILE_NAME=${{env.COVERAGE_NAME}}-${{env.OS_VER}}-shared-${{matrix.shared_library}}-no_hwloc-${{matrix.disable_hwloc}}


can't we still use matrix.os here?

lukaszstolarczuk · 2025-05-14T11:22:10Z

.github/workflows/reusable_basic.yml

    - name: Install oneAPI basekit
      if: matrix.compiler.cxx == 'icpx'
      run: |
+        sudo apt-get update


pls note sudo is probably not passwordless on the docker, so e.g. use syntax like here: https://github.com/pmem/pmemstream/blob/master/utils/docker/prepare-for-build.sh#L20

rbanka1 added 2 commits May 5, 2025 15:58

add dockers

b668842

fix

c2d35fc

KFilipek requested changes May 8, 2025

View reviewed changes

rbanka1 added 2 commits May 8, 2025 11:22

test

ef8733e

Merge branch 'main' into newDocker

fd055d3

lukaszstolarczuk mentioned this pull request May 8, 2025

Docker workflow #1105

Closed

10 tasks

rbanka1 added 5 commits May 13, 2025 09:51

separated detect changes - test

5bc7506

Merge branch 'newDocker' of https://github.com/rbanka1/unified-memory…

4b44fe4

…-framework into newDocker

Merge branch 'main' into newDocker

b442229

separated detect changes - test

e14f45e

Merge branch 'newDocker' of https://github.com/rbanka1/unified-memory…

ae65dbf

…-framework into newDocker

rbanka1 force-pushed the newDocker branch from 00e2a5d to 6039c67 Compare May 13, 2025 11:54

separated detect changes

b1d2ec2

rbanka1 force-pushed the newDocker branch from 6039c67 to b1d2ec2 Compare May 13, 2025 11:58

lukaszstolarczuk reviewed May 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New docker UMF #1298

New docker UMF #1298

rbanka1 commented May 6, 2025 •

edited by lukaszstolarczuk

Loading

KFilipek commented May 7, 2025 •

edited

Loading

KFilipek May 7, 2025

lukaszstolarczuk May 8, 2025

KFilipek May 7, 2025

lukaszstolarczuk May 8, 2025

KFilipek May 7, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

lukaszstolarczuk May 14, 2025

		@@ -1,10 +1,10 @@
		# Copyright (C) 2024 Intel Corporation
		# Copyright (C) 2024-2025 Intel Corporation

New docker UMF #1298

Are you sure you want to change the base?

New docker UMF #1298

Conversation

rbanka1 commented May 6, 2025 • edited by lukaszstolarczuk Loading

Checklist

KFilipek commented May 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbanka1 commented May 6, 2025 •

edited by lukaszstolarczuk

Loading

KFilipek commented May 7, 2025 •

edited

Loading