418 pkg confg#497
Conversation
|
Yaswant Pradhan (@yaswant) , ready for review. Not urgent for the release, it's just makes site-configs more convenient to write. |
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
I'm not sure this is a problem which needs an object solution. In particular an object per library seems a bit odd since that obvious is immediately thrown away after the library has been interrogated.
It may be helpful to look at https://github.com/MetOffice/lfric_core/blob/main/lfric_build/pkg_config.py for an alternative approach. Although this is not necessarily the best solution either.
Also, the shell command seems like a different change. Can it be moved to its own pull request?
That is the first time that I got a comment to remove OO ;) The classes here inherit from the
I had a quick look, and while it supports version handling and better shared/static support, it's otherwise pretty much the same - one class per package? I don't see the real difference to the way this implementation is handling this. Also, after looking at the code, it appears that it combined But the support for version specification and better static/shared handling would actually be good to add imho.
I added a new section to the documentation to document the new tools, and then added a paragraph of explicit documentation for the existing I wonder if perhaps the previous PR wasn't clean or so, and showed some other changes? I've just updated this PR to latest TBH, since your implementation has already support for version specifications and static/shared linking, I am also happy to close this PR, so you can add your version instead. |
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
One minor issue of location.
There was a problem hiding this comment.
nfconfig is a very specific implementation detail which doesn't feel like it belongs in the build framework. This feels much more like an odd-ball quirk of a particular project. So lets move it to lfric_core where it seems like a better fit. You will probably want to delete the pkg_config library which is already there, since this replaces it.
By contrast, pkg_config is a standardised tool, much more appropriate across projects.
Adds a simple interface to
pkg-configandnf-config.