Skip to content

Add files via upload#905

Closed
pmjherman wants to merge 2 commits intoUnidata:masterfrom
pmjherman:pmjherman-changes-for-MINGW
Closed

Add files via upload#905
pmjherman wants to merge 2 commits intoUnidata:masterfrom
pmjherman:pmjherman-changes-for-MINGW

Conversation

@pmjherman
Copy link
Copy Markdown

These changes were made in order to make compilation of netcdf with opendap capabilities possible under MINGW-MSYS2. I refer to the email conversation with ticket netCDF #JZZ-916731. The present pull request is changed from my earlier patches, based on your response. In particular, I have avoided use of the generic compilation flag WIN32 but made everything as specific and tailored to the functions involved as possible. This should not interfere with other builds than builds under MINGW (I hope).

The changes, per file, are as follows:

  1. /libdispatch/dutil.c Correction of a typing error in the code. I bumped onto this, probably because something else in the compilation had gone wrong, so I am not sure that this correction is absolutely needed. However, it will also not harm to correct
  2. CMakeLists.txt The changes in this file are as follows: (a) in a number of places changes for MSVC are also needed for MINGW. These changes were copied from an existing MSYS2 compilation recipe, but instead of using the flag WIN32 I made it more specific (MINGW) so as not to interfere with other compilation procedures (e.g. Cygwin or others). (b) In addition, I defined two new variables that are set both under MSVC and MINGW. HAVE_WIN_TEMP expresses the fact that one has to look for the tmp directory using the Windows environmental variable TEMP, and is used in /libdispatch/ddispatch.c; HAVE_WIN_SSCANF expresses the fact that the Windows runtime library is used for the sscanf function (this is done both by MSVC and by MINGW). In the Windows implementation of sscanf, two differences occur: the specifier %hhu is not recognized, and NaN is not recognized as a special character sequence but results in an error if one assigns to a float or double. The flag HAVE_WIN_SSCANF is used in the files /lbdap2/dapcvt.c and /ncgen/cvt.c
  3. config.h.cmake.in is changed in order to set the compiler flags HAVE_WIN_TEMP and HAVE_WIN_SSCANF with #cmakedefine commands
  4. ncconfigure.h is changed in order to solve a problem with strlcat. This function is not available under MINGW, and the alternative formulation that belongs to the netcdf source could not be linked to certain functions because strlcat was not exported to the dll. I have changed the header in such a way that this export happens, but only when HAVE_STRLCAT is undefined. It should change nothing to builds where the function is natively present.

Note one point where interference with other builds is, in principle, possible. Some changes in the use of sscanf under MSVC were made specific to a certain version of MSVC in the file dapcvt.c, while I have now set the flag HAVE_WIN_SSCANF generic for all versions of MSVC. I did this because I could not find back any documentation from Microsoft suggesting that sscanf had been changed and the lacking functionality added in later versions. In addition, I think it very unlikely that lack of functionality was only present in a single version, not in earlier or later ones. However, I am not entirely certain about this and it is a matter of consideration

Note, further, that I only made changes to the CMake system, not to the config/autoconfig system as I did not use that and do not know very well how it functions.

These changes were made in order to make compilation of netcdf with opendap capabilities possible under MINGW-MSYS2. I refer to the email conversation with ticket netCDF #JZZ-916731. The present pull request is changed from my earlier patches, based on your response. In particular, I have avoided use of the generic compilation flag WIN32 but made everything as specific and tailored to the functions involved as possible. This should not interfere with other builds than builds under MINGW (I hope).

The changes, per file, are as follows:
1. /libdispatch/dutil.c   Correction of a typing error in the code. I bumped onto this, probably because something else in the compilation had gone wrong, so I am not sure that this correction is absolutely needed. However, it will also not harm to correct
2. CMakeLists.txt  The changes in this file are as follows: (a) in a number of places changes for MSVC are also needed for MINGW. These changes were copied from an existing MSYS2 compilation recipe, but instead of using the flag WIN32 I made it more specific (MINGW) so as not to interfere with other compilation procedures (e.g. Cygwin or others). (b) In addition, I defined two new variables that are set both under MSVC and MINGW. HAVE_WIN_TEMP expresses the fact that one has to look for the tmp directory using the Windows environmental variable TEMP, and is used in /libdispatch/ddispatch.c; HAVE_WIN_SSCANF expresses the fact that the Windows runtime library is used for the sscanf function (this is done both by MSVC and by MINGW). In the Windows implementation of sscanf, two differences occur: the specifier %hhu is not recognized, and NaN is not recognized as a special character sequence but results in an error if one assigns to a float or double. The flag HAVE_WIN_SSCANF is used in the files /lbdap2/dapcvt.c and /ncgen/cvt.c
3. config.h.cmake.in is changed in order to set the compiler flags HAVE_WIN_TEMP and HAVE_WIN_SSCANF with #cmakedefine commands
4. ncconfigure.h is changed in order to solve a problem with strlcat. This function is not available under MINGW, and the alternative formulation that belongs to the netcdf source could not be linked to certain functions because strlcat was not exported to the dll. I have changed the header in such a way that this export happens, but only when HAVE_STRLCAT is undefined. It should change nothing to builds where the function is natively present.

Note one point where interference with other builds is, in principle, possible. Some changes in the use of sscanf under MSVC were made specific to a certain version of MSVC in the file dapcvt.c, while I have now set the flag HAVE_WIN_SSCANF generic for all versions of MSVC. I did this because I could not find back any documentation from Microsoft suggesting that sscanf had been changed and the lacking functionality added in later versions. In addition, I think it very unlikely that lack of functionality was only present in a single version, not in earlier or later ones. However, I am not entirely certain about this and it is a matter of consideration

Note, further, that I only made changes to the CMake system, not to the config/autoconfig system as I did not use that and do not know very well how it functions.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 17, 2018

CLA assistant check
All committers have signed the CLA.

@WardF WardF self-assigned this Apr 12, 2018
@WardF WardF added this to the 4.7.0 milestone Apr 12, 2018
@WardF
Copy link
Copy Markdown
Member

WardF commented Apr 12, 2018

Having just finished a blocking task on our end, we are reviewing and catching up on pull requests. We will take care of the conflicts, etc, and review this, thank you!

@pmjherman
Copy link
Copy Markdown
Author

pmjherman commented Apr 13, 2018 via email

@DennisHeimbigner
Copy link
Copy Markdown
Collaborator

  1. I am in the process of rebuilding dapcvt.c, so those changes here may get overwritten.
  2. I do not understand the strlcat note; we provide a strlcat implementation when
    strlcat is not available; are you saying you use it or not?

@pmjherman
Copy link
Copy Markdown
Author

Dennis, with respect to strlcat.
When the function is natively available, my changes do nothing.
When the function is not natively available, the netcdf implementation of it is used. However, for the linker to be able to find this function in a Windows system, it needs to be exported explicitly into the dll. That has to be done in the header file. My changes only do this: pick the implementation of strlcat provided by netcdf (if it is not natively available) and make sure its code arrives in the dll so that it can be linked to throughout the netcdf compilation process.
Hope it is clearer now.
Peter

@WardF
Copy link
Copy Markdown
Member

WardF commented May 18, 2018

Hi, can you re-push without Windows line endings? It makes comparing the changes in CMakeLists.txt difficult :)

@WardF WardF modified the milestones: 4.6.2, 4.7.1 Apr 30, 2019
@WardF WardF modified the milestones: 4.7.1, 4.7.2 Aug 6, 2019
@WardF WardF closed this Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants