edf-hpc / verrou

floating-point errors checker
https://edf-hpc.github.io/verrou/vr-manual.html
GNU General Public License v2.0
49 stars 13 forks source link

Find python via the environment + finish python3 port #18

Closed HadrienG2 closed 6 years ago

HadrienG2 commented 6 years ago

Currently, the path to the python binary is hardcoded to /usr/bin/python[3]. This can fail if the python executable is not installed at this location. It is better to let PATH do its job by using env instead.

lathuili commented 6 years ago

The -S option of env is not portable. We will have to test with #!/usr/bin/env PYTHONUNBUFFERED=1 python3 But before I want to add regression tests for the script verrou_dd.

HadrienG2 commented 6 years ago

I'll fix the env -S issue. Another thing which I was wondering about is, should I migrate the remaining python2 code to python3 while I'm at it? (After all, mixed python2 + python3 codebases are awkward)

HadrienG2 commented 6 years ago

So, I tried to run 2to3 on the codebase and require python3 everywhere, but that part is not tested yet => WIPping for now.

HadrienG2 commented 6 years ago

I have a strange hang here which you might be more familiar with than I am.

This does not work:

dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # head verrou_dd 
#!/usr/bin/env PYTHONUNBUFFERED=1 python3

import subprocess
import sys
import os
import shutil
#import md5
import hashlib
import copy
from valgrind import DD
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # ls -l verrou_dd 
-rwxr-xr-x 1 root root 20004 Sep  4 12:35 verrou_dd
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # python3 verrou_dd 
Usage: verrou_dd runScript cmpScript
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # PYTHONUNBUFFERED=1 python3 verrou_dd 
Usage: verrou_dd runScript cmpScript
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # /usr/bin/env PYTHONUNBUFFERED=1 python3 verrou_dd 
Usage: verrou_dd runScript cmpScript
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # ./verrou_dd
<hangs indefinitely>

This works:

dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # head verrou_dd 
#!/usr/bin/env -S python3 -u

import subprocess
import sys
import os
import shutil
#import md5
import hashlib
import copy
from valgrind import DD
dcfc7ed03ce0:/valgrind-3.13.0+verrou-dev/verrou # ./verrou_dd
Usage: verrou_dd runScript cmpScript

Any idea of how that can happen?

HadrienG2 commented 6 years ago

Ah, okay, found the answer in the "env" docs:

"Most operating systems (e.g. GNU/Linux, BSDs) treat all text after the first space as a single argument. When using ‘env’ in a script it is thus not possible to specify multiple arguments"

The more I learn about what Unices do with shebangs, the sillier it gets :)

Just out of curiosity, why do you need unbuffered python here?

HadrienG2 commented 6 years ago

Alright, I found a dirty workaround anyhow. Now, onto these remaining test failures...

HadrienG2 commented 6 years ago

I do not manage to make this fail anymore. All the tests described in the README pass, and a trivial verrou_dd run passed as well. I'm not sure how to test the scripts in the ddTest directory though. @lathuili , any input?

lathuili-home commented 6 years ago

Why we need unbuffered? Good question... I think it is just to get fancier print but I m not sur... We could think to remove it.

To run ddTest : verrou_dd ddRun.py ddCmp.py and you will get ddmax (global): sym7.c:10 (sym-7) sym7.c:9 (sym-7) sym8.c:10 (sym-8) sym8.c:9 (sym-8) sym9.c:10 (sym-9) sym9.c:9 (sym-9)

On the branch no-hardcoded-python3 on repository https://github.com/lathuili-home/verrou.git I push two corrections.

HadrienG2 commented 6 years ago

Integrated, thanks! :)

So... are we all set?