X Tutup
Skip to content

Upgrade CGNS, meson, and ninja#2531

Merged
bmunguia merged 12 commits intodevelopfrom
fix_cgns_build
Jul 14, 2025
Merged

Upgrade CGNS, meson, and ninja#2531
bmunguia merged 12 commits intodevelopfrom
fix_cgns_build

Conversation

@bmunguia
Copy link
Member

@bmunguia bmunguia commented Jul 7, 2025 •
edited
Loading

Proposed Changes

Upgrade external CGNS library to v4.5, fixing a compilation issue on Linux.

Related Work

#2409: Fixes compilation issue 1 (undefined _POSIX_SOURCE in CGNS library)

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@bmunguia
Copy link
Member Author

bmunguia commented Jul 7, 2025

meson is generating warnings for undefined configuration variables related to #define directives

Configuring cgnstypes.h using configuration
externals/cgns/meson.build:49: WARNING: The variable(s) 'BUILD64BIT', 'BUILDBASESCOPE', 'BUILDHDF5', 'BUILDLEGACY', 'BUILDSCOPE', 'CGLONGT', 'CGULONGT', 'FORTRAN_DEFAULT_INTEGER_C_INT64_T', 'HAVE_COMPLEX_C99_EXT', 'HAVE_STAT64_STRUCT', 'HDF5_HAVE_FILE_SPACE_STRATEGY', 'HDF5_HAVE_MULTI_DATASETS' in the input file 'externals/cgns/cgnstypes.h.in' are not present in the given configuration data.
Configuring cg_hash_types.h using configuration
externals/cgns/meson.build:53: WARNING: The variable(s) 'BUILD64BIT' in the input file 'externals/cgns/cg_hash_types.h.in' are not present in the given configuration data.
Configuring cgnsconfig.h using configuration
externals/cgns/meson.build:57: WARNING: The variable(s) 'BUILDBASESCOPE', 'BUILDDEBUG', 'BUILDFORTRAN', 'H5NEEDMPI', 'H5NEEDSZIP', 'H5NEEDZLIB', 'HDF5INC', 'HDF5LIB', 'MPIINC', 'MPILIBS', 'SZIPLIB', 'ZLIBLIB' in the input file 'externals/cgns/cgnsconfig.h.in' are not present in the given configuration data.

The values were previously hard-coded e.g.

diff --git a/externals/cgns/cgnstypes.h.in b/externals/cgns/cgnstypes.h.in
index 0da04c11d8..f91955f646 100644
--- a/externals/cgns/cgnstypes.h.in
+++ b/externals/cgns/cgnstypes.h.in
@@ -29,21 +29,20 @@
 #ifndef CGNSTYPES_H
 #define CGNSTYPES_H
 
-#define CG_BUILD_HDF5   1
-#define CG_BUILD_LEGACY 0
-#define CG_BUILD_64BIT  1
-#define CG_BUILD_SCOPE  0
-#define CG_BUILD_BASESCOPE 0
+#define CG_BUILD_HDF5   @BUILDHDF5@
+#define CG_BUILD_LEGACY @BUILDLEGACY@
+#define CG_BUILD_64BIT  @BUILD64BIT@
+#define CG_BUILD_SCOPE  @BUILDSCOPE@
+#define CG_BUILD_BASESCOPE @BUILDBASESCOPE@
 #define CG_BUILD_PARALLEL  @BUILDPARALLEL@
-#define CG_BUILD_COMPLEX_C99_EXT 1
-
+#define CG_BUILD_COMPLEX_C99_EXT @HAVE_COMPLEX_C99_EXT@
 
 #define CG_MAX_INT32 0x7FFFFFFF
-#ifdef _WIN32
-# define CG_LONG_T __int64
-#else
-# define CG_LONG_T long
-#endif
+
+#include <stdint.h>   /* For C9x types */
+#include <inttypes.h> /* C99/POSIX.1 header for uint64_t, PRIu64 */
+#define CG_LONG_T @CGLONGT@
+#define CG_U_LONG_T @CGULONGT@
 
 #if CG_BUILD_HDF5

Should I just set these in meson.build to their old values?

@bmunguia bmunguia changed the title Upgrade to CGNS v4.5 [WIP] Upgrade to CGNS v4.5 Jul 7, 2025
@pcarruscag
Copy link
Member

pcarruscag commented Jul 8, 2025

Yep if they require those flags then using the old values we had sounds reasonable

@bmunguia
Copy link
Member Author

bmunguia commented Jul 8, 2025 •
edited
Loading

While I'm at it, I get an error building ninja with my system's python (3.13)

ninja executable not found. Building ...
Traceback (most recent call last):
  File "/home/brian/SU2/externals/ninja/configure.py", line 26, in <module>
    import pipes
ModuleNotFoundError: No module named 'pipes'

I've been installing in a virtual environment with python 3.12 which builds successfully but gives a deprecation warning

ninja executable not found. Building ...
/home/brian/SU2/externals/ninja/configure.py:26: DeprecationWarning: 'pipes' is deprecated and slated for removal in Python 3.13
  import pipes

This has already been fixed by ninja-build/ninja#2340 almost 2 years ago, but we're using a 5 year old version of ninja. Should I go ahead and update meson/ninja?

@pcarruscag
Copy link
Member

pcarruscag commented Jul 8, 2025

Go for it, we also changed to c++17 recently, we might make someone unhappy 🤣

@bmunguia
Copy link
Member Author

bmunguia commented Jul 9, 2025

Some of the regressions are failing after updating meson/ninja even though they all passed after updating CGNS. The tests that failed are

  • Tutorials: propeller_variable_load
  • Serial Regression: naca0012_geo, shape_opt_euler_py
  • Parallel Regression: fsi2d
  • Address Sanitizer Tests: naca0012_geo

propeller_variable_load and fsi2d both produce out of bounds access errors:

/usr/include/c++/15/bits/stl_vector.h:1263: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator [with _Tp = double; _Alloc = std::allocator; reference = double&; size_type = long unsigned int]: Assertion '__n < this->size()' failed.
/usr/include/c++/15/bits/stl_vector.h:1263: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator [with _Tp = double; _Alloc = std::allocator; reference = double&; size_type = long unsigned int]: Assertion '__n < this->size()' failed.

The other 2 tests fail running SU2_GEO

pcarruscag
pcarruscag reviewed Jul 9, 2025
View reviewed changes
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

We should not be seeing asserts in release builds, so something fishy is happening.

@bmunguia
Copy link
Member Author

bmunguia commented Jul 9, 2025

It looks like meson changed how assertions are controlled in v1.4. If b_ndebug==false (default false), it enables debugging assertions:

For GCC, -D_GLIBCXX_ASSERTIONS=1 is set.

For Clang, -D_GLIBCXX_ASSERTIONS=1 is set to cover libstdc++ usage, and -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE or -D_LIBCPP_ENABLE_ASSERTIONS=1 is used depending on the Clang version.

So now I set b_ndebug=true by default in meson.build (which sets -DNDEBUG) and undefine NDEBUG for debug builds, which should result in the pre-upgrade behavior (and seemed to fix the regressions).

@bmunguia bmunguia changed the title [WIP] Upgrade to CGNS v4.5 Upgrade to CGNS v4.5 Jul 9, 2025
@bmunguia bmunguia changed the title Upgrade to CGNS v4.5 Upgrade CGNS, meson, and ninja Jul 9, 2025
@bmunguia bmunguia marked this pull request as ready for review July 10, 2025 00:00
@bmunguia bmunguia mentioned this pull request Jul 10, 2025
Merged
pcarruscag
pcarruscag reviewed Jul 14, 2025
View reviewed changes
@pcarruscag
Copy link
Member

pcarruscag commented Jul 14, 2025

You need to install the precommit hooks to solve the formatting issues.
If the issues are in the CGNS files, you need to exclude them from formatting.
More info here #1948 and in the dev docs page.

@bmunguia bmunguia merged commit 5864796 into develop Jul 14, 2025
35 of 37 checks passed
@bmunguia bmunguia deleted the fix_cgns_build branch July 14, 2025 20:48
@pcarruscag
Copy link
Member

pcarruscag commented Jul 16, 2025

Are you working on fixing this @bmunguia? -> https://github.com/su2code/SU2/actions/runs/16277520538

@bmunguia bmunguia restored the fix_cgns_build branch July 16, 2025 15:57
This was referenced Jul 17, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcarruscag pcarruscag pcarruscag approved these changes

Assignees

No one assigned

Labels

bug changelog:fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bmunguia @pcarruscag
X Tutup