From 91cb9e0642ad3e64360d7259f8c53c5e21930ef8 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 10 Jul 2024 18:10:37 +0200 Subject: [PATCH 01/25] Homogenize finding our components. We currently rely on two files being present in pre-defined directories: * nrnunits.lib * libpywrapper.so (or similar) These are searched for in: * CMake's binary directory for NMODL * CMake's install directory for NMODL * The environment variable `NMODLHOME` Leading to some acrobatics to always set NMODLHOME correctly. This PR attempts to also find the above files relative to the NMODL executable, in the hope that some use cases of NMODLHOME can be removed. --- src/config/config.cpp.in | 68 ++++++++++++++++++++++++++++++++++--- src/config/config.h | 67 ++++++++++++++++++++---------------- src/main.cpp | 4 ++- src/parser/main_units.cpp | 2 +- src/pybind/pyembed.cpp | 12 ++----- src/visitors/main.cpp | 2 +- test/unit/units/parser.cpp | 2 +- test/unit/visitor/units.cpp | 2 +- 8 files changed, 111 insertions(+), 48 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 09a4da97c7..3ea8792b20 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -7,14 +7,19 @@ #include "config/config.h" +#include +#include +#include +#include + +namespace fs = std::filesystem; + /// Git version of the project const std::string nmodl::Version::GIT_REVISION = "@NMODL_GIT_REVISION@"; /// NMODL version const std::string nmodl::Version::NMODL_VERSION = "@PROJECT_VERSION@"; -const std::string nmodl::CMakeInfo::SHARED_LIBRARY_SUFFIX = "@CMAKE_SHARED_LIBRARY_SUFFIX@"; - /** * \brief Path of nrnutils.lib file * @@ -23,5 +28,60 @@ const std::string nmodl::CMakeInfo::SHARED_LIBRARY_SUFFIX = "@CMAKE_SHARED_LIBRA * from CMAKE_INSTALL_PREFIX. Note that this use of NMODL_PROJECT_BINARY_DIR * will cause ccache misses when the build prefix is changed. */ -std::vector nmodl::NrnUnitsLib::NRNUNITSLIB_PATH = - {"@CMAKE_INSTALL_PREFIX@/share/nmodl/nrnunits.lib", "@NMODL_PROJECT_BINARY_DIR@/share/nmodl/nrnunits.lib"}; +const std::vector nmodl::PathHelper::BASE_SEARCH_PATHS = + {"@CMAKE_INSTALL_PREFIX@", "@NMODL_PROJECT_BINARY_DIR@"}; + +const std::string nmodl::PathHelper::SHARED_LIBRARY_SUFFIX = "@CMAKE_SHARED_LIBRARY_SUFFIX@"; + +namespace { + +std::string maybe_from_env(const std::string& varname) { + const auto value = std::getenv(varname.c_str()); + if (value != nullptr) { + return value; + } + return ""; +} + +} + +std::string nmodl::PathHelper::nmodl_home = maybe_from_env("NMODL_HOME"); + +std::string nmodl::PathHelper::get_path(const std::string& what, bool add_library_suffix) { + std::vector search_paths = BASE_SEARCH_PATHS; + if (!get_home().empty()) { + search_paths.emplace(search_paths.begin(), get_home()); + } + + // check paths in order and return if found + for (const auto& path: search_paths) { + auto full_path = fs::path(path) / fs::path(what); + if (add_library_suffix) { + full_path += SHARED_LIBRARY_SUFFIX; + } + std::ifstream f(full_path); + if (f.good()) { + return full_path; + } + } + std::ostringstream err_msg; + err_msg << "Could not find '" << what << "' in any of:\n"; + for (const auto& path: search_paths) { + err_msg << "\t" << path << "\n"; + } + err_msg << "Please try setting the NMODLHOME environment variable\n"; + throw std::runtime_error(err_msg.str()); +} + +void nmodl::PathHelper::setup(const std::string& executable) { + // We give precedence to NMODLHOME - don't override if the home is already defined + if (nmodl_home.empty()) { + auto executable_dir = fs::canonical(fs::path(executable)).parent_path(); + if (executable_dir.filename() == "bin") { + nmodl_home = executable_dir.parent_path(); + } else { + // On Windows, we may find ourselves in the top-level directory without a bin/ + nmodl_home = executable_dir; + } + } +} diff --git a/src/config/config.h b/src/config/config.h index df5d7d379a..30de49da52 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -15,9 +15,6 @@ * \brief Version information and units file path */ -#include -#include -#include #include #include @@ -44,38 +41,48 @@ struct Version { /** * \brief Information of units database i.e. `nrnunits.lib` */ -struct NrnUnitsLib { - /// paths where nrnunits.lib can be found - static std::vector NRNUNITSLIB_PATH; +class PathHelper { + /// pre-defined paths to search for files + const static std::vector BASE_SEARCH_PATHS; + + /// suffix to use when looking for libraries + const static std::string SHARED_LIBRARY_SUFFIX; + + /// base directory of the NMODL installation + static std::string nmodl_home; /** - * Return path of units database file + * Search for a given relative file path + */ + static std::string get_path(const std::string& what, bool add_library_suffix=false); + + public: + /** + * Set the NMODL base installation directory from the executable if not defined in the + * environment + */ + static void setup(const std::string& executable); + + /** + * Return the NMODL base installation directory */ - static std::string get_path() { - // first look for NMODLHOME env variable - if (const char* nmodl_home = std::getenv("NMODLHOME")) { - auto path = std::string(nmodl_home) + "/share/nmodl/nrnunits.lib"; - NRNUNITSLIB_PATH.emplace(NRNUNITSLIB_PATH.begin(), path); - } - - // check paths in order and return if found - for (const auto& path: NRNUNITSLIB_PATH) { - std::ifstream f(path.c_str()); - if (f.good()) { - return path; - } - } - std::ostringstream err_msg; - err_msg << "Could not find nrnunits.lib in any of:\n"; - for (const auto& path: NRNUNITSLIB_PATH) { - err_msg << path << "\n"; - } - throw std::runtime_error(err_msg.str()); + static std::string get_home() { + return nmodl_home; } -}; -struct CMakeInfo { - static const std::string SHARED_LIBRARY_SUFFIX; + /** + * Return path of units database file + */ + static std::string get_units_path() { + return get_path("share/nmodl/nrnunits.lib"); + }; + + /** + * Return path of the python wrapper library + */ + static std::string get_wrapper_path() { + return get_path("lib/libpywrapper", true); + }; }; } // namespace nmodl diff --git a/src/main.cpp b/src/main.cpp index be322c0e99..8a5a06106e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -62,6 +62,8 @@ using nmodl::parser::NmodlDriver; // NOLINTNEXTLINE(readability-function-cognitive-complexity) int main(int argc, const char* argv[]) { + PathHelper::setup(argv[0]); + CLI::App app{fmt::format("NMODL : Source-to-Source Code Generation Framework [{}]", Version::to_string())}; @@ -142,7 +144,7 @@ int main(int argc, const char* argv[]) { std::string scratch_dir("tmp"); /// directory where units lib file is located - std::string units_dir(NrnUnitsLib::get_path()); + std::string units_dir(PathHelper::get_units_path()); /// true if ast should be converted to json bool json_ast(false); diff --git a/src/parser/main_units.cpp b/src/parser/main_units.cpp index 6c6966aae0..95ce6946ff 100644 --- a/src/parser/main_units.cpp +++ b/src/parser/main_units.cpp @@ -28,7 +28,7 @@ int main(int argc, const char* argv[]) { fmt::format("Unit-Parser : Standalone Parser for Units({})", Version::to_string())}; std::vector units_files; - units_files.push_back(NrnUnitsLib::get_path()); + units_files.push_back(PathHelper::get_units_path()); app.add_option("units_files", units_files, "One or more Units files to process"); CLI11_PARSE(app, argc, argv); diff --git a/src/pybind/pyembed.cpp b/src/pybind/pyembed.cpp index 7e4bfb8dcc..83d557e353 100644 --- a/src/pybind/pyembed.cpp +++ b/src/pybind/pyembed.cpp @@ -81,21 +81,15 @@ void EmbeddedPythonLoader::load_libraries() { assert_compatible_python_versions(); - if (std::getenv("NMODLHOME") == nullptr) { + if (PathHelper::get_home().empty()) { logger->critical("NMODLHOME environment variable must be set to load embedded python"); throw std::runtime_error("NMODLHOME not set"); } - auto pybind_wraplib_env = fs::path(std::getenv("NMODLHOME")) / "lib" / "libpywrapper"; - pybind_wraplib_env.concat(CMakeInfo::SHARED_LIBRARY_SUFFIX); - if (!fs::exists(pybind_wraplib_env)) { - logger->critical("NMODLHOME doesn't contain libpywrapper{} library", - CMakeInfo::SHARED_LIBRARY_SUFFIX); - throw std::runtime_error("NMODLHOME doesn't have lib/libpywrapper library"); - } + auto pybind_wraplib_env = PathHelper::get_wrapper_path(); pybind_wrapper_handle = dlopen(pybind_wraplib_env.c_str(), dlopen_opts); if (!pybind_wrapper_handle) { const auto errstr = dlerror(); - logger->critical("Tried but failed to load {}", pybind_wraplib_env.string()); + logger->critical("Tried but failed to load {}", pybind_wraplib_env); logger->critical(errstr); throw std::runtime_error("Failed to dlopen"); } diff --git a/src/visitors/main.cpp b/src/visitors/main.cpp index 9a6b969663..0653f866e4 100644 --- a/src/visitors/main.cpp +++ b/src/visitors/main.cpp @@ -95,7 +95,7 @@ int main(int argc, const char* argv[]) { "SympyConductanceVisitor"}, {std::make_shared(), "sympy-solve", "SympySolverVisitor"}, {std::make_shared(), "neuron-solve", "NeuronSolveVisitor"}, - {std::make_shared(NrnUnitsLib::get_path()), "units", "UnitsVisitor"}, + {std::make_shared(PathHelper::get_units_path()), "units", "UnitsVisitor"}, }; const std::vector const_visitors = { diff --git a/test/unit/units/parser.cpp b/test/unit/units/parser.cpp index 3ffd3497e1..9d17d94e44 100644 --- a/test/unit/units/parser.cpp +++ b/test/unit/units/parser.cpp @@ -33,7 +33,7 @@ bool is_valid_construct(const std::string& construct) { std::string parse_string(const std::string& unit_definition) { nmodl::parser::UnitDriver correctness_driver; - correctness_driver.parse_file(nmodl::NrnUnitsLib::get_path()); + correctness_driver.parse_file(nmodl::PathHelper::get_units_path()); correctness_driver.parse_string(unit_definition); std::stringstream ss; correctness_driver.table->print_units_sorted(ss); diff --git a/test/unit/visitor/units.cpp b/test/unit/visitor/units.cpp index 06f3865cd8..4bfcdc5544 100644 --- a/test/unit/visitor/units.cpp +++ b/test/unit/visitor/units.cpp @@ -36,7 +36,7 @@ std::tuple, std::shared_ptr> run const auto& ast = driver.get_ast(); // Parse nrnunits.lib file and the UNITS block of the mod file - const std::string units_lib_path(NrnUnitsLib::get_path()); + const std::string units_lib_path(PathHelper::get_units_path()); UnitsVisitor units_visitor = UnitsVisitor(units_lib_path); units_visitor.visit_program(*ast); From c6dbd202cffe38a9f694b922de160263dcd5bf08 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 10 Jul 2024 18:17:49 +0200 Subject: [PATCH 02/25] =?UTF-8?q?=E2=9D=84=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/config/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.h b/src/config/config.h index 30de49da52..53c7257af6 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -54,7 +54,7 @@ class PathHelper { /** * Search for a given relative file path */ - static std::string get_path(const std::string& what, bool add_library_suffix=false); + static std::string get_path(const std::string& what, bool add_library_suffix = false); public: /** From 3fafed83348546a631748bfe24dbc5a82420f234 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 10 Jul 2024 18:23:42 +0200 Subject: [PATCH 03/25] Shrink API. --- src/config/config.cpp.in | 4 ++-- src/config/config.h | 7 ------- src/pybind/pyembed.cpp | 4 ---- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 3ea8792b20..b765e8823a 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -49,8 +49,8 @@ std::string nmodl::PathHelper::nmodl_home = maybe_from_env("NMODL_HOME"); std::string nmodl::PathHelper::get_path(const std::string& what, bool add_library_suffix) { std::vector search_paths = BASE_SEARCH_PATHS; - if (!get_home().empty()) { - search_paths.emplace(search_paths.begin(), get_home()); + if (!nmodl_home.empty()) { + search_paths.emplace(search_paths.begin(), nmodl_home); } // check paths in order and return if found diff --git a/src/config/config.h b/src/config/config.h index 53c7257af6..14d896ca26 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -63,13 +63,6 @@ class PathHelper { */ static void setup(const std::string& executable); - /** - * Return the NMODL base installation directory - */ - static std::string get_home() { - return nmodl_home; - } - /** * Return path of units database file */ diff --git a/src/pybind/pyembed.cpp b/src/pybind/pyembed.cpp index 83d557e353..1c38813d88 100644 --- a/src/pybind/pyembed.cpp +++ b/src/pybind/pyembed.cpp @@ -81,10 +81,6 @@ void EmbeddedPythonLoader::load_libraries() { assert_compatible_python_versions(); - if (PathHelper::get_home().empty()) { - logger->critical("NMODLHOME environment variable must be set to load embedded python"); - throw std::runtime_error("NMODLHOME not set"); - } auto pybind_wraplib_env = PathHelper::get_wrapper_path(); pybind_wrapper_handle = dlopen(pybind_wraplib_env.c_str(), dlopen_opts); if (!pybind_wrapper_handle) { From 6441841c843f867c0469ad0cb9aa9174e75a9c1f Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Thu, 11 Jul 2024 11:28:34 +0200 Subject: [PATCH 04/25] Attempt to be more ARGV independent. --- src/config/config.cpp.in | 56 ++++++++++++++++++++++++++++------------ src/config/config.h | 8 +----- src/main.cpp | 2 -- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index b765e8823a..7d4d062905 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -12,6 +12,14 @@ #include #include +#if defined(_WIN32) +#include +#elif defined(__APPLE__) +#include +#include +#else +#endif + namespace fs = std::filesystem; /// Git version of the project @@ -40,17 +48,44 @@ std::string maybe_from_env(const std::string& varname) { if (value != nullptr) { return value; } - return ""; + +#if defined(_WIN32) + std::vector buffer; + DWORD copied = 0; + do { + buffer.resize(buffer.size() + MAX_PATH); + copied = GetModuleFileName(0, &buffer.at(0), buffer.size()); + } while (copied >= buffer.size()); + buffer.resize(copied); + fs::path executable(std::wstring(buffer.begin(), buffer.end())); +#elif defined(__APPLE__) + char buffer[PATH_MAX + 1]; + uint32_t bufsize = PATH_MAX + 1; + if( _NSGetExecutablePath(buf, &bufsize) != 0) { + return ""; + } + auto executable = fs::read_symlink(buffer); +#else + auto executable = fs::read_symlink("/proc/self/exe"); +#endif + + auto executable_dir = fs::weakly_canonical(executable).parent_path(); + if (executable_dir.filename() == "bin") { + return executable_dir.parent_path(); + } else { + // On Windows, we may find ourselves in the top-level directory without a bin/ + return executable_dir; + } } } -std::string nmodl::PathHelper::nmodl_home = maybe_from_env("NMODL_HOME"); +const std::string nmodl::PathHelper::NMODL_HOME = maybe_from_env("NMODL_HOME"); std::string nmodl::PathHelper::get_path(const std::string& what, bool add_library_suffix) { std::vector search_paths = BASE_SEARCH_PATHS; - if (!nmodl_home.empty()) { - search_paths.emplace(search_paths.begin(), nmodl_home); + if (!NMODL_HOME.empty()) { + search_paths.emplace(search_paths.begin(), NMODL_HOME); } // check paths in order and return if found @@ -72,16 +107,3 @@ std::string nmodl::PathHelper::get_path(const std::string& what, bool add_librar err_msg << "Please try setting the NMODLHOME environment variable\n"; throw std::runtime_error(err_msg.str()); } - -void nmodl::PathHelper::setup(const std::string& executable) { - // We give precedence to NMODLHOME - don't override if the home is already defined - if (nmodl_home.empty()) { - auto executable_dir = fs::canonical(fs::path(executable)).parent_path(); - if (executable_dir.filename() == "bin") { - nmodl_home = executable_dir.parent_path(); - } else { - // On Windows, we may find ourselves in the top-level directory without a bin/ - nmodl_home = executable_dir; - } - } -} diff --git a/src/config/config.h b/src/config/config.h index 14d896ca26..b26f698671 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -49,7 +49,7 @@ class PathHelper { const static std::string SHARED_LIBRARY_SUFFIX; /// base directory of the NMODL installation - static std::string nmodl_home; + const static std::string NMODL_HOME; /** * Search for a given relative file path @@ -57,12 +57,6 @@ class PathHelper { static std::string get_path(const std::string& what, bool add_library_suffix = false); public: - /** - * Set the NMODL base installation directory from the executable if not defined in the - * environment - */ - static void setup(const std::string& executable); - /** * Return path of units database file */ diff --git a/src/main.cpp b/src/main.cpp index 8a5a06106e..891476389e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -62,8 +62,6 @@ using nmodl::parser::NmodlDriver; // NOLINTNEXTLINE(readability-function-cognitive-complexity) int main(int argc, const char* argv[]) { - PathHelper::setup(argv[0]); - CLI::App app{fmt::format("NMODL : Source-to-Source Code Generation Framework [{}]", Version::to_string())}; From 9499115b3235e62f3a5a815f150b737b1f4a59bc Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Thu, 11 Jul 2024 11:48:38 +0200 Subject: [PATCH 05/25] Fixeth for the AppleOS --- src/config/config.cpp.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 7d4d062905..6dd69f0537 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -61,7 +61,7 @@ std::string maybe_from_env(const std::string& varname) { #elif defined(__APPLE__) char buffer[PATH_MAX + 1]; uint32_t bufsize = PATH_MAX + 1; - if( _NSGetExecutablePath(buf, &bufsize) != 0) { + if( _NSGetExecutablePath(buffer, &bufsize) != 0) { return ""; } auto executable = fs::read_symlink(buffer); From f4863e4a04be462cf42b5bb0db3fb8f8e811bc63 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Thu, 11 Jul 2024 13:06:21 +0200 Subject: [PATCH 06/25] More AppleOS fixes. --- src/config/config.cpp.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 6dd69f0537..fe1a96d3fe 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -64,7 +64,7 @@ std::string maybe_from_env(const std::string& varname) { if( _NSGetExecutablePath(buffer, &bufsize) != 0) { return ""; } - auto executable = fs::read_symlink(buffer); + auto executable = fs::path(buffer); #else auto executable = fs::read_symlink("/proc/self/exe"); #endif From c765a3a1e575827556dbae0b7c7f7c0c97b49864 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Fri, 12 Jul 2024 15:54:54 +0200 Subject: [PATCH 07/25] Fix Windows after local reproducer. --- src/config/config.cpp.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index fe1a96d3fe..77180f30d6 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -13,7 +13,7 @@ #include #if defined(_WIN32) -#include +#include #elif defined(__APPLE__) #include #include @@ -50,7 +50,7 @@ std::string maybe_from_env(const std::string& varname) { } #if defined(_WIN32) - std::vector buffer; + std::vector buffer; DWORD copied = 0; do { buffer.resize(buffer.size() + MAX_PATH); From 81b5c27faa394c746a8c0bb789824dfd12920027 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Mon, 22 Jul 2024 16:25:50 +0200 Subject: [PATCH 08/25] Try to build Windwos Wheels --- azure-pipelines.yml | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 7b456ef3a2..312f977570 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -284,6 +284,53 @@ stages: displayName: 'Publish wheel as build artifact' - template: ci/upload-wheels.yml + - job: 'windows_wheels_x86_64' + timeoutInMinutes: 45 + pool: + vmImage: 'windows-2022' + steps: + - checkout: self + submodules: True + condition: succeeded() + - script: | + choco install winflexbison3 + condition: succeeded() + displayName: 'Install Dependencies' + + - script: | + if (Compare-Object $env:RELEASEWHEELBUILD "True") { + Write-Output "##vso[task.setvariable variable=TAG;]-nightly" + } else { + Write-Output "##vso[task.setvariable variable=TAG;]" + } + Write-Output "Wheel tag: $env:TAG" + displayName: "Set wheel tag" + + - script: | + Write-Output "##vso[task.setvariable variable=CIBW_BUILD;]cp38* cp312*" + Write-Output "Build identifiers: $env:CIBW_BUILD" + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) + displayName: "Set build identifiers" + + - task: UsePythonVersion@0 + + - script: | + python3 -m pip install --upgrade pip + python3 -m pip install cibuildwheel==2.16.5 tomli tomli-w + # change the name accordingly + python3 packaging/change_name.py pyproject.toml "NMODL$env:TAG" + $env:SETUPTOOLS_SCM_PRETEND_VERSION=((git describe --tags) -split "-"|Select-Object -First 2) -join "." + python3 -m cibuildwheel --output-dir wheelhouse + condition: succeeded() + displayName: 'Build Windows Wheel (x86_64)' + + - task: PublishBuildArtifacts@1 + inputs: + pathToPublish: '$(Build.SourcesDirectory)/wheelhouse' + condition: succeeded() + displayName: 'Publish wheel as build artifact' + - template: ci/upload-wheels.yml + - job: 'test_manylinux_wheels' dependsOn: 'manylinux_wheels' timeoutInMinutes: 45 From a0172d234b56503b5221b46fb17b1506991bbd4d Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Mon, 22 Jul 2024 16:38:33 +0200 Subject: [PATCH 09/25] Powdershell. --- azure-pipelines.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 312f977570..263a050250 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -292,12 +292,12 @@ stages: - checkout: self submodules: True condition: succeeded() - - script: | + - pwsh: | choco install winflexbison3 condition: succeeded() displayName: 'Install Dependencies' - - script: | + - pwsh: | if (Compare-Object $env:RELEASEWHEELBUILD "True") { Write-Output "##vso[task.setvariable variable=TAG;]-nightly" } else { @@ -306,7 +306,7 @@ stages: Write-Output "Wheel tag: $env:TAG" displayName: "Set wheel tag" - - script: | + - pwsh: | Write-Output "##vso[task.setvariable variable=CIBW_BUILD;]cp38* cp312*" Write-Output "Build identifiers: $env:CIBW_BUILD" condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) @@ -314,7 +314,7 @@ stages: - task: UsePythonVersion@0 - - script: | + - pwsh: | python3 -m pip install --upgrade pip python3 -m pip install cibuildwheel==2.16.5 tomli tomli-w # change the name accordingly From b17eb7edb7cd9c3e572e68bb3b10ef0afa0aa53b Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Mon, 22 Jul 2024 17:41:24 +0200 Subject: [PATCH 10/25] Magic variations? --- azure-pipelines.yml | 16 +++++++++------- pyproject.toml | 5 ++++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 263a050250..5d4d74bfd7 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -298,29 +298,31 @@ stages: displayName: 'Install Dependencies' - pwsh: | - if (Compare-Object $env:RELEASEWHEELBUILD "True") { + if (Compare-Object $RELEASEWHEELBUILD "True") { + Write-Output "Using nightly identifier" Write-Output "##vso[task.setvariable variable=TAG;]-nightly" } else { Write-Output "##vso[task.setvariable variable=TAG;]" } - Write-Output "Wheel tag: $env:TAG" + Write-Output "Wheel tag: $TAG" displayName: "Set wheel tag" - pwsh: | Write-Output "##vso[task.setvariable variable=CIBW_BUILD;]cp38* cp312*" - Write-Output "Build identifiers: $env:CIBW_BUILD" + Write-Output "Build identifiers: $CIBW_BUILD" condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) displayName: "Set build identifiers" - task: UsePythonVersion@0 - pwsh: | - python3 -m pip install --upgrade pip - python3 -m pip install cibuildwheel==2.16.5 tomli tomli-w + Write-Output "Aye have $env:TAG oar $TAG" + python -m pip install --upgrade pip + python -m pip install cibuildwheel==2.16.5 tomli tomli-w # change the name accordingly - python3 packaging/change_name.py pyproject.toml "NMODL$env:TAG" + python packaging/change_name.py pyproject.toml "NMODL$env:TAG" $env:SETUPTOOLS_SCM_PRETEND_VERSION=((git describe --tags) -split "-"|Select-Object -First 2) -join "." - python3 -m cibuildwheel --output-dir wheelhouse + python -m cibuildwheel --output-dir wheelhouse condition: succeeded() displayName: 'Build Windows Wheel (x86_64)' diff --git a/pyproject.toml b/pyproject.toml index 28082e0146..82ae74e86c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,4 +76,7 @@ environment = { PATH = "/nmodlwheel/flex/bin:/nmodlwheel/bison/bin:$PATH" } test-command = "true" [tool.cibuildwheel.windows] -environment = { SKBUILD_CMAKE_ARGS = "-DNMODL_BUILD_WHEEL=ON;-DFLEX_INCLUDE_PATH=C:/ProgramData/chocolatey/lib/winflexbison3/tools" } \ No newline at end of file +environment = { SKBUILD_CMAKE_ARGS = "-DNMODL_BUILD_WHEEL=ON;-DFLEX_INCLUDE_PATH=C:/ProgramData/chocolatey/lib/winflexbison3/tools" } +test-command = [ + "bash {package}/packaging/test_wheel.bash python {wheel} false", +] \ No newline at end of file From 9bccc20b6f50fe051a12590dd02d7325b6b53b6f Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Mon, 22 Jul 2024 17:45:34 +0200 Subject: [PATCH 11/25] Fix. --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 5d4d74bfd7..f7e27e517a 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -298,7 +298,7 @@ stages: displayName: 'Install Dependencies' - pwsh: | - if (Compare-Object $RELEASEWHEELBUILD "True") { + if (Compare-Object $env:RELEASEWHEELBUILD "True") { Write-Output "Using nightly identifier" Write-Output "##vso[task.setvariable variable=TAG;]-nightly" } else { From fab9a4ae55aafe6010550808730cde2207ee508c Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Tue, 23 Jul 2024 10:06:33 +0200 Subject: [PATCH 12/25] Fix merge. --- src/pybind/pyembed.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pybind/pyembed.cpp b/src/pybind/pyembed.cpp index 001e72b93d..1c38813d88 100644 --- a/src/pybind/pyembed.cpp +++ b/src/pybind/pyembed.cpp @@ -82,8 +82,7 @@ void EmbeddedPythonLoader::load_libraries() { assert_compatible_python_versions(); auto pybind_wraplib_env = PathHelper::get_wrapper_path(); - std::string env_str = pybind_wraplib_env.string(); - pybind_wrapper_handle = dlopen(env_str.c_str(), dlopen_opts); + pybind_wrapper_handle = dlopen(pybind_wraplib_env.c_str(), dlopen_opts); if (!pybind_wrapper_handle) { const auto errstr = dlerror(); logger->critical("Tried but failed to load {}", pybind_wraplib_env); From 0200e0f1105cc118d99f0890ce5d2e17f33f429b Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Tue, 23 Jul 2024 11:58:22 +0200 Subject: [PATCH 13/25] Explicit conversion to string. --- src/config/config.cpp.in | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 77180f30d6..d0eee8b093 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -39,6 +39,7 @@ const std::string nmodl::Version::NMODL_VERSION = "@PROJECT_VERSION@"; const std::vector nmodl::PathHelper::BASE_SEARCH_PATHS = {"@CMAKE_INSTALL_PREFIX@", "@NMODL_PROJECT_BINARY_DIR@"}; +const std::string nmodl::PathHelper::SHARED_LIBRARY_PREFIX = "@CMAKE_SHARED_LIBRARY_PREFIX@"; const std::string nmodl::PathHelper::SHARED_LIBRARY_SUFFIX = "@CMAKE_SHARED_LIBRARY_SUFFIX@"; namespace { @@ -71,10 +72,10 @@ std::string maybe_from_env(const std::string& varname) { auto executable_dir = fs::weakly_canonical(executable).parent_path(); if (executable_dir.filename() == "bin") { - return executable_dir.parent_path(); + return executable_dir.parent_path().string(); } else { // On Windows, we may find ourselves in the top-level directory without a bin/ - return executable_dir; + return executable_dir.string(); } } @@ -82,7 +83,7 @@ std::string maybe_from_env(const std::string& varname) { const std::string nmodl::PathHelper::NMODL_HOME = maybe_from_env("NMODL_HOME"); -std::string nmodl::PathHelper::get_path(const std::string& what, bool add_library_suffix) { +std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library) { std::vector search_paths = BASE_SEARCH_PATHS; if (!NMODL_HOME.empty()) { search_paths.emplace(search_paths.begin(), NMODL_HOME); @@ -90,9 +91,11 @@ std::string nmodl::PathHelper::get_path(const std::string& what, bool add_librar // check paths in order and return if found for (const auto& path: search_paths) { - auto full_path = fs::path(path) / fs::path(what); - if (add_library_suffix) { - full_path += SHARED_LIBRARY_SUFFIX; + auto full_path = fs::path(path); + if (is_library) { + full_path /= SHARED_LIBRARY_PREFIX + what + SHARED_LIBRARY_SUFFIX; + } else { + full_path /= what; } std::ifstream f(full_path); if (f.good()) { From 62a5a497577e3af480c4ae5f81a4739bf0c7cf0c Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Tue, 23 Jul 2024 14:07:18 +0200 Subject: [PATCH 14/25] Forgotten header modifications. --- src/config/config.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/config/config.h b/src/config/config.h index b26f698671..40310adc83 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -45,6 +45,9 @@ class PathHelper { /// pre-defined paths to search for files const static std::vector BASE_SEARCH_PATHS; + /// prefix to use when looking for libraries + const static std::string SHARED_LIBRARY_PREFIX; + /// suffix to use when looking for libraries const static std::string SHARED_LIBRARY_SUFFIX; @@ -54,7 +57,7 @@ class PathHelper { /** * Search for a given relative file path */ - static std::string get_path(const std::string& what, bool add_library_suffix = false); + static std::string get_path(const std::string& what, bool is_library = false); public: /** From d6d14f4c013d4bc55bff3fb448e7f1ee85be3681 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 14:19:47 +0200 Subject: [PATCH 15/25] Produce wheels. Works locally with py312. --- CMakeLists.txt | 17 ++++++++------- azure-pipelines.yml | 10 ++++----- packaging/test_wheel.ps1 | 43 +++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- python/nmodl/__init__.py | 13 +++++------ python/nmodl/_binwrapper.py | 37 +++++++++++++++++-------------- src/config/config.cpp.in | 4 +++- src/config/config.h | 2 +- src/pybind/CMakeLists.txt | 8 ++++++- src/pybind/pyembed.cpp | 5 +++++ src/pybind/wrapper.hpp | 4 ++-- test/unit/CMakeLists.txt | 4 ---- 12 files changed, 101 insertions(+), 48 deletions(-) create mode 100644 packaging/test_wheel.ps1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 84e01c07a8..b6088dec5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -84,6 +84,13 @@ project( NMODL VERSION ${NMODL_GIT_LAST_TAG} LANGUAGES CXX) + +# ============================================================================= +# Adjust install prefix for wheel +# ============================================================================= +if(NOT LINK_AGAINST_PYTHON AND NOT NMODL_AS_SUBPROJECT) +set(NMODL_INSTALL_DIR_SUFFIX "nmodl/.data/") +endif() # ============================================================================= # HPC Coding Conventions @@ -131,9 +138,10 @@ endif() cpp_cc_git_submodule(json BUILD PACKAGE nlohmann_json REQUIRED) cpp_cc_git_submodule(pybind11 BUILD PACKAGE pybind11 REQUIRED) if(WIN32) - cpp_cc_git_submodule(dlfcn-win32 BUILD) + cpp_cc_git_submodule(dlfcn-win32 BUILD EXCLUDE_FROM_ALL) add_library(dlfcn-win32::dl ALIAS dl) set(CMAKE_DL_LIBS dlfcn-win32::dl) + install(TARGETS dl DESTINATION ${NMODL_INSTALL_DIR_SUFFIX}bin) endif() # Tell spdlog not to use its bundled fmt, it should either use the fmt submodule or a truly external # installation for consistency. This line should be harmless if we use an external spdlog. @@ -166,13 +174,6 @@ add_custom_target( clean_ipynb "${CMAKE_SOURCE_DIR}/docs/notebooks/*.ipynb") -# ============================================================================= -# Adjust install prefix for wheel -# ============================================================================= -if(NOT LINK_AGAINST_PYTHON AND NOT NMODL_AS_SUBPROJECT) - set(NMODL_INSTALL_DIR_SUFFIX "nmodl/.data/") -endif() - # ============================================================================= # Find required python packages # ============================================================================= diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f7e27e517a..56cac062b9 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -299,24 +299,22 @@ stages: - pwsh: | if (Compare-Object $env:RELEASEWHEELBUILD "True") { - Write-Output "Using nightly identifier" - Write-Output "##vso[task.setvariable variable=TAG;]-nightly" - } else { Write-Output "##vso[task.setvariable variable=TAG;]" + } else { + Write-Output "##vso[task.setvariable variable=TAG;]-nightly" } - Write-Output "Wheel tag: $TAG" displayName: "Set wheel tag" - pwsh: | Write-Output "##vso[task.setvariable variable=CIBW_BUILD;]cp38* cp312*" - Write-Output "Build identifiers: $CIBW_BUILD" condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) displayName: "Set build identifiers" - task: UsePythonVersion@0 - pwsh: | - Write-Output "Aye have $env:TAG oar $TAG" + Write-Output "Using TAG '$env:TAG'" + Write-Output "Using CIBW_BUILD '$env:CIBW_BUILD'" python -m pip install --upgrade pip python -m pip install cibuildwheel==2.16.5 tomli tomli-w # change the name accordingly diff --git a/packaging/test_wheel.ps1 b/packaging/test_wheel.ps1 new file mode 100644 index 0000000000..e8bd861305 --- /dev/null +++ b/packaging/test_wheel.ps1 @@ -0,0 +1,43 @@ +param ( + [Parameter(Mandatory=$true)][string]$wheel, + [bool]$venv=$true +) + +$TEST_DIR = "$($Env:temp)/tmp$([convert]::tostring((get-random 65535),16).padleft(4,'0')).tmp" +New-Item -ItemType Directory -Path $TEST_DIR +New-Item -ItemType Directory -Path $TEST_DIR/input +New-Item -ItemType Directory -Path $TEST_DIR/output + +$NMODL_ROOT=(Split-Path -Parent $PSScriptRoot) + +Write-Output $NMODL_ROOT +Get-ChildItem -Path (Join-Path $NMODL_ROOT "python/nmodl/ext/example") -Filter "*.mod" | ForEach-Object { + Copy-Item $_ $TEST_DIR/input +} +Copy-Item "$NMODL_ROOT/test/integration/mod/cabpump.mod" $TEST_DIR/input +Copy-Item "$NMODL_ROOT/test/integration/mod/var_init.inc" $TEST_DIR/input +Copy-Item "$NMODL_ROOT/test/integration/mod/glia_sparse.mod" $TEST_DIR/input + +if ($venv) { + python -m venv wheel_test_venv + ./wheel_test_venv/Scripts/activate.ps1 + + pip uninstall -y nmodl nmodl-nightly + pip install "${wheel}[test]" + pip show nmodl-nightly +} + +Get-ChildItem -Path $TEST_DIR/input -Filter "*.mod" | ForEach-Object { + $path = $_ -replace "\\","/" + Write-Output "nmodl -o $TEST_DIR/output $path sympy --analytic" + nmodl -o $TEST_DIR/output $path sympy --analytic + if (! $?) { + Exit 1 + } + python -c "import nmodl; driver = nmodl.NmodlDriver(); driver.parse_file('$path')" + if (! $?) { + Exit 1 + } +} + +# rm -r $TEST_DIR \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 82ae74e86c..44cd58fa1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,5 +78,5 @@ test-command = "true" [tool.cibuildwheel.windows] environment = { SKBUILD_CMAKE_ARGS = "-DNMODL_BUILD_WHEEL=ON;-DFLEX_INCLUDE_PATH=C:/ProgramData/chocolatey/lib/winflexbison3/tools" } test-command = [ - "bash {package}/packaging/test_wheel.bash python {wheel} false", + "pwsh -File {package}/packaging/test_wheel.ps1 {wheel} -venv:false", ] \ No newline at end of file diff --git a/python/nmodl/__init__.py b/python/nmodl/__init__.py index b427d14af7..07a8379d83 100644 --- a/python/nmodl/__init__.py +++ b/python/nmodl/__init__.py @@ -22,14 +22,11 @@ "to the Python library" ) from exc -# add nmodl home to environment (i.e. necessary for nrnunits.lib) if not -# already set -# `files` will automatically raise a `ModuleNotFoundError` -os.environ["NMODLHOME"] = os.environ.get( - "NMODLHOME", - str(files("nmodl") / ".data"), -) - +if os.name == "nt": + # On Windows, DLLs get installed alongside with the binary. But _nmodl also links + # against them, so instruct Python where to look for the DLLs + bindir = files("nmodl") / ".data" / "bin" + os.add_dll_directory(bindir) import builtins diff --git a/python/nmodl/_binwrapper.py b/python/nmodl/_binwrapper.py index 036334ceb6..97b83433e5 100755 --- a/python/nmodl/_binwrapper.py +++ b/python/nmodl/_binwrapper.py @@ -4,6 +4,8 @@ Please create a softlink with the binary name to be called. """ import os +from pathlib import Path +import subprocess import stat import sys @@ -26,22 +28,25 @@ def main(): except PackageNotFoundError: pass - NMODL_PREFIX = files("nmodl") - NMODL_HOME = NMODL_PREFIX / ".data" - NMODL_BIN = NMODL_HOME / "bin" + prefix = files("nmodl") + exe = prefix / ".data" / "bin" / Path(sys.argv[0]).name - # add libpython*.so path to environment - os.environ["NMODL_PYLIB"] = find_libpython() - - # add nmodl home to environment (i.e. necessary for nrnunits.lib) - os.environ["NMODLHOME"] = str(NMODL_HOME) + if os.name == "nt": + exe = exe.with_suffix(".exe") + else: + st = os.stat(exe) + os.chmod(exe, st.st_mode | stat.S_IEXEC) + env = dict(os.environ) + # add libpython*.so path to environment + env["NMODL_PYLIB"] = find_libpython() # set PYTHONPATH for embedded python to properly find the nmodl module - os.environ["PYTHONPATH"] = ( - str(NMODL_PREFIX.parent) + ":" + os.environ.get("PYTHONPATH", "") - ) - - exe = NMODL_BIN / os.path.basename(sys.argv[0]) - st = os.stat(exe) - os.chmod(exe, st.st_mode | stat.S_IEXEC) - os.execv(exe, sys.argv) + env["PYTHONPATH"] = str(prefix.parent) + if pth := os.environ.get("PYTHONPATH"): + env["PYTHONPATH"] += os.pathsep + pth + + cmd = [exe] + sys.argv[1:] + try: + subprocess.check_call(cmd, env=env) + except subprocess.CalledProcessError: + sys.exit(1) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index d0eee8b093..bd933de51a 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -86,6 +86,8 @@ const std::string nmodl::PathHelper::NMODL_HOME = maybe_from_env("NMODL_HOME"); std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library) { std::vector search_paths = BASE_SEARCH_PATHS; if (!NMODL_HOME.empty()) { + search_paths.emplace(search_paths.begin(), (fs::path(NMODL_HOME) / "bin").string()); + search_paths.emplace(search_paths.begin(), (fs::path(NMODL_HOME) / "lib").string()); search_paths.emplace(search_paths.begin(), NMODL_HOME); } @@ -99,7 +101,7 @@ std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library } std::ifstream f(full_path); if (f.good()) { - return full_path; + return full_path.string(); } } std::ostringstream err_msg; diff --git a/src/config/config.h b/src/config/config.h index 40310adc83..d40d8a130b 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -71,7 +71,7 @@ class PathHelper { * Return path of the python wrapper library */ static std::string get_wrapper_path() { - return get_path("lib/libpywrapper", true); + return get_path("pywrapper", true); }; }; diff --git a/src/pybind/CMakeLists.txt b/src/pybind/CMakeLists.txt index b9d561b943..0bb7fadc80 100644 --- a/src/pybind/CMakeLists.txt +++ b/src/pybind/CMakeLists.txt @@ -114,7 +114,13 @@ file(COPY ${NMODL_PROJECT_SOURCE_DIR}/python/nmodl/ext DESTINATION ${CMAKE_BINAR # move the .so libs # ~~~ if(NOT LINK_AGAINST_PYTHON AND NOT NMODL_AS_SUBPROJECT) - install(TARGETS pywrapper DESTINATION ${NMODL_INSTALL_DIR_SUFFIX}lib) + if(MSVC) + # DLLs are required both by the executable and the library - prefer + # installation next to the former. + install(TARGETS pywrapper DESTINATION ${NMODL_INSTALL_DIR_SUFFIX}bin) + else() + install(TARGETS pywrapper DESTINATION ${NMODL_INSTALL_DIR_SUFFIX}lib) + endif() if(NMODL_ENABLE_PYTHON_BINDINGS) install(TARGETS _nmodl DESTINATION nmodl/) endif() diff --git a/src/pybind/pyembed.cpp b/src/pybind/pyembed.cpp index 1c38813d88..2a7fffdba9 100644 --- a/src/pybind/pyembed.cpp +++ b/src/pybind/pyembed.cpp @@ -64,11 +64,16 @@ void assert_compatible_python_versions() { } void EmbeddedPythonLoader::load_libraries() { +#ifdef _WIN32 + // Windows does not require a full search path + const char* pylib_env = "python3.dll"; +#else const auto pylib_env = std::getenv("NMODL_PYLIB"); if (!pylib_env) { logger->critical("NMODL_PYLIB environment variable must be set to load embedded python"); throw std::runtime_error("NMODL_PYLIB not set"); } +#endif const auto dlopen_opts = RTLD_NOW | RTLD_GLOBAL; dlerror(); // reset old error conditions pylib_handle = dlopen(pylib_env, dlopen_opts); diff --git a/src/pybind/wrapper.hpp b/src/pybind/wrapper.hpp index 725f9f8113..56b035effa 100644 --- a/src/pybind/wrapper.hpp +++ b/src/pybind/wrapper.hpp @@ -54,13 +54,13 @@ struct pybind_wrap_api { }; #ifdef _WIN32 -#define NMODL_EXPORT +#define NMODL_EXPORT __declspec(dllexport) #else #define NMODL_EXPORT __attribute__((visibility("default"))) #endif extern "C" { -pybind_wrap_api nmodl_init_pybind_wrapper_api() noexcept; +NMODL_EXPORT pybind_wrap_api nmodl_init_pybind_wrapper_api() noexcept; } diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 1845367288..cdb9036f54 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -119,10 +119,6 @@ target_link_libraries(testunitparser PRIVATE lexer test_util config) # Use catch_discover instead of add_test for granular test result reporting. # ============================================================================= set(test_env ${NMODL_SANITIZER_ENABLE_ENVIRONMENT}) -set(testvisitor_env "PYTHONPATH=${PROJECT_BINARY_DIR}/lib:$ENV{PYTHONPATH}") -if(NOT LINK_AGAINST_PYTHON) - list(APPEND testvisitor_env "NMODL_PYLIB=$ENV{NMODL_PYLIB}") -endif() # Without main from Catch2 target_link_libraries(testcodegen PRIVATE Catch2::Catch2) From 027830ab45e310302dc3cad6d0f41c8af06a0e75 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 14:25:32 +0200 Subject: [PATCH 16/25] Debug. --- azure-pipelines.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 56cac062b9..5e614d8abe 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -298,6 +298,7 @@ stages: displayName: 'Install Dependencies' - pwsh: | + Write-Output $env:RELEASEWHEELBUILD if (Compare-Object $env:RELEASEWHEELBUILD "True") { Write-Output "##vso[task.setvariable variable=TAG;]" } else { From 517bed4a3998ef5769a22f32d9f20ef40dc75fd3 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 14:52:16 +0200 Subject: [PATCH 17/25] Fixes? --- CMakeLists.txt | 2 +- azure-pipelines.yml | 7 +++---- python/nmodl/__init__.py | 8 ++++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b6088dec5b..547de8b699 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -89,7 +89,7 @@ project( # Adjust install prefix for wheel # ============================================================================= if(NOT LINK_AGAINST_PYTHON AND NOT NMODL_AS_SUBPROJECT) -set(NMODL_INSTALL_DIR_SUFFIX "nmodl/.data/") + set(NMODL_INSTALL_DIR_SUFFIX "nmodl/.data/") endif() # ============================================================================= diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 5e614d8abe..9be1bd7756 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -298,11 +298,10 @@ stages: displayName: 'Install Dependencies' - pwsh: | - Write-Output $env:RELEASEWHEELBUILD - if (Compare-Object $env:RELEASEWHEELBUILD "True") { - Write-Output "##vso[task.setvariable variable=TAG;]" - } else { + if (Compare-Object $env:RELEASEWHEELBUILD "False") { Write-Output "##vso[task.setvariable variable=TAG;]-nightly" + } else { + Write-Output "##vso[task.setvariable variable=TAG;]" } displayName: "Set wheel tag" diff --git a/python/nmodl/__init__.py b/python/nmodl/__init__.py index 07a8379d83..b5faefb0e5 100644 --- a/python/nmodl/__init__.py +++ b/python/nmodl/__init__.py @@ -27,6 +27,14 @@ # against them, so instruct Python where to look for the DLLs bindir = files("nmodl") / ".data" / "bin" os.add_dll_directory(bindir) +else: + # add nmodl home to environment (i.e. necessary for nrnunits.lib) if not + # already set + # `files` will automatically raise a `ModuleNotFoundError` + os.environ["NMODLHOME"] = os.environ.get( + "NMODLHOME", + str(files("nmodl") / ".data"), + ) import builtins From 3c1ab018cdd5a4238660353819ec5bc1b6300c6e Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 14:55:00 +0200 Subject: [PATCH 18/25] Debug. --- packaging/test_wheel.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/test_wheel.ps1 b/packaging/test_wheel.ps1 index e8bd861305..0397e1ce19 100644 --- a/packaging/test_wheel.ps1 +++ b/packaging/test_wheel.ps1 @@ -30,7 +30,7 @@ if ($venv) { Get-ChildItem -Path $TEST_DIR/input -Filter "*.mod" | ForEach-Object { $path = $_ -replace "\\","/" Write-Output "nmodl -o $TEST_DIR/output $path sympy --analytic" - nmodl -o $TEST_DIR/output $path sympy --analytic + nmodl --verbose=trace -o $TEST_DIR/output $path sympy --analytic if (! $?) { Exit 1 } From 299fcce4b4a2aeefe842904c4c720d45ac977578 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 15:26:30 +0200 Subject: [PATCH 19/25] Log failure reason on win testing. --- packaging/test_wheel.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packaging/test_wheel.ps1 b/packaging/test_wheel.ps1 index 0397e1ce19..967415b426 100644 --- a/packaging/test_wheel.ps1 +++ b/packaging/test_wheel.ps1 @@ -30,12 +30,14 @@ if ($venv) { Get-ChildItem -Path $TEST_DIR/input -Filter "*.mod" | ForEach-Object { $path = $_ -replace "\\","/" Write-Output "nmodl -o $TEST_DIR/output $path sympy --analytic" - nmodl --verbose=trace -o $TEST_DIR/output $path sympy --analytic + nmodl -o $TEST_DIR/output $path sympy --analytic if (! $?) { + Write-Output "Failed NMODL run" Exit 1 } python -c "import nmodl; driver = nmodl.NmodlDriver(); driver.parse_file('$path')" if (! $?) { + Write-Output "Failed NMODL Python module parsing" Exit 1 } } From 9a6d6102284d86959e5a951e8f139f8da3eb3b5d Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 15:27:28 +0200 Subject: [PATCH 20/25] Debug. --- python/nmodl/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/nmodl/__init__.py b/python/nmodl/__init__.py index b5faefb0e5..16816e1c1f 100644 --- a/python/nmodl/__init__.py +++ b/python/nmodl/__init__.py @@ -35,6 +35,7 @@ "NMODLHOME", str(files("nmodl") / ".data"), ) + print("Setting HOME to: ", os.environ["NMODLHOME"]) import builtins From 2f9fc4b42b91047d660ec6d4fbb9621802a132c4 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 15:31:25 +0200 Subject: [PATCH 21/25] Clearer exception when file not found. --- src/config/config.cpp.in | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index bd933de51a..c8c9161adb 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -91,21 +91,21 @@ std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library search_paths.emplace(search_paths.begin(), NMODL_HOME); } + std::string filename = what; + if (is_library) { + filename = SHARED_LIBRARY_PREFIX + what + SHARED_LIBRARY_SUFFIX;j + } + // check paths in order and return if found for (const auto& path: search_paths) { - auto full_path = fs::path(path); - if (is_library) { - full_path /= SHARED_LIBRARY_PREFIX + what + SHARED_LIBRARY_SUFFIX; - } else { - full_path /= what; - } + auto full_path = fs::path(path) / filename; std::ifstream f(full_path); if (f.good()) { return full_path.string(); } } std::ostringstream err_msg; - err_msg << "Could not find '" << what << "' in any of:\n"; + err_msg << "Could not find '" << filename << "' in any of:\n"; for (const auto& path: search_paths) { err_msg << "\t" << path << "\n"; } From e7165e70a974cdd68eba169fe8546066e4c7a699 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 15:47:49 +0200 Subject: [PATCH 22/25] Silly typo. --- src/config/config.cpp.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index c8c9161adb..1c6a438652 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -93,7 +93,7 @@ std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library std::string filename = what; if (is_library) { - filename = SHARED_LIBRARY_PREFIX + what + SHARED_LIBRARY_SUFFIX;j + filename = SHARED_LIBRARY_PREFIX + what + SHARED_LIBRARY_SUFFIX; } // check paths in order and return if found From 166295280b60b60d6adace4ae890809939ca3767 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 15:48:56 +0200 Subject: [PATCH 23/25] ENV variable has no _ --- src/config/config.cpp.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cpp.in b/src/config/config.cpp.in index 1c6a438652..ab33ed6b0b 100644 --- a/src/config/config.cpp.in +++ b/src/config/config.cpp.in @@ -81,7 +81,7 @@ std::string maybe_from_env(const std::string& varname) { } -const std::string nmodl::PathHelper::NMODL_HOME = maybe_from_env("NMODL_HOME"); +const std::string nmodl::PathHelper::NMODL_HOME = maybe_from_env("NMODLHOME"); std::string nmodl::PathHelper::get_path(const std::string& what, bool is_library) { std::vector search_paths = BASE_SEARCH_PATHS; From 19d0fa5230d9f91df50e1c0a448380f7b56dbcb6 Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 16:17:12 +0200 Subject: [PATCH 24/25] Test Azure variable differently? --- azure-pipelines.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9be1bd7756..3c00d62b82 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -298,10 +298,10 @@ stages: displayName: 'Install Dependencies' - pwsh: | - if (Compare-Object $env:RELEASEWHEELBUILD "False") { - Write-Output "##vso[task.setvariable variable=TAG;]-nightly" - } else { + if ($env:RELEASEWHEELBUILD) { Write-Output "##vso[task.setvariable variable=TAG;]" + } else { + Write-Output "##vso[task.setvariable variable=TAG;]-nightly" } displayName: "Set wheel tag" From 86024d5b537c73c0b473b99595395593137f5a9e Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Wed, 24 Jul 2024 16:19:23 +0200 Subject: [PATCH 25/25] List files with pip. --- packaging/test_wheel.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packaging/test_wheel.ps1 b/packaging/test_wheel.ps1 index 967415b426..d281f29098 100644 --- a/packaging/test_wheel.ps1 +++ b/packaging/test_wheel.ps1 @@ -24,9 +24,11 @@ if ($venv) { pip uninstall -y nmodl nmodl-nightly pip install "${wheel}[test]" - pip show nmodl-nightly } +pip show -f nmodl +pip show -f nmodl-nightly + Get-ChildItem -Path $TEST_DIR/input -Filter "*.mod" | ForEach-Object { $path = $_ -replace "\\","/" Write-Output "nmodl -o $TEST_DIR/output $path sympy --analytic"