sheredom / subprocess.h

🐜 single header process launching solution for C and C++
The Unlicense
1.1k stars 97 forks source link

Add CWD option to subprocess_create_ex #83

Open caesay opened 5 months ago

caesay commented 5 months ago

Closes #52.

Disclaimer: I am not a posix developer. I verified the Windows code works, but did not test the UNIX stuff. I just copied this code from that issue without testing:

  // Set working directory
  if (0 != posix_spawn_file_actions_addchdir_np(&actions, process_cwd)) {
    posix_spawn_file_actions_destroy(&actions);
    return -1;
  }

I did notice that there was a posix_spawn_file_actions_addchdir_np function (from the screenshot) and a posix_spawn_file_actions_addchdir function, the latter of which is "portable" and the former is "non-portable". I don't really understand what that means in this context, however when searching I did notice that Rust uses the "non-portable" variation, so I'm guessing it's the preferred one.

sheredom commented 5 months ago

As far as I know (not an expert either) the _np ones are just not compliant with POSIX, but if enough systems support them you can just YOLO use them. For instance macOS has a bunch of them that aren't standard, but on macOS they effectively are!

shakfu commented 5 months ago

I was a getting a segfault with this PR version and I figured out how to fix it by making the following change to subprocess_create (i.e. changing SUBPROCESS_NULL to "."):

int subprocess_create(const char *const commandLine[], int options,
                      struct subprocess_s *const out_process) {
  return subprocess_create_ex(commandLine, options, SUBPROCESS_NULL,
                              // out_process, SUBPROCESS_NULL);
                              out_process, ".")
caesay commented 5 months ago

It will need to be SUBPROCESS_NULL on windows, I suspect a better fix will be to check if cwd is null, and if so, skip the call to posix_spawn_file_actions_addchdir_np. I'll make this change along with the other comments on the PR.

caesay commented 5 months ago

I have implemented the requested changes. Before doing so, I noticed some tests on ubuntu had failed with:

error: implicit declaration of function ‘posix_spawn_file_actions_addchdir_np’;

I assume this is because the function is missing, but this function is imported via the spawn.h header, so I'm guessing this function is just missing on that OS/compiler? Not sure how to resolve this.

sheredom commented 5 months ago

Yeah we're gonna have to find another function we can use on Linux it seems. I'll have a google about.