ReproNim / reproman

ReproMan (AKA NICEMAN, AKA ReproNim TRD3)
https://reproman.readthedocs.io
Other
24 stars 14 forks source link

ENH: use PS4 to providing timing of operations in runscript #513

Closed yarikoptic closed 4 years ago

yarikoptic commented 4 years ago

I think it could come very useful for various debugging attempts (not that it probably would "interfere" with the flushing approach @kyleam had done)

kyleam commented 4 years ago

I prefer to not have this in the main line and to not switch to bash unless we really need to. But if you disagree, feel free to merge.

codecov[bot] commented 4 years ago

Codecov Report

Merging #513 into master will decrease coverage by 0.00%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
- Coverage   89.60%   89.60%   -0.01%     
==========================================
  Files         148      148              
  Lines       12290    12290              
==========================================
- Hits        11013    11012       -1     
- Misses       1277     1278       +1     
Impacted Files Coverage Δ
reproman/interface/diff.py 95.00% <0.00%> (-0.72%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 214dbea...f31560c. Read the comment docs.

yarikoptic commented 4 years ago

As for bash - I would have preferred to avoid it too, emailed dash mailing list about the stalling (you CCed). Unfortunately with posh there is no evaluation of PS4 as bash does so there is no time stamp. I started to use PS4 more and more instead of ad-hoc INFO messages etc. POSIX aspect though: unfortunately https://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_03 says

PS4 When an execution trace ( set -x) is being performed in an interactive shell, before each line in the execution trace, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value is "+ ". This volume of IEEE Std 1003.1-2001 specifies the effects of the variable only for systems supporting the User Portability Utilities option.

so it is a subject only to parameter expansion and not command substitution. So I guess what bash --posix does is actually not following POSIX entirely in this respect, and I found no other way to embed at least time into PS4 :-(

yarikoptic commented 4 years ago

since could have side effects with dash (upstream issued a fix but it will take to propagate), and due to no immediate need ATM, I will just let it RiP for now.