fixes for MacOS and new CI to prevent future breaks (#164)

jeanbez · github-actions[bot] · jeanbez · commit e0024e307ccd · 2024-01-08T22:56:58.000-08:00
* include MacOS in CI
* include dependencies
* replace TCP with sockets
* add libuuid
* fix find UUID to correctly locate files in Ubuntu and MacOS
* fix random segfault in strdup + malloc due to wrong allocation
* ensure consistent use of defined variables
* change transport for MacOS tests
* update env
* update dependencies-macos.sh
* replace found to TRUE/FALSE
* update documentation with timeout and MacOS specifics
* fix git link to avoid authentication
* change transport for tests
* configure network for MacOS tests

---------

Co-authored-by: github-actions &lt;github-actions[bot]@users.noreply.github.com&gt;
diff --git a/.github/workflows/dependencies-macos.sh b/.github/workflows/dependencies-macos.sh
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+
+set -eu -o pipefail
+
+export HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=FALSE
+
+brew install open-mpi automake
+
+# libfabric
+wget https://github.com/ofiwg/libfabric/archive/refs/tags/v1.15.2.tar.gz
+tar xf v1.15.2.tar.gz
+cd libfabric-1.15.2
+./autogen.sh
+./configure --disable-usnic --disable-mrail --disable-rstream --disable-perf --disable-efa --disable-psm2 --disable-psm --disable-verbs --disable-shm --disable-static --disable-silent-rules
+make -j2 && sudo make install
+make check
+cd ..
+
+# Mercury
+git clone --recursive https://github.com/mercury-hpc/mercury.git
+cd mercury
+git checkout v2.2.0
+mkdir build && cd build
+cmake ../  -DCMAKE_C_COMPILER=gcc -DBUILD_SHARED_LIBS=ON -DBUILD_TESTING=ON -DNA_USE_OFI=ON -DNA_USE_SM=OFF -DMERCURY_USE_CHECKSUMS=OFF -DNA_OFI_TESTING_PROTOCOL=sockets
+make -j2 && sudo make install
+ctest
diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
@@ -0,0 +1,33 @@
+name: MacOS
+
+on:
+  pull_request:
+    branches: [ stable, develop ]
+
+  # Allows to run this workflow manually from the Actions tab
+  workflow_dispatch:
+
+jobs:
+  PDC:
+    runs-on: macos-13
+    timeout-minutes: 60
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Dependencies
+        run: .github/workflows/dependencies-macos.sh
+
+      - name: Build PDC
+        run: |
+          mkdir build && cd build
+          cmake ../ -DBUILD_MPI_TESTING=ON -DBUILD_SHARED_LIBS=ON -DBUILD_TESTING=ON -DPDC_ENABLE_MPI=ON -DCMAKE_C_COMPILER=mpicc
+          make -j 2
+
+      - name: Test PDC
+        working-directory: build
+        run: |
+          sudo sh -c 'echo "`ipconfig getifaddr en0` PDC" >> /etc/hosts'
+          sudo scutil --set HostName PDC
+          export HG_TRANSPORT="sockets"
+          ctest -L serial
diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml
@@ -1,8 +1,6 @@
-name: linux 
+name: Ubuntu
 
 on:
-  # push:
-  #   branches: [ stable ]
   pull_request:
     branches: [ stable, develop ]
 
@@ -27,14 +25,6 @@ jobs:
           cmake ../ -DBUILD_MPI_TESTING=ON -DBUILD_SHARED_LIBS=ON -DBUILD_TESTING=ON -DPDC_ENABLE_MPI=ON -DCMAKE_C_COMPILER=mpicc
           make -j2
 
-      # - name: Debug test PDC
-      #   working-directory: ./src/build/bin
-      #   run: |
-      #     mpirun -n 1 ./pdc_server.exe &
-      #     sleep 1
-      #     mpirun -n 1 ./pdc_init
-      #     mpirun -n 1 ./close_server
-
       - name: Test PDC
         working-directory: build
         run: ctest -L serial
diff --git a/CMake/FindMERCURY.cmake b/CMake/FindMERCURY.cmake
@@ -28,9 +28,9 @@ if(MERCURY_FOUND)
   )
 
   find_library(MERCURY_UTIL_LIBRARY
-    NAMES
-    mercury_util
-    HINTS ${MERCURY_DIR}
+      NAMES
+      mercury_util
+      HINTS ${MERCURY_DIR}
   )
 
   set(MERCURY_LIBRARIES ${MERCURY_LIBRARY} ${MERCURY_NA_LIBRARY} ${MERCURY_UTIL_LIBRARY})
diff --git a/CMake/FindUUID.cmake b/CMake/FindUUID.cmake
@@ -1,36 +1,41 @@
-# FindUUID.cmake
+# On Mac OS X the uuid functions are in the System library
 
-# Find the system's UUID library
-# This will define:
-#
-# UUID_FOUND - System has UUID
-# UUID_INCLUDE_DIRS - The UUID include directory
-# UUID_LIBRARIES - The libraries needed to use UUID
+if(APPLE)
+  set(UUID_LIBRARY_VAR System)
+else()
+  set(UUID_LIBRARY_VAR uuid)
+endif()
 
-# - Try to find UUID
-# Once done this will define
-#  UUID_FOUND - System has UUID
-#  UUID_INCLUDE_DIRS - The UUID include directories
-#  UUID_LIBRARIES - The libraries needed to use UUID
-
-find_package(PkgConfig)
-pkg_check_modules(PC_UUID uuid)
+find_library(UUID_LIBRARY
+  NAMES ${UUID_LIBRARY_VAR}
+  PATHS /usr/local/lib64 /usr/local/lib /usr/lib64 /usr/lib
+)
 
 find_path(UUID_INCLUDE_DIR uuid/uuid.h
   HINTS ${PC_UUID_INCLUDEDIR} ${PC_UUID_INCLUDE_DIRS}
-  PATHS /usr/local/include /usr/include)
-
-find_library(UUID_LIBRARY NAMES uuid
-  HINTS ${PC_DRC_LIBDIR} ${PC_DRC_LIBRARY_DIRS}
-  PATHS /usr/local/lib64 /usr/local/lib /usr/lib64 /usr/lib)
-
-set(UUID_INCLUDE_DIRS ${UUID_INCLUDE_DIR})
-set(UUID_LIBRARIES ${UUID_LIBRARY})
-
-include(FindPackageHandleStandardArgs)
-# handle the QUIETLY and REQUIRED arguments and set UUID_FOUND to TRUE
-# if all listed variables are TRUE
-find_package_handle_standard_args(UUID DEFAULT_MSG
-                                  UUID_INCLUDE_DIR UUID_LIBRARY)
-
-mark_as_advanced(UUID_INCLUDE_DIR UUID_LIBRARY)
+  PATHS /usr/local/include /usr/include
+)
+
+if (UUID_LIBRARY AND UUID_INCLUDE_DIR)
+  set(UUID_LIBRARIES ${UUID_LIBRARY})
+  set(UUID_FOUND "TRUE")
+else ()
+  set(UUID_FOUND "FALSE")
+endif ()
+
+if (UUID_FOUND)
+   if (NOT UUID_FIND_QUIETLY)
+      message(STATUS "Found UUID: ${UUID_LIBRARIES}")
+   endif ()
+else ()
+   if (UUID_FIND_REQUIRED)
+      message( "library: ${UUID_LIBRARY}" )
+      message( "include: ${UUID_INCLUDE_DIR}" )
+      message(FATAL_ERROR "Could not find UUID library")
+   endif ()
+endif ()
+
+mark_as_advanced(
+  UUID_LIBRARY
+  UUID_INCLUDE_DIR
+)
diff --git a/docs/source/getting_started.rst b/docs/source/getting_started.rst
@@ -42,6 +42,7 @@ PDC can use either MPICH or OpenMPI as the MPI library, if your system doesn't h
 We provide detailed instructions for installing libfabric, Mercury, and PDC below.
 
 .. attention:: 
+
 	Following the instructions below will record all the environmental variables needed to run PDC in the ``$WORK_SPACE/pdc_env.sh`` file, which can be used for future PDC runs with ``source $WORK_SPACE/pdc_env.sh``.
 
 
@@ -57,9 +58,9 @@ Before installing the dependencies and downloading the code repository, we assum
 	mkdir -p $WORK_SPACE/install
 
 	cd $WORK_SPACE/source
-	git clone git@github.com:ofiwg/libfabric.git
-	git clone git@github.com:mercury-hpc/mercury.git --recursive
-	git clone git@github.com:hpc-io/pdc.git
+	git clone https://github.com/ofiwg/libfabric
+	git clone https://github.com/mercury-hpc/mercury --recursive
+	git clone https://github.com/hpc-io/pdc
 
 	export LIBFABRIC_SRC_DIR=$WORK_SPACE/source/libfabric
 	export MERCURY_SRC_DIR=$WORK_SPACE/source/mercury
@@ -118,9 +119,18 @@ Install libfabric
 
 
 .. note::
+
 	``CC=mpicc`` may need to be changed to the corresponding compiler in your system, e.g. ``CC=cc`` or ``CC=gcc``.
 	On Perlmutter@NERSC, ``--disable-efa --disable-sockets`` should be added to the ``./configure`` command when compiling on login nodes.
 
+.. attention::
+
+	If you're installing PDC on MacOS, you need to make sure you enable ``sockets``:
+
+	.. code-block: Bash
+
+		./configure CFLAG=-O2 --enable-sockets=yes --enable-tcp=yes --enable-udp=yes --enable-rxm=yes
+
 
 Install Mercury
 ---------------
@@ -149,6 +159,15 @@ Install Mercury
 	``CC=mpicc`` may need to be changed to the corresponding compiler in your system, e.g. ``-DCMAKE_C_COMPILER=cc`` or ``-DCMAKE_C_COMPILER=gcc``.
 	Make sure the ctest passes. PDC may not work without passing all the tests of Mercury.
 
+.. attention::
+
+	If you're installing PDC on MacOS, for the tests to work you need to specify the protocol used by Mercury:
+
+	.. code-block: Bash
+
+		cmake -DCMAKE_INSTALL_PREFIX=$MERCURY_DIR -DCMAKE_C_COMPILER=mpicc -DBUILD_SHARED_LIBS=ON \
+	      -DBUILD_TESTING=ON -DNA_USE_OFI=ON -DNA_USE_SM=OFF -DNA_OFI_TESTING_PROTOCOL=sockets
+
 
 Install PDC
 -----------
@@ -170,12 +189,21 @@ Install PDC
 	echo 'export PATH=$PDC_DIR/include:$PDC_DIR/lib:$PATH' >> $WORK_SPACE/pdc_env.sh
 
 .. note::
+
 	``-DCMAKE_C_COMPILER=mpicc -DMPI_RUN_CMD=mpiexec`` may need to be changed to ``-DCMAKE_C_COMPILER=cc -DMPI_RUN_CMD=srun`` depending on your system environment.
 
 .. note::
-	If you are trying to compile PDC on your Mac, ``LibUUID`` needs to be installed on your MacOS first. Simple use ``brew install ossp-uuid`` to install it.
+
+	If you are trying to compile PDC on MacOS, ``LibUUID`` needs to be installed on your MacOS first. Simple use ``brew install ossp-uuid`` to install it.
 	If you are trying to compile PDC on Linux, you should also make sure ``LibUUID`` is installed on your system. If not, you can install it with ``sudo apt-get install uuid-dev`` on Ubuntu or ``yum install libuuid-devel`` on CentOS.
 
+	In MacOS you also need to export the following environment variable so PDC (i.e., Mercury) uses the ``socket`` protocol, the only one supported in MacOS:
+
+	.. code-block: Bash
+
+		export HG_TRANSPORT="sockets"
+
+
 Test Your PDC Installation
 --------------------------
 PDC's ``ctest`` contains both sequential and parallel/MPI tests, and can be run with the following in the `build` directory.
@@ -184,6 +212,12 @@ PDC's ``ctest`` contains both sequential and parallel/MPI tests, and can be run
 
 	ctest
 
+You can also specify a timeout (e.g., 2 minutes) for the tests by specifying the ``timeout`` parameter when calling ``ctest``:
+
+.. code-block:: Bash
+
+	ctest --timeout 120
+
 .. note::
 	If you are using PDC on an HPC system, e.g. Perlmutter@NERSC, ``ctest`` should be run on a compute node, you can submit an interactive job on Perlmutter: ``salloc --nodes 1 --qos interactive --time 01:00:00 --constraint cpu --account=mxxxx``
 
diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt
@@ -42,7 +42,7 @@ if(MERCURY_FOUND)
   set(PDC_EXT_INCLUDE_DEPENDENCIES ${MERCURY_INCLUDE_DIR}
     ${PDC_EXT_INCLUDE_DEPENDENCIES}
     )
-  set(PDC_EXT_LIB_DEPENDENCIES mercury ${PDC_EXT_LIB_DEPENDENCIES})
+  set(PDC_EXT_LIB_DEPENDENCIES ${MERCURY_LIBRARIES} ${PDC_EXT_LIB_DEPENDENCIES})
 endif()
 
 include_directories(${PDC_EXT_INCLUDE_DEPENDENCIES})
diff --git a/src/api/pdc_obj/pdc_cont.c b/src/api/pdc_obj/pdc_cont.c
@@ -86,6 +86,7 @@ PDCcont_create(const char *cont_name, pdcid_t cont_prop_id)
     p->cont_pt->pdc->local_id = cont_prop->pdc->local_id;
 
     ret = PDC_Client_create_cont_id(cont_name, cont_prop_id, &(p->cont_info_pub->meta_id));
+
     if (ret == FAIL)
         PGOTO_ERROR(0, "Unable to create container on the server!");
 
@@ -316,8 +317,10 @@ PDC_cont_get_info(pdcid_t cont_id)
     FUNC_ENTER(NULL);
 
     id_info = PDC_find_id(cont_id);
-    info    = (struct _pdc_cont_info *)(id_info->obj_ptr);
+    if (id_info == NULL)
+        PGOTO_ERROR(NULL, "cannot locate object");
 
+    info      = (struct _pdc_cont_info *)(id_info->obj_ptr);
     ret_value = PDC_CALLOC(1, struct _pdc_cont_info);
     if (ret_value)
         memcpy(ret_value, info, sizeof(struct _pdc_cont_info));
@@ -326,9 +329,8 @@ PDC_cont_get_info(pdcid_t cont_id)
 
     ret_value->cont_info_pub = PDC_CALLOC(1, struct pdc_cont_info);
     if (ret_value->cont_info_pub)
-        memcpy(ret_value, info, sizeof(struct pdc_cont_info));
-    else
-        PGOTO_ERROR(NULL, "cannot allocate ret_value->cont_info_pub");
+        memcpy(ret_value->cont_info_pub, info->cont_info_pub, sizeof(struct pdc_cont_info));
+
     if (info->cont_info_pub->name)
         ret_value->cont_info_pub->name = strdup(info->cont_info_pub->name);
 
diff --git a/src/server/pdc_server_metadata.c b/src/server/pdc_server_metadata.c
@@ -969,7 +969,7 @@ PDC_Server_delete_metadata_by_id(metadata_delete_by_id_in_t *in, metadata_delete
                 continue;
 
             if (cont_entry->cont_id == target_obj_id) {
-                hash_table_remove(container_hash_table_g, &pair.key);
+                hash_table_remove(container_hash_table_g, pair.key);
                 out->ret  = 1;
                 ret_value = SUCCEED;
                 goto done;
diff --git a/src/tests/cont_del.c b/src/tests/cont_del.c
@@ -80,7 +80,7 @@ main(int argc, char **argv)
     }
 
     printf("trying to open a deleted container, should fail\n");
-    cont = PDCcont_open("VPIC_cont", pdc);
+    cont = PDCcont_open(cont_name, pdc);
     if (cont > 0)
         printf("Error: opened a container that was just deleted @ line  %d!\n", __LINE__);
 

Original file line number	Diff line number	Diff line change
`@@ -28,9 +28,9 @@ if(MERCURY_FOUND)`
`28`	`28`	`)`
`29`	`29`
`30`	`30`	`find_library(MERCURY_UTIL_LIBRARY`
`31`		`- NAMES`
`32`		`- mercury_util`
`33`		`- HINTS ${MERCURY_DIR}`
	`31`	`+ NAMES`
	`32`	`+ mercury_util`
	`33`	`+ HINTS ${MERCURY_DIR}`
`34`	`34`	`)`
`35`	`35`
`36`	`36`	`set(MERCURY_LIBRARIES ${MERCURY_LIBRARY} ${MERCURY_NA_LIBRARY} ${MERCURY_UTIL_LIBRARY})`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ if(MERCURY_FOUND)`
`42`	`42`	`set(PDC_EXT_INCLUDE_DEPENDENCIES ${MERCURY_INCLUDE_DIR}`
`43`	`43`	`${PDC_EXT_INCLUDE_DEPENDENCIES}`
`44`	`44`	`)`
`45`		`- set(PDC_EXT_LIB_DEPENDENCIES mercury ${PDC_EXT_LIB_DEPENDENCIES})`
	`45`	`+ set(PDC_EXT_LIB_DEPENDENCIES ${MERCURY_LIBRARIES} ${PDC_EXT_LIB_DEPENDENCIES})`
`46`	`46`	`endif()`
`47`	`47`
`48`	`48`	`include_directories(${PDC_EXT_INCLUDE_DEPENDENCIES})`
Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,7 @@ main(int argc, char **argv)`
`80`	`80`	`}`
`81`	`81`
`82`	`82`	`printf("trying to open a deleted container, should fail\n");`
`83`		`- cont = PDCcont_open("VPIC_cont", pdc);`
	`83`	`+ cont = PDCcont_open(cont_name, pdc);`
`84`	`84`	`if (cont > 0)`
`85`	`85`	`printf("Error: opened a container that was just deleted @ line %d!\n", __LINE__);`
`86`	`86`