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

Fixes esmf-org/esmf#136, fix cygwin build #348

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

harmenwierenga
Copy link

At Deltares we rely on a Cygwin build of ESMF. There was an attempt at updating ESMF two few years ago, see #136. I have now managed to create a working build of ESMF 8.8.0, and it required me to make a few changes to build everything (including PIO) for Windows.

I installed the following packages in cygwin (not 100% sure that this list is exhaustive, since some other packages were already installed):

  • gcc-core 12.4.0-3
  • gcc-fortran 12.4.0-3
  • gcc-g++ 12.4.0-3
  • make 4.4.1-2
  • libnetcdf-devel 4.9.2-2
  • libnetcdf-fortran-devel 4.5.4-1
  • zlib-devel 1.3.1-1
  • libhwloc-devel 2.10.0-1
  • libevent-devel 2.1.12-1
  • cmake 3.28.3-1
set -o igncr
export ESMF_DIR=/cygdrive/c/<ESMF_CHECKOUT_DIR>
export ESMF_COMM=mpiuni
export ESMF_OPTLEVEL=2
export ESMF_NETCDF=split
export ESMF_NETCDF_INCLUDE=/usr/include
export ESMF_NETCDF_LIBPATH=/usr/lib
export ESMF_INSTALL_PREFIX=install
export ESMF_CXXSTD=sysdefault # some C++ libraries require gcc posix extensions, specifically the sigaction definition
export ESMF_RANLIB=true # ranlib does not work during the install stage, use this no-op

The issues that I encountered:

  1. Some POSIX headers are used, and the default C++ standard -std=c++11 turns off GNU extensions of the compiler. I had to export ESMF_CXXSTD=sysdefault to ensure that the standard used is -std=gnu++17 so that GNU extensions were turned on. Going to C++17 did not give any issues.
  2. In the PIO CMake (src/Infrastructure/IO/PIO/ParallelIO/src/clib/CMakeLists.txt), there is a check that long long and size_t are the same size. On cygwin, these sizes cannot be retrieved, which makes them both empty strings. This causes the check to fail, because empty strings cannot be parsed as numbers so ${SIZEOF_SIZE_T} EQUAL ${SIZEOF_LONG_LONG} evaluates to false in CMake. I have added a small fix for this that does nothing if either returns an empty string.
  3. In the PIO C library (src/Infrastructure/IO/PIO/ParallelIO/src/clib/pioc_support.c) there is an include of execinfo.h. This header is missing on many platforms, including Cygwin. I have added a (I believe portable) check to see if the header is available, and I have excluded the include and the use of the backtrace functionality from that file when execinfo.h is not available. The backtrace functionality appears to be added as some useful logging for debugging purposes, but not as a necessary feature of the library, so I hope I was correct that it could simply be turned off. I have tried to suppress any warnings of unused variables.
  4. The order of linker flags of NetCDF and PIOC was wrong: it was -lnetcdff -lnetcdf -lpioc, but since pioc requires NetCDF, it will add a bunch of extra missing symbols that then cannot be found any more. It could be fixed through a bit of a hack (I set export ESMF_NETCDF_LIBS="-lpioc -lnetcdff -lnetcdf", in hindsight I should have probably export ESMF_PIO_LIBS="-lpioc -lnetcdf"). Instead, I now reordered PIO and NetCDF in the makefile for third party components, so that the -lpioc flag gets added before netcdf.
  5. During the install, ranlib kept complaining that it did not understand the binaries. Since it appears to be an optimization for the linker, but does not appear necessary in this case, I export ESMF_RANLIB=true, since true is a no-op command in bash. This was also a bit of a hack.

This PR addresses points 2, 3, and 4. Please let me know if I can do more to get this merged!

@danrosen25
Copy link
Member

@harmenwierenga
Hello and thank you for your contributions.

The PIO code change suggestions should be requested to the PIO repository here. We can then upgrade the PIO version within ESMF. The code maintainer of PIO @jedwards4b will review your suggestions once a PR is opened.

In the common.mk file the PIO library information should follow the NetCDF library information because there are (sort of) checks related to NetCDF. I would suggest prepending the -lpio flag to the third party library flags ESMF_CXXLINKLIBSTHIRD ESMF_F90LINKLIBSTHIRD. See example. Variable expansion is a little tricky in make but it should work. We can also think rewriting the entire PIO block to be within NETCDF

ifdef ESMF_PIO_LIBS
ESMF_CXXLINKLIBSTHIRD     := $(ESMF_PIO_LIBS) $(ESMF_CXXLINKLIBSTHIRD)
ESMF_CXXLINKRPATHSTHIRD   += $(addprefix $(ESMF_CXXRPATHPREFIX),$(subst -L,,$(filter -L%,$(ESMF_PIO_LIBS))))
ESMF_F90LINKLIBSTHIRD     := $(ESMF_PIO_LIBS) $(ESMF_F90LINKLIBSTHIRD)
ESMF_F90LINKRPATHSTHIRD   += $(addprefix $(ESMF_F90RPATHPREFIX),$(subst -L,,$(filter -L%,$(ESMF_PIO_LIBS))))
endif

or maybe

ifdef ESMF_PIO_LIBS
ESMF_CXXLINKLIBSTHIRD     := $(addprefix $(ESMF_PIO_LIBS) ,$(ESMF_CXXLINKLIBSTHIRD))
ESMF_CXXLINKRPATHSTHIRD   += $(addprefix $(ESMF_CXXRPATHPREFIX),$(subst -L,,$(filter -L%,$(ESMF_PIO_LIBS))))
ESMF_F90LINKLIBSTHIRD     := $(addprefix $(ESMF_PIO_LIBS) ,$(ESMF_F90LINKLIBSTHIRD))
ESMF_F90LINKRPATHSTHIRD   += $(addprefix $(ESMF_F90RPATHPREFIX),$(subst -L,,$(filter -L%,$(ESMF_PIO_LIBS))))
endif

Copy link
Member

@danrosen25 danrosen25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Prepend to make variable instead of moving PIO block.
  • Send PIO contributions to PIO repository.

@danrosen25 danrosen25 self-assigned this Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants