Skip to content

Update OS used by the CI#614

Closed
JAuriac wants to merge 1 commit intomainfrom
613-remove-macos-13-from-ci
Closed

Update OS used by the CI#614
JAuriac wants to merge 1 commit intomainfrom
613-remove-macos-13-from-ci

Conversation

@JAuriac
Copy link
Copy Markdown
Contributor

@JAuriac JAuriac commented Dec 12, 2025

Update OS used by the CI, to keep oldest non-EOL Linux and MacOS.

List of things to check before making a PR

Before merging your code, please check the following:

  • you have added a line describing your changes to the Changelog;
  • you have added unit tests for any new or improved feature;
  • In case you updated dependencies, you have checked pdi/docs/CheckList.md
  • you have checked your code format:
    • you have checked that you respect all conventions specified in CONTRIBUTING.md;
    • you have checked that the indentation and formatting conforms to the .clang-format;
    • you have documented with doxygen any new or changed function / class;
  • you have correctly updated the copyright headers:
    • your institution is in the copyright header of every file you (substantially) modified;
    • you have checked that the end-year of the copyright there is the current one;
  • you have updated the AUTHORS file:
    • you have added yourself to the AUTHORS file;
    • if this is a new contribution, you have added it to the AUTHORS file;
  • you have added everything to the user documentation:
    • any new CMake configuration option;
    • any change in the yaml config;
    • any change to the public or plugin API;
    • any other new or changed user-facing feature;
    • any change to the dependencies;
  • you have correctly linked your MR to one or more issues:
    • your MR solves an identified issue;
    • your commit contain the Fix #issue keyword to autoclose the issue when merged.

@JAuriac JAuriac linked an issue Dec 12, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Member

@benoitmartin88 benoitmartin88 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@benoitmartin88 benoitmartin88 left a comment

Choose a reason for hiding this comment

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

Please fix broken ubuntu

@JAuriac
Copy link
Copy Markdown
Contributor Author

JAuriac commented Dec 12, 2025

@benoitmartin88
Copy link
Copy Markdown
Member

@JAuriac Could you take a look at pdidev/test_env#24 ?

@benoitmartin88
Copy link
Copy Markdown
Member

Also, tests are failing on macos-26. @Yushan-Wang Any ideas ?

@Yushan-Wang
Copy link
Copy Markdown
Member

Yushan-Wang commented Dec 15, 2025

Also, tests are failing on macos-26. @Yushan-Wang Any ideas ?

What I found:
Apple has been updating compression libraries in newer macOS versions. The issue is often with zlib version differences (macOS 15+ ships with newer zlib)
Shall we try the macos-latest? or macos-15-intel? @JAuriac

@benoitmartin88
Copy link
Copy Markdown
Member

@Yushan-Wang I think the error is due to something else. I just had a talk with @tpadioleau, and he has a bug on mac os on the same tests. We believe that the test is bugged. The test allocates a double** with multiple calls to new. Later, the double** is exposed in PDI and it is assumed that the data is contiguous, which may not be the case because of the may the double** is allocated.

@benoitmartin88
Copy link
Copy Markdown
Member

For ubuntu/jammy, we are waiting for pdidev/test_env#25

@JAuriac
Copy link
Copy Markdown
Contributor Author

JAuriac commented Dec 16, 2025

@Yushan-Wang I think the error is due to something else. I just had a talk with @tpadioleau, and he has a bug on mac os on the same tests. We believe that the test is bugged. The test allocates a double** with multiple calls to new. Later, the double** is exposed in PDI and it is assumed that the data is contiguous, which may not be the case because of the may the double** is allocated.

Changed the double** for a vector to get continuous memory. May require another PR as this issue may require an independent fix.

Copy link
Copy Markdown
Member

@jbigot jbigot left a comment

Choose a reason for hiding this comment

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

The PR should change the minimum versions in all cmake files & in spack.yaml at the root, ensure that the workarounds in build_and_run_all_tests for older versions are removed and document that in both the changelogs and in doxygen.

Comment thread plugins/decl_hdf5/tests/decl_hdf5_test_deflate.cxx
Comment thread plugins/decl_hdf5/tests/decl_hdf5_test_deflate.cxx
Comment thread CHANGELOG.md Outdated
Comment on lines +18 to +19
* Update OS used by the CI, to keep oldest non-EOL Linux and MacOS.
[#613](https://github.com/pdidev/pdi/issues/613)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please list the impact for users as done in previous situations: list all minimum version changes in the changelog of each subproject.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed from :
CMake 3.16, NetCDF 4.7, Python 3.8, mpi4py 3.0, numpy 1.17, pybind11 2.4
toward :
CMake 3.22, NetCDF 4.9, Python 3.10, mpi4py 3.1, numpy 1.21, pybind11 2.10
to account for oldest non-EOL Ubuntu

Copy link
Copy Markdown
Member

@benoitmartin88 benoitmartin88 left a comment

Choose a reason for hiding this comment

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

Unrelated changes should be in a separate PR and solve an identified issue.

@JAuriac
Copy link
Copy Markdown
Contributor Author

JAuriac commented Dec 17, 2025

@Yushan-Wang I think the error is due to something else. I just had a talk with @tpadioleau, and he has a bug on mac os on the same tests. We believe that the test is bugged. The test allocates a double** with multiple calls to new. Later, the double** is exposed in PDI and it is assumed that the data is contiguous, which may not be the case because of the may the double** is allocated.

Changed the double** for a vector to get continuous memory. May require another PR as this issue may require an independent fix.

Dedicated PR with #628.

Comment thread bin/build_and_run_all_tests Outdated
@JAuriac
Copy link
Copy Markdown
Contributor Author

JAuriac commented Jan 14, 2026

Depends on #630 to pass the "pages" CI.

Check for cmake_minimum_required through the CI, and mention this in CHANGELOG.md
Fix MacOS test_deflate to handle both MacOS-15 (and lesser) and MacOS-26
Update requirements for core; To Do: Update requirement for vendors
@jbigot jbigot force-pushed the 613-remove-macos-13-from-ci branch from 46f1c7f to 0df712e Compare January 20, 2026 19:56
@jbigot
Copy link
Copy Markdown
Member

jbigot commented Jan 24, 2026

Replaced by #637

@jbigot jbigot closed this Jan 24, 2026
@jbigot jbigot deleted the 613-remove-macos-13-from-ci branch January 25, 2026 13:11
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.

Update OS of the CI (remove macos-13 and ubuntu/focal)

4 participants