Build system improvements #354
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,31 @@ | |
| # If the cice binary is passed as an argument and the file exists, | ||
| # copy it into the run directory and don't build the model. | ||
|
|
||
| set dohelp = 0 | ||
| set directmake = 0 | ||
| set target = "UnDEFineD" | ||
| set ciceexe = "UnDEFineD" | ||
| if ($#argv == 0) then | ||
| # continue, standard way to build | ||
| else if ($#argv == 1) then | ||
|
Member
There was a problem hiding this comment. I think the argument checking is too drastic; it prevents one from using all the features of make (ex. see https://www.gnu.org/software/make/manual/make.html#Instead-of-Execution where some flags that are supported by make are listed). This is one of the reason I passed all arguments in my initial implementation (
Contributor
Author
There was a problem hiding this comment. OK, let me see what I can do. I didn't think about passing command line arguments directly to gmake and wanted to keep the parsing simple. But I can see what that might be useful, I will try to refactor. Thanks for the careful review, not too picky at all!
Contributor
Author
There was a problem hiding this comment. @phil-blain, what do you think the requirements are here? Is it just cice.build # standard approach or is there also a requirement for cice.build [gmake option] [gmake option] ... [target] and so forth. Should we support gmake arguments with and/or without a target like gmake --debug --keep-going and others as well? It seems to me if we are going to allow the ability to pass the gmake arguments from cice.build to gmake, it needs to be general, support multiple arguments, and work with a target as well. I will try to do that.
Contributor
Author
There was a problem hiding this comment. I think I've got it working, we definitely want the most flexibility and I think that was suggested in the review.
Member
There was a problem hiding this comment. Yes, I agree that more flexibility is good, so all of these should work : cice.build # standard approach As a side note, with the changes you already made we are now able to call |
||
| # -h, --help, or target | ||
| if ("$1" == "-h" || "$1" == "--help") then | ||
| set dohelp = 1 | ||
| else | ||
| set directmake = 1 | ||
| set target = $1 | ||
| if ("$target" =~ "-*") then | ||
| echo "${0}: ERROR: invalid target ${target} starts with dash" | ||
| exit -99 | ||
| endif | ||
| endif | ||
| else if ($#argv == 2) then | ||
| # --exe ciceexe (undocumented feature, used only in test suites) | ||
| if ("$1" != "--exe") then | ||
| echo "${0}: ERROR: 2 arguments assume --exe ciceexe" | ||
| exit -99 | ||
| endif | ||
| set ciceexe = $2 | ||
| echo "${0}: ciceexe = ${ciceexe}" | ||
| if (-e ${ciceexe}) then | ||
|
|
||
|
|
@@ -25,6 +47,45 @@ if ($#argv == 1) then | |
| exit 0 | ||
|
|
||
| endif | ||
| else | ||
| echo "${0}: ERROR in arguments" | ||
| set dohelp = 1 | ||
| endif | ||
|
|
||
| if (${dohelp} == 1) then | ||
| cat << EOF1 | ||
|
|
||
| NAME | ||
| cice.build [-h,--help] [target] | ||
|
|
||
| SYNOPSIS | ||
| -h || --help | ||
| target | ||
|
|
||
| DESCRIPTION | ||
| --help, -h : help | ||
| target : specify make target, defined in Makefile | ||
|
|
||
| EXAMPLES | ||
| cice.build | ||
| will build the model using the recommended approach with all the bells and whistles | ||
| cice.build clean | ||
| is an example of a targeted build | ||
| cice.build --help | ||
| will show cice.build help | ||
| cice.build targets | ||
| will show valid Makefile targets | ||
|
|
||
| ADDITIONAL INFORMATION | ||
| The argument implementation is fairly primitive. It accepts only no arguments | ||
| (where it builds the default target), a target argument, or a -h||--help | ||
| argument. These cannot be mixed. | ||
|
|
||
| SEE ALSO | ||
| User Documentation at https://github.com/cice-consortium/cice/ | ||
|
Member
There was a problem hiding this comment. Do we want to link to read-the-docs here instead ?
Contributor
Author
There was a problem hiding this comment. The problem is what version of readthedocs? Almost certainly whatever version we might link in the help output is incorrect. Master documentation is only correct for the current master. For released versions, would we need to manually change the link for each release? This is a case where it's easy for things to get out of sync. While pointing to the general cice repo isn't particularly useful either, at least I feel like it's never incorrect. |
||
|
|
||
| EOF1 | ||
| exit -99 | ||
| endif | ||
|
|
||
| #==================================== | ||
|
|
@@ -47,10 +108,12 @@ set stamp = `date '+%y%m%d-%H%M%S'` | |
| set ICE_BLDLOG_FILE = "cice.bldlog.${stamp}" | ||
| set quiet = ${ICE_QUIETMODE} | ||
|
|
||
| # tcraig, this is handled below, is it needed here? | ||
| #if (${ICE_CLEANBUILD} == 'true') then | ||
| # echo "cleaning objdir" | ||
| # rm -r -f ${ICE_OBJDIR} | ||
| #endif | ||
| if !(-d ${ICE_RUNDIR}) mkdir -p ${ICE_RUNDIR} | ||
| if !(-d ${ICE_OBJDIR}) mkdir -p ${ICE_OBJDIR} | ||
| cd ${ICE_OBJDIR} | ||
|
|
||
|
|
@@ -79,6 +142,18 @@ ${ICE_SANDBOX}/cicecore/shared | |
| ${ICE_SANDBOX}/icepack/columnphysics | ||
| EOF | ||
|
|
||
| if (${directmake} == 1) then | ||
|
Member
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. Good catch, I am making those changes.
Contributor
Author
There was a problem hiding this comment. On the second point, I think we decided for a direct make to avoid all the fluff. I think writing the make output to a bldlog file is part of that. I guess I view the circular dependencies as part of the fluff you get if you use the scripts the standard way. Also, the quietmode should not generally be used except by travis. For whatever reason, if we don't use the quiet mode with travis, it creates problems. it either concatenates the raw log file or it makes it very hard to find the errors. this was added just for travis and serves a purpose.
Member
There was a problem hiding this comment. ok, I guess I viewed the 'fluff' as only the README.case file. But I don't feel strongly about that. So either way is fine. |
||
| echo "make ${target}" | ||
| ${ICE_MACHINE_MAKE} -j ${ICE_MACHINE_BLDTHRDS} VPFILE=Filepath EXEC=${ICE_RUNDIR}/cice \ | ||
| -f ${ICE_CASEDIR}/Makefile MACFILE=${ICE_CASEDIR}/Macros.${ICE_MACHCOMP} ${target} | ||
| set bldstat = ${status} | ||
| if (${bldstat} != 0) then | ||
| echo "${0}: targeted make FAILED" | ||
| exit -99 | ||
| endif | ||
| exit 0 | ||
| endif | ||
|
|
||
| echo " " | ||
| echo ICE_GRID = ${ICE_GRID} | ||
| echo ICE_NTASK = ${ICE_NTASKS} | ||
|
|
@@ -88,13 +163,8 @@ echo "Filepath = " | |
| cat ${ICE_OBJDIR}/Filepath | ||
| echo " " | ||
|
|
||
| echo "building cice > ${ICE_OBJDIR}/${ICE_BLDLOG_FILE}" | ||
|
|
||
| if !($?ICE_MACHINE_BLDTHRDS) then | ||
| set ICE_MACHINE_BLDTHRDS = 1 | ||
| endif | ||
|
|
||
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.
a single
.PHONY: all cice db_files db_flags mostlyclean clean realcleanline would also workThere 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, good to know. I may do that instead, it's cleaner.