-
Notifications
You must be signed in to change notification settings - Fork 80
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
base: develop
Are you sure you want to change the base?
Conversation
@harmenwierenga 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 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 |
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.
- Prepend to make variable instead of moving PIO block.
- Send PIO contributions to PIO repository.
From a CI setup I did a while back in #202, you could add to the list of Cygwin packages:
I also managed to get it working with Your idea of changing the C++ standard is more elegant than my method of just defining visibility macros until ESMF compiled |
…ise netcdf missing symbols are added too late
38c4d52
to
375d02c
Compare
@danrosen25 Thank you for the suggestion. I have taken it over, and I will try to send the other contributions to PIO. |
Thank you for the response. In my case, I used mpiuni, so I do not need openmpi, and the default compilation does not require lapack either. make was also in my list indeed. I also did not try to run the tests, so that is probably why I did not require the others. ranlib is needed during the install step, I could not get it to work with any version of ranlib that I found in cygwin. It would be great if an automatic test could be set up after this! |
I have also created a PR to PIO: NCAR/ParallelIO#2002 Edit: These changes were merged to PIO. Their last official release is from August 2023, would ESMF want to wait for a new PIO release to update it? |
I suspect a reply would be more successful than an edit in getting ESMF maintainer attention on this thread |
Yes, I have replied to and tagged @danrosen25 5 days ago. |
https://github.com/NCAR/ParallelIO/releases. New release pio2_6_5 |
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):
The issues that I encountered:
-std=c++11
turns off GNU extensions of the compiler. I had toexport 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.${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.-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 setexport ESMF_NETCDF_LIBS="-lpioc -lnetcdff -lnetcdf"
, in hindsight I should have probablyexport 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.export ESMF_RANLIB=true
, sincetrue
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!