Cleaning nrnivmodl.in #1781
Cleaning nrnivmodl.in #1781
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1781 +/- ##
==========================================
- Coverage 45.49% 45.45% -0.04%
==========================================
Files 551 551
Lines 113149 113149
==========================================
- Hits 51476 51431 -45
- Misses 61673 61718 +45
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
There was a problem hiding this comment.
Wouldn't say I have confidence in my BASH skills but overall looks OK to me. (I would have tagged @matz-e but he is off this week as well)
Only prominent thing that came to my mind is that we should have a test that covers compiling of MOD file in a directory containing spaces.
|
Yes, i agree with that, the whole file is designed and complex for support this and i was not able to find a single file with space in nmodldb! |
@alkino : actually, it's about the mkdir -p /tmp/drive/Shared with me/
cd /tmp/drive/Shared with me/
# copy some mod files in .
nrnivmodl . |
bin/nrnivmodl.in
Outdated
| if [ $(echo "\n") ] ; then | ||
| newline="\n" | ||
| else | ||
| newline="\\\\n" | ||
| fi |
There was a problem hiding this comment.
I'm not convinced by that and I don't understand what it tried to fix. Some problem with bash & windows?
There was a problem hiding this comment.
I don't recall exactly, but it would have to have been the result of trial and error to get
echo '
void modl_reg(){
if (!nrn_nobanner_) if (nrnmpi_myid < 1) {
fprintf(stderr, "Additional mechanisms from files'$newline'");
' >> mod_func.cpp
to emit a valid line fprintf(stderr, "Additional mechanisms from files\n");
I observe on the mac (zsh) that
% echo "\n"
%
though it is correct on the mac with the bash shell.
There was a problem hiding this comment.
I have no objection to updating to bash syntax from the bog level bourne shell syntax (that was just my habit from autotools days).
Compiling c files was always a possibility. But nowadays, given the shift to c++, has become problematic. No objection to removing that option.
A lot of effort went into the handling of spaces in file paths. That is very common on mac and windows. I don't know much about shopts but if that solves that problem as well, then I'm fine with the change. I agree with @pramodk that a test with several mod files with path spaces would be helpful.
An example of interesting nrnivmodl usage which required some workaround for coreneuron is in the tqperf repository README.md file. I.e.
# The -lcrypto is required for modx/invlfiresha.mod
cp mod/*.mod modx # until allow multiple directories for nrnivmod-core
nrnivmodl -coreneuron -l -lcrypto -loadflags -lcrypto modx
I would rather have written
nrnivmodl -coreneuron -l -lcrypto -loadflags -lcrypto mod modx
If this new version works, as the old one did, with
nrnivmodl -l -lcrypto "./path with spaces/mod" "./path with spaces/modx"
then I approve.
|
|
|
A directory with a space has been added |
|
@nrnhines You mean that you wish that coreneuron support several mod directories. Is it that? |
Yes. Basically any number of directories that contain mod files and explicit paths to mod files. |
@nrnhines : Just to clarify here though: currently |
That is fine. |
…ividual redirects.
alkino
force-pushed
the
clean_nrnivmodl
branch
5 times, most recently
from
d383b0c to
01d802d
Compare
bin/nrnivmodl.in
Outdated
| if [ $(echo "\n") ] ; then | ||
| newline="\n" | ||
| else | ||
| newline="\\\\n" | ||
| fi |
There was a problem hiding this comment.
I don't recall exactly, but it would have to have been the result of trial and error to get
echo '
void modl_reg(){
if (!nrn_nobanner_) if (nrnmpi_myid < 1) {
fprintf(stderr, "Additional mechanisms from files'$newline'");
' >> mod_func.cpp
to emit a valid line fprintf(stderr, "Additional mechanisms from files\n");
I observe on the mac (zsh) that
% echo "\n"
%
though it is correct on the mac with the bash shell.
|
The problem with printf is the way to go to do complicate printing |
No description provided.