Replace hocload.sh with std::filesystem utils
#3293
Conversation
| #if defined(WIN32) | ||
| static const auto os_pathsep = std::string(";"); | ||
| #else | ||
| static const auto os_pathsep = std::string(":"); | ||
| #endif |
There was a problem hiding this comment.
Not sure if the scope of this var should be placed at the level of split_paths (see below) or left as-is at file level scope.
| const std::vector<std::string>& paths) { | ||
| namespace fs = std::filesystem; | ||
| for (const auto& path: paths) { | ||
| for (const auto& entry: fs::directory_iterator(path)) { |
There was a problem hiding this comment.
Not sure if this returns the same ordering as egrep '^func|^proc|^begintemplate' $p/*.oc $p/*.hoc (the C++ standard does not specify ordering, so I guess it depends on the library?).
There was a problem hiding this comment.
The entries may not be sorted at all, whereas the shell bit you point out first goes through all .oc files, followed by all .hoc.
Since only the first match is used, would one expect multiple definitions of the same entity?
There was a problem hiding this comment.
The entries may not be sorted at all
Indeed! I changed the implementation so it's actually sorted, it should even respect the locale now since that's what the POSIX glob does (had a whole rant ready about locales, but it turns out it's surprisingly simple to use, at least for this usecase :)
Since only the first match is used, would one expect multiple definitions of the same entity?
I'd expect it to throw an error in case of multiple definitions, or provide some mechanism to disambiguate, but I don't think there is anything of the sort, so I just sought to replicate the existing behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3293 +/- ##
==========================================
- Coverage 67.07% 67.05% -0.02%
==========================================
Files 571 571
Lines 111055 111097 +42
==========================================
+ Hits 74486 74492 +6
- Misses 36569 36605 +36 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
|
✔️ 6a3f8d8 -> Azure artifacts URL |
|
Model 150245 from modelDB fails (see https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/12369627580/job/34521930207), but I am 99.99% sure it's not related since it fails on the nightly as well (see https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/12361469702/job/34498702435). |
|
|
✔️ 253637f -> Azure artifacts URL |
hoc_loadseems to be doing the following:proc,func,begintemplategargstr) is a defined symbol in the current scopehocload.shwith 3 args: the keyword, the symbol name, andhoc_pid. This script seems togrepall HOC files in${PWD},${HOC_LIBRARY_PATH}for the patternfunc|proc|begintemplate, stores them in the temp filenames, then usessedto find out whether there is a matching symbol in some file. If there is, it echoes the first match, and returns 0, and if there aren't any matches, it returns 1.hoc_Load_fileI've replaced most of the above using
std::filesystem, though I am not sure whether the logic of returning the first match is right in the first place, but nevertheless I tried replicating the existing behavior as closely as possible.