Patch #3090 - Fix and test S3 enabled builds

.github/workflows/run_tests_osx.yml

@@ -70,6 +70,7 @@ jobs:
         use_nc4: [ nc3, nc4 ]
         use_dap: [ dap_off, dap_on ]
         use_nczarr: [ nczarr_off, nczarr_on ]
+        use_s3: [ s3_off, s3_on ]
     steps:
 
       - uses: actions/checkout@v4
@@ -97,6 +98,12 @@ jobs:
         if: matrix.use_nczarr == 'nczarr_off'
       - run: echo "NETCDF_ENABLE_NCZARR=--enable-nczarr" >> $GITHUB_ENV
         if: matrix.use_nczarr == 'nczarr_on'
+      - run: echo "NETCDF_ENABLE_NCZARR=--disable-s3" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_off'
+      - run: |
+          echo "NETCDF_ENABLE_NCZARR=--enable-s3" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_NCZARR=--enable-s3-internal" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_on'
 
       ###
       # Fetch Cache
@@ -176,6 +183,7 @@ jobs:
         use_nc4: [ nc3, nc4 ]
         use_dap: [ dap_off, dap_on ]
         use_nczarr: [ nczarr_off, nczarr_on ]
+        use_s3: [ s3_off, s3_on ]
     steps:
 
       - uses: actions/checkout@v4
@@ -202,6 +210,12 @@ jobs:
         if: matrix.use_nczarr == 'nczarr_off'
       - run: echo "NETCDF_ENABLE_NCZARR=ON" >> $GITHUB_ENV
         if: matrix.use_nczarr == 'nczarr_on'
+      - run: echo "NETCDF_ENABLE_S3=OFF" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_off'
+      - run: |
+          echo "NETCDF_ENABLE_S3=ON" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3_INTERNAL=ON" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_on'
       - run: echo "CTEST_OUTPUT_ON_FAILURE=1" >> $GITHUB_ENV
 
     ###

.github/workflows/run_tests_ubuntu.yml

@@ -4,7 +4,7 @@
 
 name: Run Ubuntu/Linux netCDF Tests
 
-on: [workflow_dispatch]
+on: [pull_request, workflow_dispatch]
 
 env:
   REMOTETESTDOWN: ${{ vars.REMOTETESTDOWN }}
@@ -659,7 +659,7 @@ jobs:
         hdf5: [1.10.8, 1.12.2, 1.14.3 ]
         use_nc4: [ nc3, nc4 ]
         use_dap: [ dap_off, dap_on ]
-        use_nczarr: [ nczarr_off, nczarr_on ]
+        use_nczarr_and_s3: [ nczarr_off, nczarr_on, nczarr_on_s3_on ]
         runs-on: [ ubuntu-22.04 ]
 
     runs-on: ${{ matrix.runs-on }}
@@ -710,9 +710,14 @@ jobs:
       - run: echo "NETCDF_ENABLE_DAP=--enable-dap" >> $GITHUB_ENV
         if: matrix.use_dap == 'dap_on'
       - run: echo "NETCDF_ENABLE_NCZARR=--disable-nczarr" >> $GITHUB_ENV
-        if: matrix.use_nczarr == 'nczarr_off'
+        if: matrix.use_nczarr_and_s3 == 'nczarr_off'
       - run: echo "NETCDF_ENABLE_NCZARR=--enable-nczarr" >> $GITHUB_ENV
-        if: matrix.use_nczarr == 'nczarr_on'
+        if: matrix.use_nczarr_and_s3 == 'nczarr_on'
+      - run: |
+          echo "NETCDF_ENABLE_NCZARR=--enable-nczarr" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3=--enable-s3" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3_INTERNAL=--enable-s3-internal" >> $GITHUB_ENV
+        if: matrix.use_nczarr_and_s3 == 'nczarr_on_s3_on'
 
       ###
       # Fetch Cache
@@ -759,7 +764,7 @@ jobs:
           done
           current_directory="$(pwd)"
           mkdir ../build
-          cd ../build && CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS} LD_LIBRARY_PATH=${LD_LIBRARY_PATH} "${current_directory}/configure" ${NETCDF_ENABLE_HDF5} ${NETCDF_ENABLE_DAP} ${NETCDF_ENABLE_NCZARR}
+          cd ../build && CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS} LD_LIBRARY_PATH=${LD_LIBRARY_PATH} "${current_directory}/configure" ${NETCDF_ENABLE_HDF5} ${NETCDF_ENABLE_DAP} ${NETCDF_ENABLE_NCZARR} ${NETCDF_ENABLE_S3} ${NETCDF_ENABLE_S3_INTERNAL}
         if: ${{ success() }}
 
       - name: Look at config.log if error
@@ -827,6 +832,7 @@ jobs:
         use_nc4: [ nc3, nc4 ]
         use_dap: [ dap_off, dap_on ]
         use_nczarr: [ nczarr_off, nczarr_on ]
+        use_s3: [ s3_off, s3_on ]
     steps:
 
       - uses: actions/checkout@v4
@@ -855,6 +861,12 @@ jobs:
         if: matrix.use_nczarr == 'nczarr_off'
       - run: echo "NETCDF_ENABLE_NCZARR=ON" >> $GITHUB_ENV
         if: matrix.use_nczarr == 'nczarr_on'
+      - run: echo "NETCDF_ENABLE_S3=OFF" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_off'
+      - run: |
+          echo "NETCDF_ENABLE_S3=ON" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3_INTERNAL=ON" >> $GITHUB_ENV
+        if: matrix.use_s3 == 's3_on'
       - run: echo "CTEST_OUTPUT_ON_FAILURE=1" >> $GITHUB_ENV
 
       ###
@@ -881,7 +893,7 @@ jobs:
         run: |
           mkdir build
           cd build
-          LD_LIBRARY_PATH=${LD_LIBRARY_PATH} cmake .. -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH} -DNETCDF_ENABLE_DAP=${NETCDF_ENABLE_DAP} -DNETCDF_ENABLE_HDF5=${NETCDF_ENABLE_HDF5} -DNETCDF_ENABLE_NCZARR=${NETCDF_ENABLE_NCZARR}
+          LD_LIBRARY_PATH=${LD_LIBRARY_PATH} cmake .. -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH} -DNETCDF_ENABLE_DAP=${NETCDF_ENABLE_DAP} -DNETCDF_ENABLE_HDF5=${NETCDF_ENABLE_HDF5} -DNETCDF_ENABLE_NCZARR=${NETCDF_ENABLE_NCZARR} -DNETCDF_ENABLE_S3=${NETCDF_ENABLE_S3} -DNETCDF_ENABLE_S3_INTERNAL=${NETCDF_ENABLE_S3_INTERNAL}
 
       - name: Print Summary
         shell: bash -l {0}

.github/workflows/run_tests_win_cygwin.yml

@@ -24,7 +24,7 @@ jobs:
     strategy:
       matrix:
         plugin_dir_option: ["", "--without-plugin-dir"]
-
+        use_nczarr_and_s3: ["nczarr_off", "nczarr_on", "nczarr_on_s3_on" ]
     steps:
       - name: Fix line endings
         shell: pwsh
@@ -50,14 +50,25 @@ jobs:
           /bin/dash /usr/bin/libtoolize --force --copy --verbose
           /usr/bin/autoreconf-2.69 --force --install --verbose --debug
 
+###
+# Setup environment variables
+###
+      - run: echo "NETCDF_ENABLE_NCZARR=--disable-nczarr" >> $GITHUB_ENV
+        if: matrix.use_nczarr_and_s3 == 'nczarr_off'
+      - run: echo "NETCDF_ENABLE_NCZARR=--enable-nczarr" >> $GITHUB_ENV
+        if: matrix.use_nczarr_and_s3 == 'nczarr_on'
+      - run: |
+          echo "NETCDF_ENABLE_NCZARR=--enable-nczarr" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3=--enable-s3" >> $GITHUB_ENV
+          echo "NETCDF_ENABLE_S3_INTERNAL=--enable-s3-internal" >> $GITHUB_ENV
+        if: matrix.use_nczarr_and_s3 == 'nczarr_on_s3_on'
+
       - name: (Autotools) Configure in-tree build
         run: >-
           /bin/dash ./configure --enable-hdf5 --enable-shared
           --disable-static --enable-dap --disable-dap-remote-tests
           --enable-plugins ${{ matrix.plugin_dir_option }}
-          --disable-nczarr --disable-nczarr-filters
-          --disable-s3 --with-s3-testing=no
-
+          ${NETCDF_ENABLE_NCZARR} ${NETCDF_ENABLE_S3} ${NETCDF_ENABLE_S3_INTERNAL}
 
       - name: Look at config.log if error
         if: ${{ failure() }}
@@ -102,6 +113,10 @@ jobs:
     defaults:
       run:
         shell: C:/cygwin/bin/bash.exe -eo pipefail -o igncr "{0}"
+
+    strategy:
+      matrix:
+        enable_nczarr_and_s3: ["OFF", "ON"]      
 
     steps:
 
@@ -139,7 +154,9 @@ jobs:
           -DNETCDF_ENABLE_TESTS=ON \
           -DNETCDF_ENABLE_HDF5=ON \
           -DNETCDF_ENABLE_NCZARR=TRUE \
-          -DNETCDF_ENABLE_PLUGINS=ON
+          -DNETCDF_ENABLE_PLUGINS=ON \
+          -DNETCDF_ENABLE_S3=${{ matrix.enable_nczarr_and_s3 }} \
+          -DNETCDF_ENABLE_S3_INTERNAL=${{ matrix.enable_nczarr_and_s3 }} 
         if: ${{ success() }}
 
       - name: (CMake) Look at CMakeCache.txt if error

include/Makefile.am

@@ -10,7 +10,7 @@
 
 include_HEADERS = netcdf.h netcdf_meta.h netcdf_mem.h netcdf_aux.h	\
 netcdf_filter.h netcdf_filter_build.h netcdf_filter_hdf5_build.h 	\
-netcdf_dispatch.h
+netcdf_dispatch.h netcdf_vutils.h
 
 include_HEADERS += netcdf_json.h netcdf_proplist.h
 

libdispatch/ncutil.h → include/netcdf_vutils.h

@@ -1,8 +1,8 @@
-/* Copyright 2018, UCAR/Unidata and OPeNDAP, Inc.
+/* Copyright 2018, UCAR/Unidata
    See the COPYRIGHT file for more information. */
 
-#ifndef UTILS_H
-#define UTILS_H 1
+#ifndef NCVUTILS_H
+#define NCVUTILS_H 1
 
 /* Define a header-only simple version of a dynamically expandable list and byte buffer */
 /* To be used in code that should be independent of libnetcdf */
@@ -15,8 +15,8 @@ typedef struct VList {
 
 typedef struct VString {
   int nonextendible; /* 1 => fail if an attempt is made to extend this string*/
-  unsigned int alloc;
-  unsigned int length;
+  unsigned alloc;
+  unsigned length;
   char* content;
 } VString;
 
@@ -55,7 +55,7 @@ static void
 vlistexpand(VList* l)
 {
   void** newcontent = NULL;
-  size_t newsz;
+  unsigned newsz;
 
   if(l == NULL) return;
   newsz = (l->length * 2) + 1; /* basically double allocated space */
@@ -132,11 +132,11 @@ static void
 vsexpand(VString* vs)
 {
   char* newcontent = NULL;
-  size_t newsz;
+  unsigned newsz;
 
   if(vs == NULL) return;
   assert(vs->nonextendible == 0);
-  newsz = (vs->alloc + VSTRALLOC); /* basically double allocated space */
+  newsz = (vs->alloc + VSTRALLOC); /* increase allocated space */
   if(vs->alloc >= newsz) return; /* space already allocated */
   newcontent=(char*)calloc(1,newsz+1);/* always room for nul term */
   assert(newcontent != NULL);
@@ -154,7 +154,7 @@ vsappendn(VString* vs, const char* elem, unsigned n)
 {
   size_t need;
   assert(vs != NULL && elem != NULL);
-  if(n == 0) {n = strlen(elem);}
+  if(n == 0) {n = (unsigned)strlen(elem);}
   need = vs->length + n;
   if(vs->nonextendible) {
      /* Space must already be available */
@@ -166,7 +166,7 @@ vsappendn(VString* vs, const char* elem, unsigned n)
   memcpy(&vs->content[vs->length],elem,n);
   vs->length += n;
   if(!vs->nonextendible)
-      vs->content[vs->length] = '\0';
+      vs->content[vs->length] = '\0'; /* guarantee nul term */
 }
 
 static void
@@ -196,7 +196,12 @@ static char*
 vsextract(VString* vs)
 {
     char* x = NULL;
-    if(vs == NULL || vs->content == NULL) return NULL;
+    if(vs == NULL) return NULL;
+    if(vs->content == NULL) {
+	/* guarantee content existence and nul terminated */
+	if((vs->content = calloc(1,sizeof(char)))==NULL) return NULL;
+	vs->length = 0;
+    }
     x = vs->content;
     vs->content = NULL;
     vs->length = 0;
@@ -229,14 +234,14 @@ util_initialize(void)
 
 /* Following are always "in-lined"*/
 #define vlistcontents(l)  ((l)==NULL?NULL:(l)->content)
-#define vlistlength(l)  ((l)==NULL?0:(int)(l)->length)
+#define vlistlength(l)  ((l)==NULL?0:(l)->length)
 #define vlistclear(l)  vlistsetlength(l,0)
 #define vlistsetlength(l,len)  do{if((l)!=NULL) (l)->length=len;} while(0)
 
 #define vscontents(vs)  ((vs)==NULL?NULL:(vs)->content)
-#define vslength(vs)  ((vs)==NULL?0:(int)(vs)->length)
+#define vslength(vs)  ((vs)==NULL?0:(vs)->length)
 #define vscat(vs,s)  vsappendn(vs,s,0)
 #define vsclear(vs)  vssetlength(vs,0)
 #define vssetlength(vs,len)  do{if((vs)!=NULL) (vs)->length=len;} while(0)
 
-#endif /*UTILS_H*/
+#endif /*NCVUTIL_H*/

libdispatch/Makefile.am

@@ -51,7 +51,7 @@ endif # NETCDF_ENABLE_BYTERANGE
 if NETCDF_ENABLE_S3
 if NETCDF_ENABLE_S3_INTERNAL
 # Renamed to avoid conflicts with the HDF5 files
-libdispatch_la_SOURCES += ncs3sdk_h5.c nch5s3comms.c nch5s3comms.h ncutil.h nccurl_setup.h \
+libdispatch_la_SOURCES += ncs3sdk_h5.c nch5s3comms.c nch5s3comms.h nccurl_setup.h \
 			  nccurl_sha256.c nccurl_sha256.h nccurl_hmac.c nccurl_hmac.h
 AM_CPPFLAGS += -I$(top_srcdir)/libncxml
 libdispatch_la_CPPFLAGS += ${AM_CPPFLAGS}

libdispatch/dhttp.c

@@ -164,7 +164,7 @@ nc_http_close(NC_HTTP_STATE* state)
 #ifdef NETCDF_ENABLE_S3
     case HTTPS3: {
 	if(state->s3.s3client)
-            NC_s3sdkclose(state->s3.s3client, state->s3.info, 0, NULL);
+            NC_s3sdkclose(state->s3.s3client, NULL);
         NC_s3clear(state->s3.info);
 	nullfree(state->s3.info);
 	state->s3.s3client = NULL;

libdispatch/dutil.c

@@ -539,3 +539,47 @@ NC_joinwith(NClist* segments, const char* sep, const char* prefix, const char* s
     ncbytesfree(buf);
     return stat;
 }
+
+static int
+lexical_compare(const void* arg1, const void* arg2)
+{
+    char* s1 = *((char**)arg1);
+    char* s2 = *((char**)arg2);
+    size_t slen1 = nulllen(s1);
+    size_t slen2 = nulllen(s2);
+    if(slen1 != slen2) return (slen1 - slen2);
+    return strcmp(s1,s2);
+}
+
+/**
+Sort a vector of strings.
+@param n Number of strings to sort
+@param env vector of strings to sort
+*/
+void
+NC_sortenvv(size_t n, char** envv)
+{
+    if(n <= 1) return;
+    qsort(envv, (int)n, sizeof(char*), lexical_compare);
+}
+
+/**
+Sort a nclist of strings.
+@param l NClist of strings
+*/
+void
+NC_sortlist(NClist* l)
+{
+    if(l == NULL || nclistlength(l) == 0) return;
+    NC_sortenvv(nclistlength(l),(char**)nclistcontents(l));
+}
+
+/* Free up a vector of strings */
+void
+NC_freeenvv(size_t nkeys, char** keys)
+{
+    size_t i;
+    for(i=0;i<nkeys;i++)
+	nullfree(keys[i]);
+    nullfree(keys);
+}

libdispatch/ncs3sdk_aws.cpp

@@ -548,21 +548,13 @@ NC_s3sdkwriteobject(void* s3client0, const char* bucket, const char* pathkey,  s
 }
 
 EXTERNL int
-NC_s3sdkclose(void* s3client0, NCS3INFO* info, int deleteit, char** errmsgp)
+NC_s3sdkclose(void* s3client0, char** errmsgp)
 {
     int stat = NC_NOERR;
 
-    NCTRACE(11,"info=%s rootkey=%s deleteit=%d",dumps3info(info),deleteit);
-
     AWSS3CLIENT s3client = (AWSS3CLIENT)s3client0;
-    if(deleteit) {
-        /* Delete the root key; ok it if does not exist */
-        switch (stat = NC_s3sdkdeletekey(s3client0,info->bucket,info->rootkey,errmsgp)) {
-        case NC_NOERR: break;
-        case NC_EEMPTY: case NC_ENOTFOUND: stat = NC_NOERR; break;
-        default: break;
-        }
-    }
+    NCTRACE(11,"info=%s y=%s deleteit=%d",s3client.);
+
 #ifdef TRANSFER
     delete s3client;
 #else
@@ -654,7 +646,7 @@ Return a list of full keys  of legal objects immediately below a specified key.
 Not necessarily sorted.
 */
 EXTERNL int
-NC_s3sdkgetkeys(void* s3client0, const char* bucket, const char* prefixkey0, size_t* nkeysp, char*** keysp, char** errmsgp)
+NC_s3sdklist(void* s3client0, const char* bucket, const char* prefixkey0, size_t* nkeysp, char*** keysp, char** errmsgp)
 {
     return getkeys(s3client0, bucket, prefixkey0, "/", nkeysp, keysp, errmsgp);
 }