-
Notifications
You must be signed in to change notification settings - Fork 90
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
Track more build options #2634
Track more build options #2634
Conversation
clang-tidy review says "All clean, LGTM! 👍" |
src/bout++.cxx
Outdated
options["has_fftw"] = bout::build::has_fftw; | ||
options["has_gettext"] = bout::build::has_gettext; | ||
options["has_lapack"] = bout::build::has_lapack; | ||
options["has_netcdf"] = bout::build::has_netcdf; | ||
options["has_petsc"] = bout::build::has_petsc; | ||
options["has_legacy_netcdf"] = bout::build::has_legacy_netcdf; |
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.
Oh, this build flag should probably be removed entirely? It's not actually used for anything
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.
Currently referenced here:
CMakeLists.txt:set(BOUT_HAS_LEGACY_NETCDF OFF)
autoconf_build_defines.hxx.in:#undef BOUT_HAS_LEGACY_NETCDF
bin/bout-config.in:has_legacy_netcdf="@BOUT_HAS_LEGACY_NETCDF@"
bin/bout-config.in: echo " --has-legacy-netcdf -> $has_legacy_netcdf"
bin/bout-config.in: echo $has_legacy_netcdf
bin/bout-v5-macro-upgrader.py: "new": "BOUT_HAS_LEGACY_NETCDF",
cmake_build_defines.hxx.in:#define BOUT_HAS_LEGACY_NETCDF 0
configure: (skipped)
configure.ac:BOUT_HAS_LEGACY_NETCDF=no
configure.ac: BOUT_HAS_LEGACY_NETCDF=yes
configure.ac: BOUT_HAS_LEGACY_NETCDF=yes
configure.ac:BOUT_DEFINE_SUBST(BOUT_HAS_LEGACY_NETCDF, [NETCDF support])
configure.ac:AC_MSG_NOTICE([ NetCDF support : $BOUT_HAS_NETCDF (legacy: $BOUT_HAS_LEGACY_NETCDF)])
include/bout/build_config.hxx:constexpr auto has_legacy_netcdf = static_cast<bool>(BOUT_HAS_LEGACY_NETCDF);
include/options_netcdf.hxx:#if !BOUT_HAS_NETCDF || BOUT_HAS_LEGACY_NETCDF
src/bout++.cxx: has_netcdf ? (has_legacy_netcdf ? " (Legacy)" : " (NetCDF4)") : "";
src/bout++.cxx: options["has_legacy_netcdf"] = bout::build::has_legacy_netcdf;
src/sys/options/options_netcdf.cxx:#if BOUT_HAS_NETCDF && !BOUT_HAS_LEGACY_NETCDF
tests/integrated/test-options-netcdf/CMakeLists.txt: CONFLICTS BOUT_HAS_LEGACY_NETCDF
tests/integrated/test-squash/runtest: "(wtime|ncalls|arkode|cvode|run_id|run_restart_from|M.?SUB|N.?PE|iteration|wall_time|has_legacy_netcdf).*"
tests/unit/sys/test_options_netcdf.cxx:#if BOUT_HAS_NETCDF && !BOUT_HAS_LEGACY_NETCDF
tools/pylib/boutconfig/__init__.py.cin: "has_legacy_netcdf": "@BOUT_HAS_LEGACY_NETCDF@",
tools/pylib/boutconfig/__init__.py.in: "has_legacy_netcdf": "@BOUT_HAS_LEGACY_NETCDF@",
configure might set this to YES - so should we wait till configure is dropped?
Or is it already safe to do so?
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.
We'll drop configure soon, but this flag is already deprecated, so no need to add it here
90369ab
to
9ce221e
Compare
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.
clang-tidy made some suggestions
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.
LGTM, please remove the legacy netcdf flag from the build options before merge
Avoids an error being thrown on restart if these flags have already been set.
xbout relies on
use_metric_3d
being present ...