Conversation
3b7c22a to
b804a1d
Compare
|
@NathanReb is this ready for review? If not, could you add |
|
To be honest I'm not even sure myself, I'll try to add the |
|
I added the proper If you look at that test you'll note that the shell conversion of echo doesn't seem to work very well. Also it seems that the shell conversions swallow quotes when they shouldn't. I'll propabaly change that test so that it doesn't depend on |
|
I just pushed a commit to simplify the shell conversion test so if you want to see the bugs I mentioned you should take a look at this. |
|
Ok it seems like it didn't swallow single quotes when translating The bug with echo is real though! |
ghost
left a comment
There was a problem hiding this comment.
Needs a couple of tweaks regarding the handling of the temporary files, but otherwise looks good.
src/dune/action_exec.ml
Outdated
| and exec_pipe outputs ts ~ectx ~eenv = | ||
| let tmp_file () = | ||
| Path.of_string | ||
| (Filename.temp_file "pipe-action-" |
There was a problem hiding this comment.
In process.ml, there is a Temp sub-module that ensures that temporary files are deleted in case of abnormal exit. We should extract it to reuse it here.
|
This makes me think that we should improvement our management of temporary files: #3432 |
905be52 to
9a0c240
Compare
|
I just rebased and fixed the conflicts! I'm guessing I should fix the temp file mgmt issue first in a separate PR or is it okay to do it here? |
|
A separate PR is better |
ae7434e to
25d64c1
Compare
|
I just rebased on top of #3521 and slightly improved the tmp file management for pipe actions by deleting intermediate files as it goes. |
5fb7969 to
e60240a
Compare
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Use tr instead of grep: - avoid some nasty quoting - it's easier to observe the effect of transformations Signed-off-by: Jeremie Dimino <jeremie@dimino.org> Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Fixes #428
This PR introduces
pipe-outputs,pipe-stdoutandpipe-stderrto the action language.The tests don't cover much atm. In particular I'd like to make sure I got the
Action_to_shpart right but I'm unsure what's the best way to test it.I also believe I need to fix the stdout/stderr-only cases by adding a
multi_useto the one that's not being piped.