Move makdep compilation to Makefile #307
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* makdep compiled from a makdep rule in the Makefile * Add CFLAG_HOST macro * makdep is only compiled if needed * This addresses CICE-Consortium#306
apcraig
reviewed
Apr 30, 2019
apcraig
reviewed
Apr 30, 2019
There was a problem hiding this comment.
Minor point, could/should the Makefile rule for makdep be
since we have defined DEPGEN earlier and use it in the Makefile. It seems for complete consistency, we should use DEPGEN here or think about getting rid of it and use "makdep" throughout. This is very minor, but might as well think about it while it's here.
apcraig
reviewed
Apr 30, 2019
There was a problem hiding this comment.
Everything works fine on conrad, running a quick suite with 4 compilers. I also tested the change in the Makefile,
-makdep: $(ICE_CASEDIR)/makdep.c
+$(DEPGEN): $(ICE_CASEDIR)/makdep.c
and that works fine too. I will approve and merge once we decide whether to include this change.
|
I agree that the external dependency generator is unlikely to change, but you are right that for consistency we can use $(DEPGEN) as the target. |
phil-blain
force-pushed
the
build-system-robustness
branch
from
71e7bb2 to
6d990a7
Compare
|
I just thought that the new make variable CFLAG_HOST should be documented. I will update the doc. Commit upcoming. |
|
I added a section to the doc. It can be previewed here : |
apcraig
approved these changes
Apr 30, 2019
phil-blain
added a commit
to phil-blain/CICE
that referenced
this pull request
Jan 14, 2020
The CFLAG_HOST variable was added in CICE-Consortium#307, and some Makefile logic was added to define the variable to be blank if the included Macros file does not define it. However, the Make syntax is wrong, as 'ifndef' takes a variable name and not a reference to a variable [1], so ifndef $(CFLAGS_HOST) should have been written ifndef CFLAGS_HOST The effect of this error is to invert the logic of the check (!) since if CFLAGS_HOST is defined, Make will check if a variable with name equal to whatever CFLAGS_HOST is defined to be exists, which will most probably be false, and the conditional will then evaluate to true and CFLAG_HOSTS will be redefined to be blank, defeating the whole purpose of the flag. Since there's no harm in Make referencing and empty variable, fix this by just removing the conditional check. [1] https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html
apcraig
pushed a commit
that referenced
this pull request
Jan 24, 2020
* machines: add env and Macro files for conda on Linux and macOS * machines: add environment.yml conda specification * doc: add section on laptop computers Uses the conda environment. * doc: recommend not to touch the 'cice' conda environment * machines: add CFLAGS_HOST to conda_macos for recent macOS Recent versions of Xcode and macOS do not install the system C header files to /usr/include, but keep them in the MacOSX SDK. Thus we need to explicitely pass their location to the compiler. * Makefile: remove uneeded (and faulty) logic around CFLAGS_HOST The CFLAG_HOST variable was added in #307, and some Makefile logic was added to define the variable to be blank if the included Macros file does not define it. However, the Make syntax is wrong, as 'ifndef' takes a variable name and not a reference to a variable [1], so ifndef $(CFLAGS_HOST) should have been written ifndef CFLAGS_HOST The effect of this error is to invert the logic of the check (!) since if CFLAGS_HOST is defined, Make will check if a variable with name equal to whatever CFLAGS_HOST is defined to be exists, which will most probably be false, and the conditional will then evaluate to true and CFLAG_HOSTS will be redefined to be blank, defeating the whole purpose of the flag. Since there's no harm in Make referencing and empty variable, fix this by just removing the conditional check. [1] https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html * doc: use curl instead of wget on macOS wget is not installed by default * machines: conda: invoke 'conda info --base' using $CONDA_EXE This variable is always defined to the path to the conda executable when the login shell is properly initialized. Calling `conda info --base` works if the shell initialization procedure puts the conda executable in the $PATH, which is the case for Bash and Zsh but not for tcsh. * doc: conda: add initialization instructions for alternative login shells The Miniconda installer only initializes Bash on macOS and Linux, so additional steps needs to be taken if your login shell is different. Add instructions for tcsh, zsh, fish and xonsh. * machines: conda: add error checking to env files Let's check if the conda executable is found and if the cice conda environment exists. * doc: separate Miniconda installation and conda initialization - Move the documentation for the conda configuration under "Porting" - Move the instructions for initializing the user's shell for use with conda to a speparate section - Add a new section for manually initializing the user's shell if they do not want to modify their startup files - Emphasize which steps are "first-time setup" steps - Add details about how the conda environment is activated during build/run * doc: conda: mention symlinks and creating a complete port
phil-blain
added a commit
to phil-blain/CICE
that referenced
this pull request
Apr 15, 2020
…CE-Consortium#393) * machines: add env and Macro files for conda on Linux and macOS * machines: add environment.yml conda specification * doc: add section on laptop computers Uses the conda environment. * doc: recommend not to touch the 'cice' conda environment * machines: add CFLAGS_HOST to conda_macos for recent macOS Recent versions of Xcode and macOS do not install the system C header files to /usr/include, but keep them in the MacOSX SDK. Thus we need to explicitely pass their location to the compiler. * Makefile: remove uneeded (and faulty) logic around CFLAGS_HOST The CFLAG_HOST variable was added in CICE-Consortium#307, and some Makefile logic was added to define the variable to be blank if the included Macros file does not define it. However, the Make syntax is wrong, as 'ifndef' takes a variable name and not a reference to a variable [1], so ifndef $(CFLAGS_HOST) should have been written ifndef CFLAGS_HOST The effect of this error is to invert the logic of the check (!) since if CFLAGS_HOST is defined, Make will check if a variable with name equal to whatever CFLAGS_HOST is defined to be exists, which will most probably be false, and the conditional will then evaluate to true and CFLAG_HOSTS will be redefined to be blank, defeating the whole purpose of the flag. Since there's no harm in Make referencing and empty variable, fix this by just removing the conditional check. [1] https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html * doc: use curl instead of wget on macOS wget is not installed by default * machines: conda: invoke 'conda info --base' using $CONDA_EXE This variable is always defined to the path to the conda executable when the login shell is properly initialized. Calling `conda info --base` works if the shell initialization procedure puts the conda executable in the $PATH, which is the case for Bash and Zsh but not for tcsh. * doc: conda: add initialization instructions for alternative login shells The Miniconda installer only initializes Bash on macOS and Linux, so additional steps needs to be taken if your login shell is different. Add instructions for tcsh, zsh, fish and xonsh. * machines: conda: add error checking to env files Let's check if the conda executable is found and if the cice conda environment exists. * doc: separate Miniconda installation and conda initialization - Move the documentation for the conda configuration under "Porting" - Move the instructions for initializing the user's shell for use with conda to a speparate section - Add a new section for manually initializing the user's shell if they do not want to modify their startup files - Emphasize which steps are "first-time setup" steps - Add details about how the conda environment is activated during build/run * doc: conda: mention symlinks and creating a complete port
phil-blain
added a commit
to phil-blain/CICE
that referenced
this pull request
May 29, 2020
…CE-Consortium#393) * machines: add env and Macro files for conda on Linux and macOS * machines: add environment.yml conda specification * doc: add section on laptop computers Uses the conda environment. * doc: recommend not to touch the 'cice' conda environment * machines: add CFLAGS_HOST to conda_macos for recent macOS Recent versions of Xcode and macOS do not install the system C header files to /usr/include, but keep them in the MacOSX SDK. Thus we need to explicitely pass their location to the compiler. * Makefile: remove uneeded (and faulty) logic around CFLAGS_HOST The CFLAG_HOST variable was added in CICE-Consortium#307, and some Makefile logic was added to define the variable to be blank if the included Macros file does not define it. However, the Make syntax is wrong, as 'ifndef' takes a variable name and not a reference to a variable [1], so ifndef $(CFLAGS_HOST) should have been written ifndef CFLAGS_HOST The effect of this error is to invert the logic of the check (!) since if CFLAGS_HOST is defined, Make will check if a variable with name equal to whatever CFLAGS_HOST is defined to be exists, which will most probably be false, and the conditional will then evaluate to true and CFLAG_HOSTS will be redefined to be blank, defeating the whole purpose of the flag. Since there's no harm in Make referencing and empty variable, fix this by just removing the conditional check. [1] https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html * doc: use curl instead of wget on macOS wget is not installed by default * machines: conda: invoke 'conda info --base' using $CONDA_EXE This variable is always defined to the path to the conda executable when the login shell is properly initialized. Calling `conda info --base` works if the shell initialization procedure puts the conda executable in the $PATH, which is the case for Bash and Zsh but not for tcsh. * doc: conda: add initialization instructions for alternative login shells The Miniconda installer only initializes Bash on macOS and Linux, so additional steps needs to be taken if your login shell is different. Add instructions for tcsh, zsh, fish and xonsh. * machines: conda: add error checking to env files Let's check if the conda executable is found and if the cice conda environment exists. * doc: separate Miniconda installation and conda initialization - Move the documentation for the conda configuration under "Porting" - Move the instructions for initializing the user's shell for use with conda to a speparate section - Add a new section for manually initializing the user's shell if they do not want to modify their startup files - Emphasize which steps are "first-time setup" steps - Add details about how the conda environment is activated during build/run * doc: conda: mention symlinks and creating a complete port
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Developer(s): Philippe Blain
Please suggest code Pull Request reviewers in the column at right. @apcraig
Are the code changes bit for bit, different at roundoff level, or more substantial? BFB
Please include the link to test results or paste the summary block from the bottom of the testing output below.
I didn't run tests since I didn't touch the code itself.
Does this PR create or have dependencies on Icepack or any other models? No, but I will do the same for Icepack.
Is the documentation being updated with this PR? (Y/N) Yes
https://cice-phil-blain.readthedocs.io/en/build-system-robustness/user_guide/ug_running.html#cross-compiling
If not, does the documentation need to be updated separately at a later time? (Y/N) No
Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.