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

Unnecessary yaFyaml dependency in GOCART build system #184

Closed
rmontuoro opened this issue Sep 14, 2022 · 11 comments
Closed

Unnecessary yaFyaml dependency in GOCART build system #184

rmontuoro opened this issue Sep 14, 2022 · 11 comments
Assignees
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) bug Something isn't working

Comments

@rmontuoro
Copy link
Contributor

yaFyaml is not needed to build GOCART. However, the following dependency is included in the build system:

find_package (YAFYAML REQUIRED)

@rmontuoro rmontuoro added 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) bug Something isn't working labels Sep 14, 2022
@mathomp4
Copy link
Member

Ah. Nuts. Yeah. We probably need a "build like UFS" build test here as well.

@rmontuoro In MAPL we handle this via USE_EXTDATA2G. I'm guessing you are building MAPL with -DUSE_EXTDATA2G=NO?

@tclune tclune removed their assignment Sep 14, 2022
@rmontuoro
Copy link
Contributor Author

@mathomp4 - yes, we build MAPL with -DUSE_EXTDATA2G=NO in the UFS.

The yaFyaml dependency is required by MAPL though, not GOCART.

@mathomp4
Copy link
Member

True. Indeed, there seem to be a lot of unneeded find_package in GOCART:

GOCART/CMakeLists.txt

Lines 61 to 64 in 4c94179

find_package (GFTL REQUIRED)
find_package (GFTL_SHARED REQUIRED)
find_package (PFLOGGER QUIET)
find_package (YAFYAML REQUIRED)

Those are needed by MAPL, not GOCART. @rmontuoro if I make up a hotfix patch, can you test it?

@rmontuoro
Copy link
Contributor Author

@mathomp4 - sure. Thanks!

@mathomp4
Copy link
Member

@rmontuoro Can you actually tell me exactly how you are building MAPL now? That is what CMake options? I should capture that.

@rmontuoro
Copy link
Contributor Author

@mathomp4
Copy link
Member

@rmontuoro Okay can you try the hotfix/mathomp4/184-ufs-cmake-fix? (see #185)

@mathomp4
Copy link
Member

@rmontuoro Actually, another question: what version of GOCART are you using? My draft PR is based on our main which is 2.1.0, but if you still use an older version, I'll need to hotfix that too.

@rmontuoro
Copy link
Contributor Author

@mathomp4 - Thank you.
We are currently using rev. a342b1b in the ufs-weather-model

@mathomp4
Copy link
Member

@mathomp4 - Thank you. We are currently using rev. a342b1b in the ufs-weather-model

Ah. Good then we can (for now) just hotfix 2.1.0 since you are (essentially) on that. I'm doing testing now and will coordinate with @amdasilva about getting a v2.1.1 out for you.

@rmontuoro
Copy link
Contributor Author

@mathomp4 - hotfix/mathomp4/184-ufs-cmake-fix built successfully in the UFS after minor modifications listed in #185 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants