From aabaa87c7d9232277dd6a2ab5c206f56df3086e3 Mon Sep 17 00:00:00 2001
From: Ben Cumming <bcumming@cscs.ch>
Date: Fri, 26 Nov 2021 11:10:29 +0100
Subject: [PATCH] Test separately built catalogues (#1748)
* add a test that tests that a catalogue built separately can be loaded via the Python interface
* further simplification of dynamic library support
* move all platform-specific code into the cpp implementation and out of header.
---
.github/workflows/basic.yml | 6 ++--
CMakeLists.txt | 8 +----
arbor/CMakeLists.txt | 1 +
arbor/mechcat.cpp | 18 +++++------
arbor/util/dl.hpp | 5 ---
arbor/util/dl_platform_posix.hpp | 52 --------------------------------
arbor/util/dylib.cpp | 50 ++++++++++++++++++++++++++++++
arbor/util/dylib.hpp | 26 ++++++++++++++++
scripts/test-catalogue.py | 21 +++++++++++++
9 files changed, 112 insertions(+), 75 deletions(-)
delete mode 100644 arbor/util/dl.hpp
delete mode 100644 arbor/util/dl_platform_posix.hpp
create mode 100644 arbor/util/dylib.cpp
create mode 100644 arbor/util/dylib.hpp
create mode 100755 scripts/test-catalogue.py
diff --git a/.github/workflows/basic.yml b/.github/workflows/basic.yml
index 8b7f7373..7fe497c5 100644
--- a/.github/workflows/basic.yml
+++ b/.github/workflows/basic.yml
@@ -178,5 +178,7 @@ jobs:
run: mpirun -n 4 -oversubscribe python3 -m unittest discover -v -s python
- name: Run Python examples
run: scripts/run_python_examples.sh
- - name: Build a catalogue
- run: build-catalogue -v default mechanisms/default
+ - name: Build and test a catalogue
+ run: |
+ build-catalogue -v default mechanisms/default
+ ./scripts/test-catalogue.py ./default-catalogue.so
diff --git a/CMakeLists.txt b/CMakeLists.txt
index a2d3ce90..ccdfe0db 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -186,13 +186,7 @@ install(TARGETS arbor-config-defs EXPORT arbor-targets)
# Interface library `arbor-private-deps` collects dependencies, options etc.
# for the arbor library.
add_library(arbor-private-deps INTERFACE)
-target_link_libraries(arbor-private-deps INTERFACE arbor-config-defs ext-random123)
-if (UNIX)
- target_compile_definitions(arbor-config-defs INTERFACE ARB_HAVE_DL)
- target_link_libraries(arbor-private-deps INTERFACE ${CMAKE_DL_LIBS})
-else()
- message(FATAL_ERROR "No support for dynamic loading on current platform.")
-endif ()
+target_link_libraries(arbor-private-deps INTERFACE arbor-config-defs ext-random123 ${CMAKE_DL_LIBS})
install(TARGETS arbor-private-deps EXPORT arbor-targets)
# Interface library `arborenv-private-deps` collects dependencies, options etc.
diff --git a/arbor/CMakeLists.txt b/arbor/CMakeLists.txt
index 9296d2ac..b4c1b994 100644
--- a/arbor/CMakeLists.txt
+++ b/arbor/CMakeLists.txt
@@ -55,6 +55,7 @@ set(arbor_sources
threading/threading.cpp
thread_private_spike_store.cpp
tree.cpp
+ util/dylib.cpp
util/hostname.cpp
util/unwind.cpp
version.cpp
diff --git a/arbor/mechcat.cpp b/arbor/mechcat.cpp
index 829339fd..3796b46a 100644
--- a/arbor/mechcat.cpp
+++ b/arbor/mechcat.cpp
@@ -12,10 +12,11 @@
#include <arbor/mechanism.hpp>
#include <arbor/util/expected.hpp>
-#include "util/rangeutil.hpp"
+#include "util/dylib.hpp"
#include "util/maputil.hpp"
+#include "util/rangeutil.hpp"
#include "util/span.hpp"
-#include "util/dl.hpp"
+#include "util/strprintf.hpp"
/* Notes on implementation:
*
@@ -587,18 +588,17 @@ const mechanism_catalogue& load_catalogue(const std::string& fn) {
typedef const void* global_catalogue_t();
global_catalogue_t* get_catalogue = nullptr;
try {
- auto plugin = dl_open(fn);
- get_catalogue = dl_get_symbol<global_catalogue_t*>(plugin, "get_catalogue");
- } catch(dl_error& e) {
+ get_catalogue = util::dl_get_symbol<global_catalogue_t*>(fn, "get_catalogue");
+ } catch(util::dl_error& e) {
throw bad_catalogue_error{e.what(), {e}};
}
if (!get_catalogue) {
throw bad_catalogue_error{util::pprintf("Unusable symbol 'get_catalogue' in shared object '{}'", fn)};
}
- /* NOTE We do not free the DSO handle here and accept retaining the handles
- until termination since the mechanisms provided by the catalogue may have
- a different lifetime than the actual catalogue itfself. This is not a
- leak proper as `dlopen` caches handles for us.
+ /* The DSO handle is not freed here: handles will be retained until
+ * termination since the mechanisms provided by the catalogue may have a
+ * different lifetime than the actual catalogue itfself. This is not a leak,
+ * as `dlopen` caches handles for us.
*/
return *((const mechanism_catalogue*)get_catalogue());
}
diff --git a/arbor/util/dl.hpp b/arbor/util/dl.hpp
deleted file mode 100644
index 8454036e..00000000
--- a/arbor/util/dl.hpp
+++ /dev/null
@@ -1,5 +0,0 @@
-#pragma once
-
-#ifdef ARB_HAVE_DL
-#include "util/dl_platform_posix.hpp"
-#endif
diff --git a/arbor/util/dl_platform_posix.hpp b/arbor/util/dl_platform_posix.hpp
deleted file mode 100644
index e2442c15..00000000
--- a/arbor/util/dl_platform_posix.hpp
+++ /dev/null
@@ -1,52 +0,0 @@
-#pragma once
-
-#include <fstream>
-#include <string>
-
-#include <dlfcn.h>
-
-#include <arbor/arbexcept.hpp>
-
-#include "util/strprintf.hpp"
-
-namespace arb {
-
-struct dl_error: arbor_exception {
- dl_error(const std::string& msg): arbor_exception{msg} {}
-};
-
-struct dl_handle {
- void* dl = nullptr;
-};
-
-inline
-dl_handle dl_open(const std::string& fn) {
- // Test if we can open file
- if (!std::ifstream{fn.c_str()}.good()) throw file_not_found_error{fn};
- // Call once to clear errors not caused by us
- dlerror();
- // Try to open shared object
- auto result = dlopen(fn.c_str(), RTLD_LAZY);
- // dlopen fails by returning NULL
- if (!result) throw dl_error{util::pprintf("[POSIX] dl_open failed with: {}", dlerror())};
- return {result};
-}
-
-template<typename T> inline
-T dl_get_symbol(const dl_handle& handle, const std::string& symbol) {
- // Call once to clear errors not caused by us
- dlerror();
- // Get symbol from shared object, may return NULL if that is what symbol refers to
- auto result = dlsym(handle.dl, symbol.c_str());
- // ... so we can only ask for dlerror here
- if (auto error = dlerror()) throw dl_error{util::pprintf("[POSIX] dl_get_symbol failed with: {}", error)};
- return reinterpret_cast<T>(result);
-}
-
-inline
-void dl_close(dl_handle& handle) {
- dlclose(handle.dl);
- handle.dl = nullptr;
-}
-
-}
diff --git a/arbor/util/dylib.cpp b/arbor/util/dylib.cpp
new file mode 100644
index 00000000..3f4826df
--- /dev/null
+++ b/arbor/util/dylib.cpp
@@ -0,0 +1,50 @@
+#include <fstream>
+#include <string>
+
+#include <dlfcn.h>
+
+#include <arbor/arbexcept.hpp>
+
+#include "util/dylib.hpp"
+#include "util/strprintf.hpp"
+
+namespace arb {
+namespace util {
+
+void* dl_open(const std::string& fn) {
+ try {
+ std::ifstream fd{fn.c_str()};
+ if(!fd.good()) throw file_not_found_error{fn};
+ } catch(...) {
+ throw file_not_found_error{fn};
+ }
+ // Call once to clear errors not caused by us
+ dlerror();
+ auto result = dlopen(fn.c_str(), RTLD_LAZY);
+ // dlopen fails by returning NULL
+ if (nullptr == result) {
+ auto error = dlerror();
+ throw dl_error{util::pprintf("[POSIX] dl_open failed with: {}", error)};
+ }
+ return result;
+}
+
+namespace impl{
+void* dl_get_symbol(const std::string& fn, const std::string& symbol) {
+ // Call once to clear errors not caused by us
+ dlerror();
+
+ auto handle = dl_open(fn);
+
+ // Get symbol from shared object, may return NULL if that is what symbol refers to
+ auto result = dlsym(handle, symbol.c_str());
+ // dlsym mayb return NULL even if succeeding
+ if (auto error = dlerror()) {
+ throw dl_error{util::pprintf("[POSIX] dl_get_symbol failed with: {}", error)};
+ }
+ return result;
+}
+} // namespace impl
+
+} // namespace util
+} // namespace arb
diff --git a/arbor/util/dylib.hpp b/arbor/util/dylib.hpp
new file mode 100644
index 00000000..ba97976a
--- /dev/null
+++ b/arbor/util/dylib.hpp
@@ -0,0 +1,26 @@
+#pragma once
+
+#include <string>
+
+#include <arbor/arbexcept.hpp>
+
+namespace arb {
+namespace util {
+
+namespace impl{
+void* dl_get_symbol(const std::string& filename, const std::string& symbol);
+} // namespace impl
+
+struct dl_error: arbor_exception {
+ dl_error(const std::string& msg): arbor_exception{msg} {}
+};
+
+// Find and return a symbol from a dynamic library with filename.
+// Throws dl_error on error.
+template<typename T>
+T dl_get_symbol(const std::string& filename, const std::string& symbol) {
+ return reinterpret_cast<T>(impl::dl_get_symbol(filename, symbol));
+}
+
+} // namespace util
+} // namespace arbor
diff --git a/scripts/test-catalogue.py b/scripts/test-catalogue.py
new file mode 100755
index 00000000..7c4716d0
--- /dev/null
+++ b/scripts/test-catalogue.py
@@ -0,0 +1,21 @@
+#!/usr/bin/env python3
+
+import argparse
+import os
+import sys
+
+import arbor
+
+P = argparse.ArgumentParser(description='Verify that a mechanism catalogue can be loaded through Python interface.')
+P.add_argument('catname', metavar='FILE', help='path of the catalogue to test.')
+
+args = P.parse_args()
+catname = args.catname
+
+print(catname)
+
+if not os.path.isfile(catname):
+ print('ERROR: unable to open catalogue file')
+ sys.exit(1)
+
+print([n for n in arbor.load_catalogue(catname).keys()])
--
GitLab