Fixes esmf-org/esmf#136, fix cygwin build#348
Fixes esmf-org/esmf#136, fix cygwin build#348harmenwierenga wants to merge 1 commit intoesmf-org:developfrom
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))))
endifor 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 |
danrosen25
left a comment
There was a problem hiding this comment.
- 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 |
|
Per discussion at Wednesday's ESMF core team meeting, I'm closing this in favor of #376 (which restores an earlier solution from @harmenwierenga ). |
|
@harmenwierenga - I'd welcome you to look at #376 and confirm that it resolves your issue. |
Reorder PIO to occur before NetCDF in common.mk This is needed with the linker on cygwin since PIO uses NetCDF, so the link flags need to be ordered -lpioc -lnetcdf. (It's possible that other linkers need this also, though we haven't seen the need for it on our test systems.) This replaces #348 , based on decided-upon solution from this week's ESMF core team meeting.
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++11turns off GNU extensions of the compiler. I had toexport ESMF_CXXSTD=sysdefaultto ensure that the standard used is-std=gnu++17so 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-lpiocflag gets added before netcdf.export ESMF_RANLIB=true, sincetrueis 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!