Adjust program and args when running executables on Windows by punchagan · Pull Request #13293 · ocaml/dune

punchagan · 2026-01-12T10:34:32Z

Windows doesn't support shebang lines and build rules generated from opam build instructions could have rules like (build (run <executable-script>)), which would fail to run on Windows. This commit adds support to parse shebang lines of such executable scripts on Windows and run them with an appropriate executable.

Closes #11174

otherlibs/stdune/src/proc.ml

otherlibs/stdune/src/proc.mli

src/dune_engine/process.ml

Alizter · 2026-01-12T17:19:49Z

Could you add a test in test/blackbox-tests/test-cases/actions/run-shebang.t. You can also mark it with

(cram
 (applies_to run-shebang)
 (alias runtest-windows))

This will run on macOS and Linux by default, and additionally on Windows which is what we want to fix.

src/dune_engine/process.ml

Leonidas-from-XIV

Code looks good to me, the implementation makes sense and the tests are fine.

Some minor concerns about wording but otherwise its good to go from my perspective. Thanks for the fix!

src/dune_engine/process.ml

test/blackbox-tests/test-cases/actions/run-shebang.t

rgrinberg

The most important thing about this change is to make sure that we only do it once per executable.

Also copying @nojb for awareness.

punchagan · 2026-01-14T06:37:20Z

The most important thing about this change is to make sure that we only do it once per executable.

Makes sense. I've now added a Hashtbl to keep track of the executable paths we discovered. Let me know if that's too simplistic and we should do better.

nojb · 2026-01-14T07:55:35Z

I am not aware of the context, but this seems of limited usefulness to me: is there any reason to expect Windows builds to have executables such as sh, etc available in the environment? Is that the case when using OPAM? Anyway, am not opposed.

Leonidas-from-XIV · 2026-01-14T10:19:27Z

Can we do the caching per executable with Memo somehow? If we keep track of the input we should be able to memoize the resolved value and also be able to recompute it when the input is updated…

@nojb This is mostly about dune files that use ./foo.sh which breaks even when run in an Unix emulation environment where sh or other commands would be available, simply because Windows does not support shebangs. It copies the same behavior from OPAM.

nojb · 2026-01-14T10:24:12Z

@nojb This is mostly about dune files that use ./foo.sh which breaks even when run in an Unix emulation environment where sh or other commands would be available, simply because Windows does not support shebangs. It copies the same behavior from OPAM.

Thanks for the pointer. Of course, it would be better for users to call the shell explicitly: (run sh ./foo.sh), but I understand the point of the PR. Thanks!

Alizter · 2026-01-14T11:09:37Z

@nojb The decision to change the semantics of run on Windows is something that was discussed previously in:

This PR is intending to resolve the discussion by allowing the same behaviour (of running scripts) across platforms. This is not a simple change and will require careful consideration about whether or not we would like it and if it makes sense for dune as a whole.

Motivation

The motivating use-case for this PR is that when we translate opam build instructions into dune actions and serialise them to a lock file, we cannot know if a given file is an executable or a script without running the action. This means that so far we have only translated such build instructions into (run) actions.

This of course leads into trouble when running such actions on Windows since we can only execute executable files (with extension .exe). There are some edge-cases for handling batch and powershell scripts, but they are not relevant here.

Ultimately, the analogy we draw in Dune between Windows' CreateProcess() and Unix's exec breaks down when it comes to shebangs. Shebangs amongst Unixes are not even standardised. In fact, they are explicitly left undefined in POSIX:

If the first line of a file of shell commands starts with the
characters #!, the results are unspecified.

See here for more details: https://www.in-ulm.de/~mascheck/various/shebang/

Solution

The solution here is (or should be) to understand if a given file is an executable before running it on Windows. If it is, then proceed as usual with CreateProcess() which spawn does for us. However, if it is a script, then we need to inspect the first line, parse it as a shebang, and use whatever interpreter we can understand (if we are aiming for maximum flexibility). I don't know if Windows can detect if a file is/was marked as executable, which might affect things here. There is no "marking as executable" on Windows AFAIK. Git on Windows certainly can do some tracking, but I am not too familiar with the details here.

The main difficulty comes from the flexibility in the definition of a shebang. I am inclined to think that we should be more restrictive with the kinds of interpreters we allow, for example bash and sh in their standard "locations" should be good enough. However there are scripts in the Wild with python3 and even ocaml in the shebang, so we need to make an explicit choice here.

Prior "art"

Other build systems have considered making analogous changes that may be worth reading:

Both cases had a different problem, required the analogous script detection as a possible fix but ultimately ended up with fixing the original problem a different way. That's maybe something we should consider doing here.

Alternatives

For solving the issue with lock directories on Windows, we can consider introducing an alternative run action that is used on all platforms which avoids us from having to make such an invasive change here. The reason this change is invasive here is because its a problem outside of lock directories.

If an author has written:

(rule
 (alias do-thing)
 (deps my-script.sh)
 (action
  (run ./my-script.sh)))

Is there really a good reason to fail this on Windows?

Alizter · 2026-01-21T00:25:58Z

src/dune_rules/run_with_path.ml

+  let adjust_prog_and_args ~metadata ~path ?dir prog args =
+    let { Process.loc; annots; _ } = metadata in
+    let prog_name = Path.basename prog in
+    let parse_win32_prog_and_args prog =


An alternative:

let parse_win32_prog_and_args prog = match Io.file_line prog 1 with | exception _ -> None, [] | line -> match String.drop_prefix line ~prefix:"#!" with | None -> None, [] | Some rest -> parse_shebang_line rest

The string.trim is stripping the trailing \r we get on Windows which is a bit confusing. Also unsure about the binary mode.

Wouldn't Io.file_line prog 1 end up reading a lot of unnecessary data, especially from binary files when a newline character doesn't appear anywhere near the beginning of the file? My implementation tries to read only the first 2 characters before trying to process the first line.

Also, I don't understand what you mean by "stripping the trailing \r" being confusing.

Io.file_line sets binary to false (so Windows line endings turn from \r\n to \n) and skips lines in a loop. It doesn't read the whole file, thus file_line prog 1 is basically just a single input_line on a non-binary file.

I did look at the source of Io.file_line and I understand it that on a non-binary file it does read the first line correctly. But on a binary file, we could end-up reading too many chars, if the newline doesn't appear anywhere the beginning. But, I guess I don't need to worry about it since the DOS Stub ends with a newline, and this code only runs on Windows.

How does opam do it? Perhaps we should just copy them.

I wrote the initial version after looking at the opam workaround shared by David in #11174 (comment) . I guess, I should've explicitly mentioned that earlier. (I had context from #11174 which others looking at this PR may be missing!)

test/blackbox-tests/test-cases/pkg/build-with-executable-script-on-windows.t

Alizter

I'm happy with the state that this PR is in. If issues arise on Windows with typical build scripts we can of course always revisit this problem.

I have only a few user-facing concerns:

If we see an invalid shebang, what should a user do? We could probably give a little more information in the error message such as the location for the (build) expression and perhaps telling them to report it if they believe its a valid shebang etc.

punchagan · 2026-01-23T06:49:05Z

If we see an invalid shebang, what should a user do? We could probably give a little more information in the error message such as the location for the (build) expression and perhaps telling them to report it if they believe its a valid shebang etc.

I've added a user-facing message to this effect. Let me know if there's anything else I need to fix in this PR, before we are able to merge this.

Leonidas-from-XIV · 2026-01-30T09:06:03Z

src/dune_rules/run_with_path.ml

+           | _ -> prog, args))
+    in
+    match parse_win32_prog_and_args prog with
+    | exception Sys_error _ -> prog, args


What is throwing exceptions here?

Stdune.Io.with_file_in would throw when prog doesn't exist.

Ah, I see. Maybe worth defining with_file_in_opt that just wraps Stdune.Io.with_file_in to contain the exception?

I'm happy to do that in a separate PR. I think it's better to keep this one focused on the Windows related change?

This PR introduces the exceptions into this context (thrown by with_file_in but caught around parse_win32_prog_and_args), so it would make sense to keep the code exception-free from the start instead of merging it and then removing it later.

test/blackbox-tests/test-cases/pkg/build-with-executable-script-on-windows.t

test/blackbox-tests/test-cases/pkg/build-with-executable-script.t

Alizter · 2026-02-02T09:19:30Z

As we've discussed before. There were a few more changes I would like:

#! on its own line gives End_of_file from input_line
having a space after a shebang and before \r\n causes some issues with the parsing
removing the hash table (I think this was done?)

punchagan · 2026-02-02T10:42:06Z

Thanks for listing out the changes!

#! on its own line gives End_of_file from input_line

I've added a test case for this and tweaked the code to handle this correctly.

* having a space after a shebang and before `\r\n` causes some issues with the parsing

I've added a test case for this, but couldn't find any issues. If you have an example handy, that would be helpful.

* removing the hash table (I think this was done?)

Yes, I've removed this.

src/dune_rules/run_with_path.ml

Windows doesn't support shebang lines and build rules generated from opam build instructions could have rules like `(build (run <executable-script>))`, which would fail to run on Windows. This commit adds support to parse shebang lines of such executable scripts on Windows and run them with an appropriate executable. Closes ocaml#11174 Signed-off-by: Puneeth Chaganti <punchagan@muse-amuse.in>

rgrinberg

LGTM.

I think it would be better to test this behavior on every OS by making it possible to enable it in a config. Few of us have access to Windows otherwise.

punchagan marked this pull request as draft January 12, 2026 10:37

Leonidas-from-XIV reviewed Jan 12, 2026

View reviewed changes

otherlibs/stdune/src/proc.ml Outdated Show resolved Hide resolved

otherlibs/stdune/src/proc.ml Outdated Show resolved Hide resolved

otherlibs/stdune/src/proc.mli Outdated Show resolved Hide resolved

punchagan force-pushed the win32-proc branch 2 times, most recently from 8bcdd4b to 5c9bf01 Compare January 12, 2026 13:58

Alizter reviewed Jan 12, 2026

View reviewed changes

src/dune_engine/process.ml Outdated Show resolved Hide resolved

punchagan force-pushed the win32-proc branch from 5c9bf01 to bd43a4a Compare January 12, 2026 16:15

punchagan force-pushed the win32-proc branch 4 times, most recently from c0baf74 to 5f70449 Compare January 13, 2026 07:46

punchagan changed the title ~~WIP: Switch the executable name on win32~~ Adjust program and args when running executables on Windows Jan 13, 2026

punchagan marked this pull request as ready for review January 13, 2026 07:47

punchagan force-pushed the win32-proc branch 2 times, most recently from 8a74a42 to 1082424 Compare January 13, 2026 09:05

Leonidas-from-XIV reviewed Jan 13, 2026

View reviewed changes

src/dune_engine/process.ml Outdated Show resolved Hide resolved

src/dune_engine/process.ml Outdated Show resolved Hide resolved

src/dune_engine/process.ml Outdated Show resolved Hide resolved

punchagan force-pushed the win32-proc branch 2 times, most recently from 40bc68c to 0179940 Compare January 13, 2026 11:04

Leonidas-from-XIV reviewed Jan 13, 2026

View reviewed changes

src/dune_engine/process.ml Outdated Show resolved Hide resolved

punchagan force-pushed the win32-proc branch from 0179940 to 994a095 Compare January 13, 2026 14:52

Leonidas-from-XIV approved these changes Jan 13, 2026

View reviewed changes

src/dune_engine/process.ml Outdated Show resolved Hide resolved

test/blackbox-tests/test-cases/actions/run-shebang.t Outdated Show resolved Hide resolved

test/blackbox-tests/test-cases/actions/run-shebang.t Outdated Show resolved Hide resolved

rgrinberg reviewed Jan 13, 2026

View reviewed changes

Alizter self-requested a review January 13, 2026 22:21

punchagan force-pushed the win32-proc branch 2 times, most recently from 38294c9 to 4ad2830 Compare January 14, 2026 06:36

Alizter reviewed Jan 21, 2026

View reviewed changes

punchagan force-pushed the win32-proc branch from 963e790 to 5cd2550 Compare January 21, 2026 07:45

Alizter reviewed Jan 21, 2026

View reviewed changes

test/blackbox-tests/test-cases/pkg/build-with-executable-script-on-windows.t Outdated Show resolved Hide resolved

Alizter approved these changes Jan 21, 2026

View reviewed changes

Alizter requested a review from rgrinberg January 21, 2026 12:13

punchagan force-pushed the win32-proc branch 5 times, most recently from 80fc7a3 to e22bc8a Compare January 23, 2026 06:47

Alizter removed the requires-team-discussion This topic requires a team discussion label Jan 23, 2026

punchagan force-pushed the win32-proc branch from e22bc8a to 310c0e5 Compare January 23, 2026 13:03

shonfeder requested review from Alizter and Leonidas-from-XIV January 29, 2026 13:34

Leonidas-from-XIV reviewed Jan 30, 2026

View reviewed changes

punchagan force-pushed the win32-proc branch 2 times, most recently from 5bfb134 to eb8fc1c Compare February 2, 2026 10:40

Alizter reviewed Feb 2, 2026

View reviewed changes

src/dune_rules/run_with_path.ml Show resolved Hide resolved

Alizter approved these changes Feb 2, 2026

View reviewed changes

punchagan force-pushed the win32-proc branch from eb8fc1c to 1144660 Compare February 2, 2026 11:05

Alizter requested review from rgrinberg and removed request for rgrinberg February 2, 2026 11:15

punchagan force-pushed the win32-proc branch from 1144660 to 3170cbe Compare February 2, 2026 13:05

rgrinberg approved these changes Feb 2, 2026

View reviewed changes

Alizter merged commit 2d96f06 into ocaml:main Feb 3, 2026
28 checks passed

punchagan deleted the win32-proc branch February 3, 2026 04:09

Conversation

punchagan commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alizter commented Jan 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

punchagan commented Jan 14, 2026

Uh oh!

nojb commented Jan 14, 2026

Uh oh!

Leonidas-from-XIV commented Jan 14, 2026

Uh oh!

nojb commented Jan 14, 2026

Uh oh!

Alizter commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Prior "art"

Alternatives

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alizter left a comment

Choose a reason for hiding this comment

Uh oh!

punchagan commented Jan 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alizter commented Feb 2, 2026

Uh oh!

punchagan commented Feb 2, 2026

Uh oh!

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

punchagan commented Jan 12, 2026 •

edited

Loading

Alizter commented Jan 14, 2026 •

edited

Loading