Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use direct call to external tool #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions rfcs/direct_call_to_external_tool.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Direct call to external toolchain

The compiler shouldn't require a shell in order to work.

# Motivation for the change

I try to fix this https://github.com/esy/esy/issues/1344
I can reduce process in esy but half of this has to be fixed in ocaml/flexdll

# Technical details of the change

Replace Sys.command by Unix.create_process or Unix.open_process_args

# Drawbacks of the change and alternatives to the change

pros:
- You earn some time without launching useless shell
- You avoid the [quoting nightmare](https://github.com/ocaml/ocaml/pull/10727) on OS with exec*
- You handle better all path (I have not tested path with $,; and other niceties)
- You workaround cmd limitation of 8187

cons:
- The flags -pp/-ppx 'binary -args' will not work
- User can still create a wrapper script
- We can add --ppopt, -ppxopt like -ccopt
- We can add -direct_pp,-direct_ppx so we don't break any projects. All good citizens that use dune will get a boost benefit freely.
- You loose usage of %COMSPEC% on windows (but is it still needed ?)
- We can still read that variable and act in consequence
- We break user that rely on this implementation detail and pass multiple opt to -ccopt/-cclib instead of repeating it.
- Workaround either split on space and/or implement word splitting that respect quoting.

cons without workaround:
- Unix will not be optional anymore (or maybe at least the creation part)

# Unresolved questions