Skip to content
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

Update submodules for ABI compatability #835

Closed
3 tasks done
hellkite500 opened this issue Jun 12, 2024 · 1 comment · Fixed by #836
Closed
3 tasks done

Update submodules for ABI compatability #835

hellkite500 opened this issue Jun 12, 2024 · 1 comment · Fixed by #836
Assignees
Labels
build Issues related to CMake and building ngen Compatibility Issues Known compatibility or other issues that might be useful to have indexed as such in the future

Comments

@hellkite500
Copy link
Member

hellkite500 commented Jun 12, 2024

Given #834 , we need to ensure the ngen submodules supported and automatically built are compatible with the current BMI ABI. It looks like SLOTH and bmi-cxx were updated as part of the process, but these were not and need to get updated.

  • SoilFreezeThaw
  • SoilMoistureProfiles
  • CFE*

*CFE may need some additional consideration, especially given the v1.0.0 release tag. This change may need to be back ported on a bugfix branch and a v1.0.1 release added to ensure this commit carries into v1. Or we drop support for v1 and only support v2 after that commit?

@PhilMiller is the change from macro to const int in the C interface problematic with the updates on ngen? Based on my reviews and understanding, I don't think it is, but I could be wrong. If it isn't an issue in our current usage, then the previous comment can likely be safely ignored for the time being.

@hellkite500 hellkite500 self-assigned this Jun 12, 2024
@hellkite500 hellkite500 added Compatibility Issues Known compatibility or other issues that might be useful to have indexed as such in the future build Issues related to CMake and building ngen labels Jun 12, 2024
hellkite500 added a commit to hellkite500/ngen that referenced this issue Jun 12, 2024
@PhilMiller
Copy link
Contributor

The change to CFE's bmi.h for macros vs const int affects the CI for other formulation modules that integrate CFE as part of their testing (i.e. they say #include "bmi_cfe.h" \ #include "bmi.hxx"), but shouldn't affect ngen itself, since we never compile the different modules together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues related to CMake and building ngen Compatibility Issues Known compatibility or other issues that might be useful to have indexed as such in the future
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants