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

GitHub Issue NOAA-EMC/GSI#302. Removal of libsrc from the authoritative repository #329

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

MichaelLueken
Copy link
Contributor

This update removes the dependence of the libsrc submodule from the GSI repository. The libsrc entries in .gitmodules have been removed, the libsrc directory has been removed, and all modulefiles that have been updated to use hpc-stack have been updated to add module load wrf_io/1.2.0. These changes were tested on WCOSS_D and Hera, where the code compiled without issue and the regression tests show bit reproducibility between this update and the current libsrc/wrflib executables.

Close #302

@aerorahul
Copy link
Contributor

@MichaelLueken-NOAA
The removal of libsrc as described in the PR looks good.

I have a follow on question.

There are references to libsrc in the build.
E.g.

  1. Lines 190-191 in the top-level CMakeLists.txt.
  2. In every FindNCEPLIB.cmake file e.g. FindSIGIO.cmake under cmake/Modules that will try to find, and if not found, build from the source expected to be in libsrc.
  3. Some of the utility CMakeLists.txt e.g. util/Correlated_Obs/CMakeLists.txt are referencing libsrc.

So, the question is, are these references to libsrc going to be removed/cleaned in a subsequent PR?

@MichaelLueken
Copy link
Contributor Author

@aerorahul Thank you very much for the review, Rahul.

Thanks for reminding me about the libsrc entries hardwired into the CMakeLists.txt files and the cmake/Modules/FindNCEPLIBS.cmake files. As of this point, I have cleaned CMakeLists.txt and the cmake/Modules/Find* files. On Monday, I will go through the CMakeLists.txt files in util to clean those up, then push the completed work.

@MichaelLueken
Copy link
Contributor Author

@aerorahul, I have removed libsrc from the CMakeLists.txt file (and CMakeLists.txt files in util) and from cmake/Modules/Find*.cmake. I have removed findHelpers.cmake and removed libsrc from FindCORELIBS.cmake. I have successfully compiled all DA components and ran the regression tests (both standard and debug) and all passed successfully.

@aerorahul
Copy link
Contributor

@MichaelLueken-NOAA
Removing references about libsrc in the CMakeLists.txt and other files looks good.

I noticed, however, that now that libsrc is gone, the build options such as BUILD_CORELIBS and other such as BUILD_BACIO, BUILD_NEMSIO etc make no sense.
The third-partly libraries (including the NCEPLibs) are now expected to be present prior to building the GSI.

Do you anticipate removing those options and further cleaning up the CMakeLists.txt and other macros in a subsequent PR? If so, that is fine.

@MichaelLueken
Copy link
Contributor Author

@aerorahul
Since changes related to BUILD_* options lie outside the scope of the removal of the libsrc submodule, I have created issue #331 to address this.

With your approval, I will commit this work to the authoritative repository tomorrow, Wednesday, March 9.

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

The changes in this PR look good.

@MichaelLueken
Copy link
Contributor Author

Thank you very much for reviewing these changes, @aerorahul! Since there are no source code changes, I will now give final approval to these changes and merge them to the authoritative repository.

@MichaelLueken MichaelLueken merged commit 47ee70b into NOAA-EMC:master Mar 9, 2022
AndrewEichmann-NOAA pushed a commit to AndrewEichmann-NOAA/GSI that referenced this pull request Jun 6, 2022
GitHub Issue NOAA-EMC#302. Removal of libsrc from the authoritative repository
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.

Removal of libsrc from the authoritative repository
2 participants