Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006903OCamlplatform support (windows, cross-compilation, etc)public2015-06-14 11:552017-09-26 11:11
Assigned To 
PlatformOSOS Version
Product Version4.02.1 
Target Version4.03.1+devFixed in Version4.05.0 +dev/beta1/beta2/beta3/rc1 
Summary0006903: Unix.execvpe doesn't change environment on Cygwin and maybe Windows
DescriptionThe following reproduces the issue:

  open Unix
  let env = Array.concat [ [| "LAPIN=lapin" |] ; Unix.environment () ]
  let () =
    match fork () with
    | 0 -> execvpe "env" [| "env" |] env
    | x -> ()

Run inside cygwin and pipe the output to 'grep lapin'. You should see nothing (unless you already had that environment variable defined).

The code in otherlibs/unix/execvp.c looks fine but does nothing. However, Unix.execve works. Both bindings are very old: execvpe in 1996 and execve in 1995 (this one, untouched since then).

The code for Unix.execvpe plays with the environ variable rather than calling execvpe(); I don't know why and this dates back to 1996: "Renommage unix.h -> unixsupport.h Petites adaptations pour Win32." ( ac9f8e8094ea19199a24fd5c67053718198ed172 / rev 951 ). One possibility is that is execvpe wasn't available back then on Windows.

As for why this doesn't work, if I remember correctly, Windows has two environments: one for ansi, one for unicode and this would require getters and setters rather than direct access to the variable.

I'm attaching a patch. I've only tested it on Cygwin and it fixes the testcase above.

I've also been a bit surprised by the following in otherslibs/threads/ (indent mine):
  let execve proc args env = do_exec (fun () -> _execve proc args env)
  let execvp proc args = do_exec (fun () -> _execvp proc args)
  let execvpe proc args = do_exec (fun () -> _execvpe proc args)

I guess that not specifying the last argument for execvpe works because of partial application but it's a bit surprising to see it written that way.
TagsNo tags attached.
Attached Filespatch file icon ocaml_execvpe.patch [^] (840 bytes) 2015-06-14 11:55 [Show Content]

- Relationships
related to 0004499resolvednojebar Changes to the environment are invisible to Sys.getenv 
related to 0007640resolvedxleroy Unix.execvpe issues since 4.05.0 

-  Notes
gasche (administrator)
2015-06-14 12:15

Could you also add the failing case to the testsuite?

(A new directory tests/lib-unix would seem fitting)
frisch (developer)
2015-06-15 10:28

> Windows has two environments

On a related note, see also 0004499: libc functions don't see changes caused by native Windows calls (SetEnvironmentVariable).
nbraud (reporter)
2015-08-01 15:43

Attached patch might cause build to fail on Mac OS X.
This may be related to [^]
adrien (reporter)
2015-08-03 13:11

It was actually me asking nbraud over IRC to comment here because I wasn't home and was worried I would forget.

I just checked and, from "man execvpe" on Linux:
       POSIX.1-2001, POSIX.1-2008.
       The execvpe() function is a GNU extension.

This has just gotten me very very sad because I believe manipulating environ in a threaded environment is racy. Well, forking in a threaded environment is racy but it's a good idea troubles as much as possible.
xleroy (administrator)
2015-11-28 12:36

Right, execvpe() is not POSIX, just a GNU extension. Could you please revise your patch accordingly? At a minimum, it should be

#ifdef _GNU_SOURCE
  // your new code using execvpe()
  // the old implementation that modifies "environ"

Alternatively, you could implement execvpe in terms of execve() (which is POSIX), using caml_search_exe_in_path (from the OCaml runtime system) to find the actual location of the executable.

adrien (reporter)
2015-12-03 21:30

NB: execvpe() is also available on Windows.

Musl-libc is MIT-licensed and has an implementation of execvpe on top of execve which can be seen at [^] .

Would it be OK to re-use that for platforms without execvpe() and to call execvpe() directly otherwise?

I don't know if I'll be able to do something myself before the next release and I don't want to "lock" the update of the patch to myself. In any case I'll do my best to review and test any patch on the topic.
xleroy (administrator)
2015-12-04 16:09

No, it's not OK to reuse the Musl code, because 1- it would probably not work under Windows, and 2- the OCaml runtime already contains functions to search executables in PATH, see "caml/osdeps.h".
adrien (reporter)
2016-01-11 07:39

Realistically I won't have time to push this forward in time for the next release: I'd like to write and test a couple patches to see and compare them but I'm too short on time for this during January and I think this issue can wait a bit more if needed.
xleroy (administrator)
2017-02-14 13:55

Fixed by commit [trunk 3627221], should be in 4.05.

- Issue History
Date Modified Username Field Change
2015-06-14 11:55 adrien New Issue
2015-06-14 11:55 adrien File Added: ocaml_execvpe.patch
2015-06-14 12:15 gasche Note Added: 0014067
2015-06-15 10:27 frisch Relationship added related to 0004499
2015-06-15 10:28 frisch Note Added: 0014069
2015-06-15 17:58 doligez Status new => acknowledged
2015-06-15 17:58 doligez Target Version => 4.03.0+dev / +beta1
2015-08-01 15:43 nbraud Note Added: 0014312
2015-08-03 13:11 adrien Note Added: 0014314
2015-11-28 12:36 xleroy Note Added: 0014886
2015-11-28 12:36 xleroy Status acknowledged => feedback
2015-12-03 21:30 adrien Note Added: 0015020
2015-12-03 21:30 adrien Status feedback => new
2015-12-04 16:09 xleroy Note Added: 0015035
2016-01-11 07:39 adrien Note Added: 0015248
2016-01-22 17:20 doligez Status new => confirmed
2016-01-22 17:20 doligez Target Version 4.03.0+dev / +beta1 => 4.03.1+dev
2017-02-14 13:55 xleroy Note Added: 0017254
2017-02-14 13:55 xleroy Status confirmed => resolved
2017-02-14 13:55 xleroy Resolution open => fixed
2017-02-14 13:55 xleroy Fixed in Version => 4.05.0 +dev/beta1/beta2/beta3/rc1
2017-02-23 16:46 doligez Category OCaml windows => platform support (windows, etc)
2017-02-23 17:16 doligez Category platform support (windows, etc) => platform support (windows, cross-compilation, etc)
2017-09-26 11:11 frisch Relationship added related to 0007640

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker