Homogenize finding our components. #1336
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1336 +/- ##
==========================================
+ Coverage 86.82% 86.90% +0.07%
==========================================
Files 179 179
Lines 13684 13667 -17
==========================================
- Hits 11881 11877 -4
+ Misses 1803 1790 -13 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
matz-e
force-pushed
the
attempted-defenestration-of-nmodl-home
branch
from
6d9e59e to
f4863e4
Compare
src/config/config.cpp.in
Outdated
| #elif defined(__APPLE__) | ||
| char buffer[PATH_MAX + 1]; | ||
| uint32_t bufsize = PATH_MAX + 1; | ||
| if( _NSGetExecutablePath(buffer, &bufsize) != 0) { | ||
| return ""; | ||
| } | ||
| auto executable = fs::path(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; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
In the past, I didn't go via this route. But if you see that this is a relatively stable approach that peopl use and works on windows/Mac/Linux then I would say this is fine as well!
There was a problem hiding this comment.
It seems that way. All my searches more or less indicated that this is the way to go. Mac OS/Windows could use some more testing though. It at least goes through the CI so far.
This comment has been minimized.
This comment has been minimized.
At least with MSVC, the majority of the code does not even compile... |
|
@pramodk I generated a tiny reproducer on Win11/MSVC that does what it should and adjusted the code accordingly. |
|
thanks for checking! 👍 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Logfiles from GitLab pipeline #223225 (:white_check_mark:) have been uploaded here! Status and direct links: |
We currently rely on two files being present in predefined directories:
nrnunits.liblibpywrapper.so(or similar)These are searched for in:
NMODLHOMELeading to some acrobatics to always set
NMODLHOMEcorrectly. This PR attempts to also find the above files relative to the NMODL executable, in the hope that some use cases ofNMODLHOMEcan be removed.It will now also treat
nrnunits.libandlibpywrapper.sothe same, before only the former was looked for in multiple directories, while the latter relied on the environment variable.