-
Notifications
You must be signed in to change notification settings - Fork 57
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
Adds "makedep" script to replace mkmf #135
Conversation
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #135 +/- ##
============================================
+ Coverage 33.45% 34.26% +0.81%
============================================
Files 262 257 -5
Lines 71434 69742 -1692
Branches 13339 12913 -426
============================================
Hits 23896 23896
+ Misses 43064 41372 -1692
Partials 4474 4474
Continue to review full report at Codecov.
|
FWIW, regression and performance tests are failing because they depend on mkmf being installed which is no longer the case. These are expected fails. |
ac/Makefile.in
Outdated
|
||
.PHONY: depend | ||
depend: | ||
../../../ac/makedep -o Makefile.dep $(SRC_DIRS) |
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.
Do we need this depend
rule? Should it just be Makefile.dep
?
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.
This target allows a single target to be used to generate the dependencies either when autoconf is making the Makefile or after the fact. make depend
is the "old-timer" conventional way to do this. I think it better that autoconf use make depend
than requiring we re-do the autoconf when changing module dependencies.
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.
OK, I see what you are going for now. I agree with it.
But maybe could do something like this (if not too pedantic):
.PHONY: depend
depend: Makefile.dep
Makefile.dep: $(SRC_DIRS)
../../../ac/makedep -o $@ $^
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.
I like it
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.
The ../../..
is wrong. It needs to be something like ${srcdir}/ac
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.
@srcdir@/ac/makedep
is working for me
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.
I have a fix for this and a few other things, will send it to your branch later today.
Currently, So I think there might be some opportunity to unwind some of the "non-standard macros" that you refer. For example:
Those three things came quickly to mind, there may be others. In general, I'd suggest purging any mkmf-y variables and using native autoconf ones. |
Good idea. |
`mkmf` is an external dependency that that uses a multi-stage approach to building a Makefile with dependencies but fails to yield an optimal link stage (it links everything, including unused modules). `makedep` is a bash script that constructs the link stage for as many programs as found, links only the necessary object files*, and is coded solely in bash thus removing an external dependency. The addition of this script is the bulk of this commit. Code changes: - when more than one program is encountered the executable is given the name following "program". Since all the programs were named "MOM_main" I have had to change them to provide unique names: - MOM_sum_driver.F90, "program MOM_main" has been changed to "MOM_sum_driver" - solo_driver/MOM_driver.F90, "program MOM_main" has been changed to "MOM6" Script changes: - Makefile.in now has a target "depend" to generate dependencies using makedep - configure.ac no longer checks for list_paths and mkmf - configure.ac now invokes "make depend" in place of mkmf - Added target "unit" to .testing/Makefile to build all programs in config_src/drivers/unit_drivers Ugliness: - I had to add a -f option to makedep to handle FMS non-standard macros - To compile FMS, the dependencies Makefile is passed CPPDEFS in addition to CPPFLAGS. - The first version of makedep was consistent with the standard gmake rules which were sufficient to build MOM6. Adding -f "rule command" allows FMS to be built: makemake -f '$(FC) $(FFLAGS) $(CPPFLAGS) $(CPPDEFS) -c $<' -x libFMS.a ../src - .inc suffix is included when searching for include directories - FMS has includes of .inc files which modify the search path passed to /lib/cpp . - Handling of badly formatted comments when searching for modules - FMS fm_util.F90, that generates fm_util_mod.mod, has some odd strings in a comment on the module declaration line. This was causing wierdness in the script. - Not just Fortran dependencies - makedep needs to also generate rules for C files in order to build FMS Todo: [ ] *A work around is used for TEOS10 (gsw_*) functions that are in separate object files even though accessed via a module (WTFortran!!!)
* Autoconf: Fix makedep path The current path of makedep in the autoconf build assumes a directory tree as in .testing. This patch uses the generalized @SrcDir@ to support more general autoconf builds. Other minor changes: * The `depend` rule was split into an explicit Makefile.dep rule and a phony rule to support `make depend` * SRC_DIRS shell assignment is replaced with an Autoconf macro. * A "self-generate" rule was added to `Makefile.in`, so that changes to `Makefile.in` do not trigger a full `./configure` run and regeneration of `.config.status`. This could possibly be extended to support `make depend` but let's first see how this one goes. * Autoconf: makedep uses autoconf var conventions This patch changes the makedep script to use autoconf environment variable conventions rather than mkmf ones: * FCFLAGS in place of FFLAGS * DEFS in place of CPPDEFS * LDFLAGS and LIBS rather than just LDFLAGS (NOTE: This differs from Makefile's LDLFLAGS/LDLIBS) This also allowed us to remove the custom build rule in the FMS build. Note that we now use an autoconf-friendly rule, rather than the default Makefile rule (which was arguably for fixed-format Fortran anyway). The description of autoconf->mkmf translation from the Makefile templates has also been removed, since they're no longer relevant. Some other minor changes in this build: * The `make depend` rule was added to the FMS Makefile template. * @SrcDir@ is directly passed to FMS makedep, rather than identically re-defining it in a variable. * Testing: Resolve makedep paths This patch resolves some issues relating to finding the path to makedep in both .testing and more generalized autoconf builds. An explicit autoconf test for makedep has been added, with a default path which includes the `ac` directory relative to `deps`. The .testing directory, which does not lie within `ac`, instead modifies the PATH to allow autoconf to find makedep. The absolute path is determined and substituted into the appropriate Makefile.in template. Some redundant operations in .testing/Makefile have been removed, but I suspect there are even more. Much of the structure required to support mkmf and list_paths is probably no longer needed.
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/15904 ✔️ Good bye, mkmf. |
initialize cpl_scalar field when created
Adds
makedep
script that constructs the link stage for as many programs as found, links only the necessary object files*, and is coded solely in bash thus removing an external dependency (namelymkmf
). The addition of this script is the bulk of this commit.Code changes:
Script changes:
Ugliness:
makemake -f '$(FC)
Todo:
[ ] A work around is used for TEOS10 (gsw_) functions that are in separate object files even though accessed via a module (WTFortran!!!)