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.

Sign up for GitHub

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

Jump to bottom

Cause of failure and a workaround to build with --enable-nczarr-s3 on MacOS (+Windows?) #2521

Open
czender opened this issue Sep 30, 2022 · 20 comments
Open

Cause of failure and a workaround to build with --enable-nczarr-s3 on MacOS (+Windows?) #2521

czender opened this issue Sep 30, 2022 · 20 comments
Assignees
@WardF
Milestone
4.9.3

Comments

@czender
Copy link
Contributor

czender commented Sep 30, 2022

This is based on today's snapshot of 4.9.1-develop.

The netCDF-C distribution, configured with --enable-nczarr-s3, FTBFS with Clang (and possibly GCC) on MacOS even though the Homebrew package aws-cpp-sdk provides the necessary libraries aws-cpp-sdk-s3 and aws-cpp-sdk-core. The failure is apparently caused because MacOS by default installs a quasi case-insensitive filesystem that causes the netCDF top-level-directory file VERSION to be #include'd into many system C++ files such as cstddef that try to #include <version> (example output follows). They get VERSION instead of version and the compilation fails.

The full potential of NCZarr will not be reached until we can more easily build netCDF with --enable-nczarr-s3 on MacOS and Windows (both case-insensitive filesystems by default). I think the current failure could be prevented with a compiler setting or code modification. But I ain't smart 'nuff to figger out how. So this is a challenge for you! A related StackOverflow article is here

In the meantime, my laughably primitive workaround to build with --enable-nczarr-s3 on MacOS is:

  1. CC='clang' CPPFLAGS="-I/opt/homebrew/include" LDFLAGS="-L/opt/homebrew/lib -laws-cpp-sdk-s3" ./configure --enable-nczarr-s3 ...
  2. make [this breaks as described above/below]
  3. mv VERSION VERSION.orig
  4. make && sudo make install [this completes!]
  5. mv VERSION.orig VERSION
  6. make check [passes!]

Without that workaround, I get a ton of failures, all like this:

gmake  all-recursive
gmake[1]: Entering directory '/Users/zender/netcdf-c'
Making all in include
gmake[2]: Entering directory '/Users/zender/netcdf-c/include'
gmake[2]: Nothing to be done for 'all'.
gmake[2]: Leaving directory '/Users/zender/netcdf-c/include'
Making all in libncxml
gmake[2]: Entering directory '/Users/zender/netcdf-c/libncxml'
gmake[2]: Nothing to be done for 'all'.
gmake[2]: Leaving directory '/Users/zender/netcdf-c/libncxml'
Making all in libdispatch
gmake[2]: Entering directory '/Users/zender/netcdf-c/libdispatch'
/bin/sh ../libtool  --tag=CXX   --mode=compile g++ -DHAVE_CONFIG_H -I. -I..  -I../include -I../include -I../oc2
 -I../libnczarr -I/opt/homebrew/include -std=c++11 -g -O2 -MT libdispatch_la-ncs3sdk.lo -MD -MP -MF .deps/libdi
spatch_la-ncs3sdk.Tpo -c -o libdispatch_la-ncs3sdk.lo `test -f 'ncs3sdk.cpp' || echo './'`ncs3sdk.cpp
libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I.. -I../include -I../include -I../oc2 -I../libnczarr -I/opt/homebr
ew/include -std=c++11 -g -O2 -MT libdispatch_la-ncs3sdk.lo -MD -MP -MF .deps/libdispatch_la-ncs3sdk.Tpo -c ncs3
sdk.cpp  -fno-common -DPIC -o .libs/libdispatch_la-ncs3sdk.o
In file included from ncs3sdk.cpp:11:
In file included from ./awsincludes.h:8:
In file included from /opt/homebrew/include/aws/core/Aws.h:7:
In file included from /opt/homebrew/include/aws/core/utils/logging/LogLevel.h:10:
In file included from /opt/homebrew/include/aws/core/utils/memory/stl/AWSString.h:10:
In file included from /opt/homebrew/include/aws/core/utils/memory/stl/AWSAllocator.h:11:
In file included from /opt/homebrew/include/aws/core/utils/memory/AWSMemory.h:12:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOS
X.sdk/usr/include/c++/v1/memory:671:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOS
X.sdk/usr/include/c++/v1/__functional_base:15:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__functional/invoke.h:14:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__functional/weak_result_type.h:16:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/type_traits:420:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/cstddef:37:
../version:1:1: error: expected unqualified-id
4.9.1-development
^
@DennisHeimbigner
Copy link
Collaborator

Does it also build if you copy VERSION to a file called "version" (lowercase)?

@czender
Copy link
Contributor Author

czender commented Sep 30, 2022

Good question. No, it fails with the same error as above. The compiler just finds the netCDF version file instead of the intended, Clang version file.

@DennisHeimbigner
Copy link
Collaborator

In any case, the problem (from my point of view) is that
aws-sdk-cpp is way overkill for what netcdf-c needs to access S3.
My belief is that we need to build our own replacement library with limited
functionality. I also believe that we can take code from HDF5
and other places to construct that replacement library.
But it still requires significant programming effort.
I keep hoping someone else will build that small footprint
library, but so far no luck.

@dopplershift
Copy link
Member

Unless relying on the AWS C SDK is completely untenable (and I've heard inconsistent reports on whether that's the case), I'd really like to see us avoid expending engineering effort on a library to support an API we have no control over and isn't our unique specialty. If we're maintaining that, then we have to maintain that knowledge internally as well.

@DennisHeimbigner
Copy link
Collaborator

I think that Charlie's point is correct:

...The full potential of NCZarr will not be reached until we can more easily build netCDF with --enable-nczarr-s3 on MacOS and Windows...

If we adopt your point of view, then we have no choice but to wait
until some better solution becomes available.

@dopplershift
Copy link
Member

dopplershift commented Oct 4, 2022 •
edited
Loading

@DennisHeimbigner Well, let's not hold up creating a library to handle remote access to the S3 API, including all of the authentication options that people expect for S3, as some kind of "quick fix" either, especially given the engineering resources we have to devote to that.

I'd rather see us continue to invest in trying to use existing libraries like aws-c-s3, given that it already exists and comes from AWS itself AFAICT. It's already packaged for conda-forge too--available on Linux, macOS, and Windows.

While last I heard, there were problems setting up a dev environment around it, unless there are fundamental shortcomings in functionality there, I'd much rather see us invest limited engineering resources in bringing an existing open source library (and improving it as necessary) to solve the problem rather than do our own thing that we have to maintain on our own. It looks like the only time we reached out to them, they were responsive, so that's a positive sign.

@DennisHeimbigner
Copy link
Collaborator

Charlie- I assume that you got aws-cpp-sdk to build on OSX, correct?
If so, what Cmake options did you use?

@czender
Copy link
Contributor Author

czender commented Oct 4, 2022

On OSX, I simply installed and used the Homebrew package aws-cpp-sdk. I did not build it from scratch.

@DennisHeimbigner
Copy link
Collaborator

DennisHeimbigner commented Oct 8, 2022 •
edited
Loading

Two other questions.

  1. Did you get the S3 related tests to work correctly?
  2. What is the exact homebrew command you used to install aws-cpp-sdk?

@czender
Copy link
Contributor Author

czender commented Oct 8, 2022

  1. I did not attempt to run the netCDF-C tests for S3 since I thought those only worked for Unidata peeps. make check did pass without errors.
  2. brew install aws-cpp-sdk. It's an 800 MB package!

@DennisHeimbigner
Copy link
Collaborator

You are right, I forgot that our S3 repo is private.

It's an 800 MB package!

Wow!

@DennisHeimbigner
Copy link
Collaborator

WRT the original VERSION issue. I could rename the netcdf-c VERSION file
except that I do not know if it is referenced outside the library build.
If so, then it could break the workflows of other users.

@czender
Copy link
Contributor Author

czender commented Oct 8, 2022

True. Also true that VERSION is impeding NCZarr on S3 and the MacOS/Clang system library header named version will be renamed. Is there a way to temporarily rename VERSION at the beginning of the netCDF S3 build process then restore it later? Or to tell the header search algorithm to ignore the top-level netCDF directory? Otherwise a permanent renaming or moving of VERSION seems inevitable.

@DennisHeimbigner
Copy link
Collaborator

I do not use OSX except via github actions. But Ward and Ryan do.
I will discuss it with them.

@DennisHeimbigner
Copy link
Collaborator

I think there is another way to do this, but it may be just as intrusive.
If we move config.h into a subdirectory -- netcdf-c/include, say --
then we can remove the -I.. from the build so that VERSION is invisible.

@DennisHeimbigner
Copy link
Collaborator

I wonder why we do not see this under Windows, which is also case
insensitive?

@czender
Copy link
Contributor Author

czender commented Oct 9, 2022

My guess is that only the MacOS/Clang system headers need to #include a file named version. It's not a portable name for source code.

@DennisHeimbigner
Copy link
Collaborator

DennisHeimbigner commented Oct 9, 2022 •
edited
Loading

Apparently, the VERSION file is not an installed file.
This means that we should be able to change its name
as we wish with no impact on users.
Of course it is possible that some package managers (apt, brew, etc)
do look at it, in which case there would be problems.
I also note that cmake does not appear to create the VERSION file
at all.

@DennisHeimbigner
Copy link
Collaborator

I did notice that g++ also has a "version" file.

DennisHeimbigner added a commit to DennisHeimbigner/netcdf-c that referenced this issue Oct 10, 2022
@DennisHeimbigner
Move construction of VERSION file to end of the build
52a4eca 
re: Unidata#2521

Charlie Zender has discovered that the netcdf created file VERSION
conflicts with the C++ version file on OSX case-insensitive file systems,
and maybe other case-insensitvie file systems.

Note:
1. Cmake does not create the VERSION file
2. The VERSION file is not installed
3. It turns out that the VERSION file is not required by the autoconf build.

It is possible that clients or package build system (e.g apt or brew)
might use the VERSION file, so we cannot delete it altogether.

So as a fix, we move the creation of the VERSION file to after the
build is complete by inserting a all-local hook into netcdf-c/Makefile.am.

# Misc. other changes
1. Suppressed warning by making use of the systeminfo command contingent on the platform being Windows.
@DennisHeimbigner DennisHeimbigner mentioned this issue Oct 10, 2022
Merged
@DennisHeimbigner
Copy link
Collaborator

Possibly fixed by PR #2527.

@WardF WardF self-assigned this Oct 11, 2022
@WardF WardF added this to the 4.9.1 milestone Oct 11, 2022
@WardF WardF moved this to Todo in netCDF-C v4.9.1 Oct 11, 2022
@WardF WardF modified the milestones: 4.9.1, 4.9.2 Feb 13, 2023
@WardF WardF modified the milestones: 4.9.2, 4.9.3 May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@WardF WardF

Labels
None yet
Projects
No open projects
netCDF-C v4.9.1
Status: Todo
Milestone
4.9.3
Development

No branches or pull requests
4 participants
@dopplershift @DennisHeimbigner @WardF @czender