eidheim / tiny-process-library

A small platform independent library making it simple to create and stop new processes in C++, as well as writing to stdin and reading from stdout and stderr of a new process
MIT License
338 stars 73 forks source link

program output not captured when program does not exist or didn't run "succesfully" #23

Closed grasmanek94 closed 7 years ago

grasmanek94 commented 7 years ago

Tested on: Windows 7 Description: Cannot get process output when process cannot be created (or process didn't run "succesfully") Code:

    Process process(options.Combine(), "", [](const char *bytes, size_t n) {
        std::cout << "xxyy" << std::string(bytes, n) << "yyxx" << std::endl;
    }, [](const char *bytes, size_t n) {
        std::cout << "xxyy" << std::string(bytes, n) << "yyxx" << std::endl;
    });

Identified cause in source code (process_win.cpp:106):

  ....
  BOOL bSuccess = CreateProcess(nullptr, process_command.empty()?nullptr:&process_command[0], nullptr, nullptr, TRUE, 0,
                                nullptr, path.empty()?nullptr:path.c_str(), &startup_info, &process_info);

  if(!bSuccess) {
    CloseHandle(process_info.hProcess);
    CloseHandle(process_info.hThread);
    return 0; // here it stops and doesn't proceed to fire the lambda's to capture output
  }
  else {
    CloseHandle(process_info.hThread);
  }

  if(stdin_fd) *stdin_fd=stdin_wr_p.detach();
  if(stdout_fd) *stdout_fd=stdout_rd_p.detach();
  if(stderr_fd) *stderr_fd=stderr_rd_p.detach();
  ....

Suggested fix:

Fire the lambda's with captured output either way (even if bSuccess == false). It's the callers responsibility to check the exit code anyway, and a separate "binary doesn't exist" code can be emitted.

eidheim commented 7 years ago

Thank you, but are you sure there are process output when the process cannot be created? Or if there should be process output if the process cannot be created? Unless I'm mistaken, the current behaviour is correct, but feel free argue against me on this:)

eidheim commented 7 years ago

Oh, Unix-like systems seems to output wrong commands when using sh -c. This seems to be a Windows issue. Feel free to look around for a solution. The problem is that bSuccess is true (if on MSYS2) but no output is sent through the pipe it seems.

edit: this worked in MSYS2, disregard my above comment.

eidheim commented 7 years ago

If the process is created through sh -c, sh will output an error message if the process cannot be created due to for instance missing/incorrect executable. However, if sh -c is not used, there will be no output to stdout or stderr at all, and this is working as intended.

grasmanek94 commented 7 years ago

If this is intended then please do see this this report as 'never submitted' :) I

I would expect there to be some output like "command doesn't exist", I actually got output on linux when a binary isn't executable (permission error) so I would expect some of the same behavior for other platforms when the executable can't be invoked / doesn't have output on its own. I do understand that not every behavior can be captured in a cross-plaform-friendly way, then those small differences at least do belong in the documentation, right? I was genuinely confused for a second when I got output on linux but not on my windows machine (later I found out the binaries were just missing, and some weren't executable).

eidheim commented 7 years ago

The command does not exist message also is output when using MSYS2 since it's using sh -c, and you can probably get a similar message on Windows outside MSYS2, but you would have to start the process with cmd /C (see https://github.com/eidheim/tiny-process-library/blob/master/examples.cpp#L120). cmd would then output this error message, but bear in mind that I did not create the cmd examples and have not tried this library outside MSYS2 on Windows.

Sorry for the confusion, but the difference in Windows and Posix process creation is significant. It is manageable though when working within MSYS2 since you have access to the sh command and other commands typically found on a Posix system.