Skip to content

Add support for #use_output "<command>" (was: #use "| <command>")#9283

Merged
5 commits merged intotrunkfrom
unknown repository
Mar 16, 2020
Merged

Add support for #use_output "<command>" (was: #use "| <command>")#9283
5 commits merged intotrunkfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Feb 5, 2020

This PR adds a new toplevel directive #use_output "<command>" for running a command and evaluating its output.

This feature allows to embed toplevel scripts directly in executables. This is nice when you want to distribute a self-contained relocatable binary and provide some toplevel features.

I would like to use this feature in Dune to provide #use_command "dune top". It would work in any toplevel and supersede dune utop (cf ocaml/dune#2952).

@lpw25
Copy link
Copy Markdown
Contributor

lpw25 commented Feb 5, 2020

I have no particular thoughts on the feature itself, but the syntax seems unnecessarily opaque. Why not use something like #use_ouput <command>?

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 5, 2020

The pipe syntax looks nicer to me, but happy to go with the flow for the syntax really.

@avsm
Copy link
Copy Markdown
Member

avsm commented Feb 5, 2020

I think Leo's suggestion of a new directive such as #use_output (or #exec) would be safer from a security perspective. Overloading filenames in order to do command execution could potentially lead to security issues if the input to the toplevel script hasn't been sanitised (for example, one of the web toplevels) and would now be able to execute commands on the host instead of just erroring out.

@ghost ghost changed the title Add support for #use "| <command>" Add support for #use_command "<command>" (was: #use "| <command>") Feb 5, 2020
@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 5, 2020

That's a good point. I updated the PR to add a new directive #use_output.

Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@ghost ghost changed the title Add support for #use_command "<command>" (was: #use "| <command>") Add support for #use_output "<command>" (was: #use "| <command>") Feb 5, 2020
jeremiedimino and others added 4 commits February 5, 2020 17:11
Co-Authored-By: David Allsopp <david.allsopp@metastack.com>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 16, 2020

This is a trivial addition to a non-critical part of the compiler, so I'm merging this.

@ghost ghost merged commit e54876a into ocaml:trunk Mar 16, 2020
dra27 added a commit to dra27/ocaml that referenced this pull request Apr 17, 2020
@dra27 dra27 mentioned this pull request Apr 17, 2020
@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 17, 2020

Note: this PR caused a regression (in trunk only) reported in #9455, about to be fixed by @dra27 in #9457.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants