Skip to content

adios2: doCheck with ctestCheckHook#414597

Merged
NickCao merged 9 commits intoNixOS:masterfrom
qbisi:adios2
Jun 17, 2025
Merged

adios2: doCheck with ctestCheckHook#414597
NickCao merged 9 commits intoNixOS:masterfrom
qbisi:adios2

Conversation

@qbisi
Copy link
Contributor

@qbisi qbisi commented Jun 6, 2025

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. labels Jun 6, 2025
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 you'd also need to use dontUseNinjaCheck = true for the hook to work, since the derivation uses ninja.

Worth noting though that there doesn't seem to be a use-case for it here. enableParallelChecking also works with plain make/ninja check phase since CMake hook also sets CTEST_PARALLEL_LEVEL env variable and that gets picked up by ctest. Its primary use is to pass additional flags / selectively disable tests in a more ergonomic manner.

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 was meant to use ctestCheckHook to disable some tests failure on x86_64-linux platform, however i managed to fix thouse tests failure. I am opening this pr to see if there exist any tests failure on other platform and would like to disable thouse test failure.

@qbisi qbisi force-pushed the adios2 branch 3 times, most recently from 1104d1e to 5e1c292 Compare June 6, 2025 23:50
@qbisi qbisi marked this pull request as ready for review June 7, 2025 01:36
@nix-owners nix-owners bot requested a review from natsukium June 7, 2025 01:38
Comment on lines 141 to 147
Copy link
Contributor

Choose a reason for hiding this comment

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

Why disable BUILD_TESTING and then reenable it conditionally? Can't this just be this:

Suggested change
(lib.cmakeBool "BUILD_TESTING" false)
# use vendored gtest as nixpkgs#gtest does not include <iomanip> in <gtest/gtest.h>
(lib.cmakeBool "ADIOS2_USE_EXTERNAL_GTEST" false)
(lib.cmakeBool "BUILD_TESTING" finalAttrs.finalPackage.doCheck)
# use vendored gtest as nixpkgs#gtest does not include <iomanip> in <gtest/gtest.h>
(lib.cmakeBool "ADIOS2_USE_EXTERNAL_GTEST" false)
(lib.cmakeBool "BUILD_TESTING" finalAttrs.finalPackage.doCheck)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for fixing me, i just forgot to delete that line.

@wolfgangwalther
Copy link
Contributor

Re-running CI due to a odd failure fixed in #416448

@qbisi
Copy link
Contributor Author

qbisi commented Jun 13, 2025

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 414597

Logs: https://github.com/qbisi/nixpkgs-review-gha/actions/runs/15645253320


x86_64-linux (sandbox = true)

✅ 8 packages built:
  • adios2
  • dolfinx
  • python312Packages.adios2
  • python312Packages.fenics-dolfinx
  • python312Packages.fenics-dolfinx.dist
  • python313Packages.adios2
  • python313Packages.fenics-dolfinx
  • python313Packages.fenics-dolfinx.dist

aarch64-linux (sandbox = true)

✅ 8 packages built:
  • adios2
  • dolfinx
  • python312Packages.adios2
  • python312Packages.fenics-dolfinx
  • python312Packages.fenics-dolfinx.dist
  • python313Packages.adios2
  • python313Packages.fenics-dolfinx
  • python313Packages.fenics-dolfinx.dist

x86_64-darwin (sandbox = true)

✅ 3 packages built:
  • adios2
  • python312Packages.adios2
  • python313Packages.adios2

aarch64-darwin (sandbox = true)

✅ 8 packages built:
  • adios2
  • dolfinx
  • python312Packages.adios2
  • python312Packages.fenics-dolfinx
  • python312Packages.fenics-dolfinx.dist
  • python313Packages.adios2
  • python313Packages.fenics-dolfinx
  • python313Packages.fenics-dolfinx.dist

@github-actions github-actions bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 14, 2025
@NickCao NickCao merged commit b7d45a0 into NixOS:master Jun 17, 2025
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants