{"action":"created","issue":{"url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/2","repository_url":"https://api.github.com/repos/klemens-morgenstern/boost-process","labels_url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/2/comments","events_url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/2/events","html_url":"https://github.com/klemens-morgenstern/boost-process/issues/2","id":133481316,"number":2,"title":"Redesign 0.6","user":{"login":"klemens-morgenstern","id":8020376,"avatar_url":"https://avatars.githubusercontent.com/u/8020376?v=3","gravatar_id":"","url":"https://api.github.com/users/klemens-morgenstern","html_url":"https://github.com/klemens-morgenstern","followers_url":"https://api.github.com/users/klemens-morgenstern/followers","following_url":"https://api.github.com/users/klemens-morgenstern/following{/other_user}","gists_url":"https://api.github.com/users/klemens-morgenstern/gists{/gist_id}","starred_url":"https://api.github.com/users/klemens-morgenstern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/klemens-morgenstern/subscriptions","organizations_url":"https://api.github.com/users/klemens-morgenstern/orgs","repos_url":"https://api.github.com/users/klemens-morgenstern/repos","events_url":"https://api.github.com/users/klemens-morgenstern/events{/privacy}","received_events_url":"https://api.github.com/users/klemens-morgenstern/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-02-13T22:58:34Z","updated_at":"2016-03-13T23:28:06Z","closed_at":null,"body":"I'd really like to have the boost.process library, but since development was on hiatus for two years, I think a few small design changes would be nice - or rather big ones, as described below.\r\nBut, not everything I write here is completely tested. I'd really need some discussion about those and other ideas how such a library could look like, because I really would change the frontend.\r\n\r\nHere's the [reference](http://www.highscore.de/boost/process0.5/index.html) of the current version.\r\n\r\n@dlaugt @borisschaeling @whizmo @hotgloupi @nat-goodspeed @havoc-io @rconde01 @JeffGarland\r\n\r\nSince we now have a variadic templates, I think the execute function can be extended and much more flexible. Please notice, that `execute` shall return an undefined type, which depends on the parameters. It might be converted to `child`, as discussed below.\r\n\r\n\r\n# 1 Defaults\r\n## 1.1 Unification\r\n\r\nFirst of all, I like to unify the signal handling, to provide the following:\r\n\r\n```cpp\r\n    void on_setup(Executor&) const\r\n    void on_error(Executor&, const std::error_code &ec) const\r\n    void on_success(Executor&) const\r\n```\r\n\r\nThis would require the posix-executor to always use the self-pipe in order to provide the on_success signal. Therefore I think it would be also possible to add a initializer which removes this, in the following way:\r\n\r\n```cpp\r\n   using namespace boost::process::initializers;\r\n   auto p = execute(\"some_prog\", posix::no_success_signal);\r\n```\r\n\r\nIf then a success handler is provided, it would provide an error, i.e.:\r\n\r\n```cpp\r\n   using namespace boost::process::initializers;\r\n   auto p = execute(\"some_prog\", posix::no_success_signal, on_success([](auto){}));\r\n   //yields an compiler error\r\n```\r\n\r\n\r\n### Posix Structure\r\n\r\nThe way it should work on posix is this:\r\n\r\n![uml](http://plantuml.com/plantuml/png/TOyz2iCm38Ntd4AZFVG26RgK-XI5O5oPY8Quo4wJuyToxL08dOJUzvv-Z4qZykLgIkqXp4F4jBoLxan5jv6c9_JW0x-wqZGjY-H1UKkJbgGu1cGEVF9bYXkwOtZbzmq1X0lyuDzO6VfpKpjtjPHQwAlbqCgKy8XKMdPaipa_N9qG-37B_ujp6AFIEm00)\r\n\r\nA possible code in posix would look like this: \r\n\r\n```cpp\r\nexecutor::operator()(...)\r\n{\r\n   pipe error_pipe = create_pipe(FD_CLOEXEC); //closes the pipe on execve call\r\n   on_setup();\r\n   auto pid = fork();\r\n   if (pid == -1)\r\n      on_fork_error();\r\n   else if (pid == 0)\r\n   {\r\n      ::close(selfe_pipe.source);\r\n      on_exec_setup();\r\n      ::execve(exe, cmd_line, env);\r\n      on_exec_error();\r\n      ::write(error_pipe.sink, &errno, sizeof(int));\r\n      ::_exit(EXIT_FAILURE);\r\n   }\r\n   //ok, here we are back in the father-process \r\n   int code, cnt;\r\n   do \r\n   {\r\n     cnt = ::read(error_pipe.source, &code, sizeof(code));\r\n      int err = errno;\r\n      if (res == EDABF)  //pipe is closed.\r\n          break;\r\n       //EAGAIN not yet forked, EINTR interrupted, i.e. try again\r\n      else if ((err != EAGAIN) && (err != EINTR)) \r\n      else\r\n          throw error();\r\n   }\r\n   while(cnt == -1);\r\n\r\n   if (cnt == 4) //error.\r\n       on_error(code);\r\n   else if (cnt > 0) //some other strang error\r\n       on_error(...); //don't know yet\r\n   else\r\n       on_success();\r\n}\r\n```\r\n\r\nNow, on default the father process is waiting. I think to provide a `ignore_error` would be very usefule, since this would then ignore this. \r\n\r\nThe additional signal-handlers will be availble as part of the `boost::process::posix` namespace.\r\n\r\n## 1.2 Throw on error\r\n\r\nI also would make the throw_on_error the default, because it just makes the most sense, since execute constructs a process-object. I.e. not being able to construct it should result in an error.\r\n\r\n```cpp\r\nauto p1 = execute(\"not_runnable\"); //throws\r\nauto p2 = execute(\"not_runnable\", ignore_error); //no throw, no handling;\r\nstd::error_code ec;\r\nauto p3 = execute(\"not_runnable\", set_on_error(ec)); //no throw, takes the error\r\nauto p3 = execute(\"not_runnable\", ec); //no throw, takes the error, shortcut.\r\n```\r\n\r\n I would introduce `posix::no_success_signal` and `ignore_error` because the posix implementation now waits for the child-process to start. A user might consider that unneeded and could hence wish to skip that.\r\n\r\n# 2. Simplified syntax\r\n\r\n## 2.1. Idea\r\n\r\nI think it would be possible to make the syntax much simpler, as in the following examples (corresponding to the current). All are equal.\r\n\r\n```cpp\r\nauto p1 = execute(exe(\"cmd\"), args(\"x\", \"y\")); //shortened version of the current syntax\r\nauto p2 = execute(exe=\"cmd\", args={\"x\", \"y\"}); //alternate syntax\r\nauto p3 = execute(exe=\"cmd\", args=\"x\", args+=\"y\"); //other syntax\r\nauto p4 = execute(\"cmd\", \"x\", \"y\"); //lazy syntax\r\nauto p5 = execute(cmd(\"cmd x y\"));\r\nauto p6 = execute(cmd=\"cmd x y\");\r\nauto p7 = execute(\"cmd x y\");\r\n```\r\n\r\nThe cmd and args get parsed differently, that is:\r\n\r\n```cpp\r\nexecute(cmd=\"cmd x y z\");//yields \"cmd x y z\"\r\nexecute(cmd=\"cmd \\\"x y\\\" z\");//  yields \"cmd \\\"x y\\\" z\"\r\nexecute(exe=\"cmd\", args={\"x y\", \"z\"}); //yields \"cmd \\\"x y\\\" z\"\r\n```\r\n\r\n## 2.2. Other initializers\r\n\r\nThe following signal handlers shall have syntax similar to the above.\r\n \r\n  - inherit_env\r\n  - env\r\n  - start_dir\r\n\r\nAnd the platform specifics.\r\n\r\n## 2.3 I/O\r\n\r\n*This is the part where I am the least confident, but those are all things I consider possible.*\r\n\r\n### 2.3.1  Close Pipe / Redirect to null\r\n\r\nIf a pipe shall be closed, it should be possible in the following way\r\n\r\n```cpp\r\nexecute(\"ls\", std_out=null);  //redirect to /dev/null\r\nexecute(\"ls\", std_out=close); //close the pipe\r\n//alternate syntax\r\nexecute(\"ls\", std_out>null);  //redirect to /dev/null\r\nexecute(\"ls\", close(std_out)); //close the pipe\r\n```\r\n\r\n### 2.3.2 Sync I/O\r\n\r\nThe sync I/O can be simplified in the following way (current [example](http://www.highscore.de/boost/process0.5/boost_process/tutorial.html#boost_process.tutorial.synchronous_i_o)). I.e. the pipe can be constructed automatically.\r\n\r\n```cpp\r\nfile_descriptor_sink sink;\r\nexecute(\"my_prog\", std_in=sink);//alternatively '|', because pipe.\r\nstream<file_descriptor_sink> str;\r\n```\r\n\r\n```cpp\r\nstream<file_descriptor_source> str;\r\nexecute(\"my_prog\", std_out=str);\r\n```\r\n\r\nIf the output shall be redirectet into a file, this can be done via a `boost::filesystem::path`:\r\n\r\n```cpp\r\nexecute(\"my_prog\", std_out=path(\"./log.txt\"));\r\n```\r\n\r\nIt may also be following to allow a lazy redirection to files:\r\n\r\n```cpp\r\nexecute(\"my_prog\", std_out>\"./log.txt\");\r\n```\r\n\r\nWith this syntax it is of course also easy to pipe from one process to another (though the implementation is not yet clear, i might need to use a named pipe on windows)\r\n\r\n```cpp\r\nprocess::pipe = create_pipe(); \r\nexecute(\"prog1\", std_out>pipe);\r\nexecute(\"prog2\", std_in<pipe);\r\n```\r\n\r\n### 2.3.3 Self-Sync I/O\r\n\r\n*Removed, because that makes child struct unecessary complicated*\r\n\r\nAnother possibility, that would definitly be possible would be to allow the process to adapt to a string. I don't know it this would actually be worth the effort.\r\n\r\n```cpp\r\nauto p = execute(\"c++filt\", std_in=self, std_out=self);\r\np << \"_ZN9wikipedia7article8wikilinkC1ERKSs\";\r\nstd::string dem;\r\np >> dem;\r\n//dem  == \"wikipedia::article::wikilink::wikilink(std::string const&)\"\r\n```\r\n\r\nThis is only convenience, it allows short-handed I/O. One could also use the stderr instead of stdout, while declaring both as self will yield an error.\r\n\r\n\r\n### 2.3.4 Async I/O\r\n\r\nThis might be the most difficult part. I imagine that we can do it in someway like this:\r\n\r\n```cpp\r\nboost::asio::io_service io_service;\r\nstringstream ss;//threadunsafe, but get's the point.\r\nstring buf; \r\n\r\nexecute(\"some_prog\", std_in<ss, std_out>buf, std_err>cerr, io_service);\r\nio_service.run();\r\n```\r\n\r\nThereby the whole mitigate thing can be hidden and managed automatically. Addtionally, if no io_service is passed, it could be created by the execute command and run it in another thread.\r\n\r\nThe [example](http://www.highscore.de/boost/process0.5/boost_process/tutorial.html#boost_process.tutorial.asynchronous_i_o) from the current tutorial would look like this:\r\n\r\n```cpp\r\nboost::asio::io_service io_service;\r\nstd::array<char, 4096> buffer;\r\n\r\nauto p = execute(\"test.exe\", std_out>buffer, io_service);\r\n\r\nio_service.run();\r\n```\r\n\r\n\r\nI can determine that buffer must be read async,because it's no stream and can construct the async_pipe in place. Also I can call `async_read` from the executor, because that does only start on `io_service.run()`.\r\nAdditionally, the io_service could be skipped and constructed in the executor and launched in a new thread. Don't know if that'd be smart though.\r\n\r\nBut to be honest: I am currently not nearly familiar enough with the way `io_service` works.\r\n\r\nThe same would apply to any `std::ostream` or `std::istream`, which is not `stream<file_descriptor_sink>`, so the `cerr ` or `ss` in my example should work this way. Now of course, binding `std_err>cerr` is by far the stupidest way, because I could pipe that directly (`std_err>stderr`) - at least if we provide a syntax for that (i.e. also redirecting `std_err>stdout`).\r\n\r\n\r\n__This whole concept needs more experimentation!__\r\n\r\n### 2.4 Environment\r\n\r\nIn 0.5 the `inherit_env` was empty for windows, I don't know if that actually works. Now I would use the following syntax:\r\n\r\n\r\n```cpp\r\nexecute(\"thingy\"); //inherit env\r\nexecute(\"thingy\", env+={\"MY_VAR=Thingy\"}); //inhertit and add\r\nexecute(\"things\", env=  {\"MY_VAR=Thingy\"}); //do not inherit\r\n```\r\n\r\nSo the following would be awesome, though I don't know if necessary: \r\n\r\n```cpp\r\n//inherit the environment and append \"MyThing\" to \"MY_VAR\"\r\nexecute(\"thingy\", env+={env_var[\"MY_VAR\"]+=\"MyThingy\"};\r\n```\r\n\r\n# 3. Classes\r\n## 3.1 Process\r\n\r\nWhen calling execute a process-object is returned, whichs type is undefined. It holds any data needed by the initialization. It has a set of functions, which are the following\r\n\r\n  - is_running\r\n  - terminate\r\n  - wait //formerly wait_for_exit\r\n  - wait_for   //wait with a timespan to wait\r\n  - wait_until //wait until a time timepoint\r\n  - exit_code\r\n\r\n## 3.2. Child\r\n\r\nIf the process shall be stored in a defined class, it can be converted to `child`. The data is then moved onto the stack and stored in a pointer inside the child. The child class will have member-functions similar to the functions for process.\r\nA child is movable, but not copyable.\r\n\r\n# 4. Launch Mode\r\n\r\nIn order to provide a process in an RAII way I do think that different launch modes should be available. I do not know as for now if this can be switched at runtime or has to be defined at launch time.\r\n\r\n#### Attached\r\n\r\nIf a childprocess runs attached it will be terminated on destructor-call.\r\n\r\n#### Detached\r\n\r\nIf a process is detached nothing will happend on destruction\r\n\r\n### Default\r\n\r\nBy default a process is launched in an attached way. This would cause this syntax to immediately terminate the application:\r\n\r\n```cpp\r\nexecute(\"prog\");\r\n```\r\n\r\nBut this behaviour could of course be specified by the following way:\r\n\r\n```cpp\r\nexecute(\"prog\", wait); //blocking\r\nexecute(\"prog\", detach); //not blocking.\r\n```\r\n\r\n# 5 this_process\r\n\r\nSimilar to `std::this_thread` I would add a `this_process` namespace with the following synopsis.\r\n\r\n```cpp\r\nnamespace this_process \r\n{\r\n    int get_id();\r\n    /*undefined*/ native_handle();\r\n    environment get_environment();\r\n}\r\n```"},"comment":{"url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/comments/196080894","html_url":"https://github.com/klemens-morgenstern/boost-process/issues/2#issuecomment-196080894","issue_url":"https://api.github.com/repos/klemens-morgenstern/boost-process/issues/2","id":196080894,"user":{"login":"klemens-morgenstern","id":8020376,"avatar_url":"https://avatars.githubusercontent.com/u/8020376?v=3","gravatar_id":"","url":"https://api.github.com/users/klemens-morgenstern","html_url":"https://github.com/klemens-morgenstern","followers_url":"https://api.github.com/users/klemens-morgenstern/followers","following_url":"https://api.github.com/users/klemens-morgenstern/following{/other_user}","gists_url":"https://api.github.com/users/klemens-morgenstern/gists{/gist_id}","starred_url":"https://api.github.com/users/klemens-morgenstern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/klemens-morgenstern/subscriptions","organizations_url":"https://api.github.com/users/klemens-morgenstern/orgs","repos_url":"https://api.github.com/users/klemens-morgenstern/repos","events_url":"https://api.github.com/users/klemens-morgenstern/events{/privacy}","received_events_url":"https://api.github.com/users/klemens-morgenstern/received_events","type":"User","site_admin":false},"created_at":"2016-03-13T23:28:06Z","updated_at":"2016-03-13T23:28:06Z","body":"### Status update\r\n\r\nReworking the whole MPL stuff with Hana took a bit more time than expected, though I have to say: hana is awesome.\r\n\r\nI think the basic structure is now alright, though it'll take a lot of time, to now implement the actual initializers.\r\n\r\nCurrently you can launch a process from a string and handle the error (throw, assign, ignore). Throw on default.\r\n\r\nI decided to remove the self-pipe feature, since it overcomplicates the child for a small convenience.\r\n\r\nIf someone is interested you might look at the following files (and yes, it's till messy)\r\n\r\n  - test/run_exe.cpp\r\n  - include/boost/process/execute.hpp\r\n  - include/boost/process/windows/executor.hpp\r\n  - include/boost/process/cmd.hpp\r\n  - include/boost/process/error.hpp"}}