-
Notifications
You must be signed in to change notification settings - Fork 0
ci: make CI runnable on self-hosted runners (orphan-container fix, distutils, drop ubuntu20) #1
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
Changes from all commits
b02376b
de73d29
26993e6
c00a0ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,12 @@ const test_timeout = core.getInput('test-timeout', {required: true}); | |
| const repo_name = process.env.GITHUB_REPOSITORY.split('/')[1]; | ||
|
|
||
| try { | ||
| // Defensively remove any leftover container from a prior/interrupted run. On a | ||
| // persistent self-hosted runner the Docker daemon survives between jobs, and the | ||
| // per-test containers below use fixed names with no --rm, so an orphan would block | ||
| // name reuse with "container name already in use". (ENF's ephemeral runners get a | ||
| // fresh daemon each job and never hit this.) | ||
| child_process.spawnSync("docker", ["rm", "-f", "base"], {stdio:"ignore"}); | ||
| if(child_process.spawnSync("docker", ["run", "--name", "base", "-v", `${process.cwd()}/build.tar.zst:/build.tar.zst`, "--workdir", `/__w/${repo_name}/${repo_name}`, container, "sh", "-c", "zstdcat /build.tar.zst | tar x"], {stdio:"inherit"}).status) | ||
| throw new Error("Failed to create base container"); | ||
| if(child_process.spawnSync("docker", ["commit", "base", "baseimage"], {stdio:"inherit"}).status) | ||
|
|
@@ -29,6 +35,8 @@ try { | |
|
|
||
| let subprocesses = []; | ||
| tests.forEach(t => { | ||
| // Clear any orphaned container of this name before reusing it (see note above). | ||
| child_process.spawnSync("docker", ["rm", "-f", t.name], {stdio:"ignore"}); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When two matrix jobs using this action land on runners that share the same Docker daemon, the fixed ctest-derived container names overlap across platforms, and Useful? React with 👍 / 👎. |
||
| subprocesses.push(new Promise(resolve => { | ||
| child_process.spawn("docker", ["run", "--security-opt", "seccomp=unconfined", "-e", "GITHUB_ACTIONS=True", "--name", t.name, "--init", "baseimage", "bash", "-c", `cd build; ctest --output-on-failure -R '^${t.name}$' --timeout ${test_timeout}`], {stdio:"inherit"}).on('close', code => resolve(code)); | ||
| })); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,9 @@ set(CPACK_DEBIAN_BASE_FILE_NAME "${CPACK_DEBIAN_FILE_NAME}.deb") | |
| string(REGEX REPLACE "^(${CMAKE_PROJECT_NAME})" "\\1-dev" CPACK_DEBIAN_DEV_FILE_NAME "${CPACK_DEBIAN_BASE_FILE_NAME}") | ||
|
|
||
| #deb package tooling will be unable to detect deps for the dev package. llvm is tricky since we don't know what package could have been used; try to figure it out | ||
| set(CPACK_DEBIAN_DEV_PACKAGE_DEPENDS "libgmp-dev, python3-distutils, python3-numpy, zlib1g-dev") | ||
| # NOTE: python3-distutils was dropped here — it was removed from Python 3.12 / Ubuntu 24.04+ | ||
| # (so the dev package became uninstallable there) and nothing in spring or TestHarness uses it. | ||
| set(CPACK_DEBIAN_DEV_PACKAGE_DEPENDS "libgmp-dev, python3-numpy, zlib1g-dev") | ||
|
Comment on lines
+65
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While removing if sys.version_info < (3, 10, 0):
from distutils import sysconfig
...If this To safely drop the import sysconfig
try:
print(sysconfig.get_path('platlib', 'deb_system'))
except KeyError:
print(sysconfig.get_path('platlib'))This is compatible with all Python 3 versions and avoids any dependency on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The dev package still installs Useful? React with 👍 / 👎. |
||
| find_program(DPKG_QUERY "dpkg-query") | ||
| if(DPKG_QUERY AND OS_RELEASE MATCHES "\n?ID=\"?ubuntu" AND LLVM_CMAKE_DIR) | ||
| execute_process(COMMAND "${DPKG_QUERY}" -S "${LLVM_CMAKE_DIR}" COMMAND cut -d: -f1 RESULT_VARIABLE LLVM_PKG_FIND_RESULT OUTPUT_VARIABLE LLVM_PKG_FIND_OUTPUT) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spawning a new process synchronously inside a
forEachloop blocks the Node.js event loop. If there are many tests, executingdocker rm -fsequentially for each test will introduce significant synchronous overhead and delay the start of all test containers.Since
docker rmsupports removing multiple containers in a single command, we can batch this operation into a singlespawnSynccall before the loop. This is much more efficient.