Ok great, this is what I had in mind. The two main issues: 1. proc.hpp includes os.hpp, so making os.hpp include proc.hpp introduces a circular dependency, no?
2. As you mentioned, ProcessStatus is linux specific for the most part. We'll either need to make it generic, or #ifdef it accordingly. What did you mean by implement things like children differently? On Thu, Apr 25, 2013 at 1:53 PM, Benjamin Hindman < [email protected]> wrote: > I think we want "everything" (pids, children, alive, etc.) in os:: with > implementations that use the proc:: functions for Linux and do other things > for Mac OS X. As in, os::pids can be implemented for Linux as a call to > proc::pids, while the implementation for Mac OS X can be whatever else it > needs to be. The trick here is that ProcessStatus is a pretty specific > Linux abstraction, so either we need to make a generic one or we need to > implement things like children differently. > > > On Thu, Apr 25, 2013 at 1:41 PM, Benjamin Mahler < > [email protected]> wrote: > >> Ok so linux/proc.hpp currently has: >> >> Try<std::set<pid_t> > pids(); >> Try<std::set<pid_t> > children(pid_t pid, bool recursive = true); >> Try<ProcessStatus> status(pid_t pid); >> >> We want these to work for OSX as well for now. >> >> Say we move linux/proc.{cpp,hpp} into stout/proc.hpp, then we can move >> alive into os and fail the compilation if anyone includes stout/proc.hpp >> without __linux__ defined. >> >> We'll also want os::pids(), os::children(pid_t) and os::status(pid_t) for >> non-linux systems. I originally wanted to have these call into >> stout/proc.hpp for linux, but that introduces a circular dependency. >> >> >> On Thu, Apr 25, 2013 at 11:51 AM, Benjamin Hindman < >> [email protected]> wrote: >> >>> I like os:: a lot. I think the type signature (i.e., taking a pid_t) is >>> sufficient for disambiguation. >>> >>> >>> >>> >>> >>> On Apr 25, 2013, at 11:43 AM, Benjamin Mahler <[email protected]> >>> wrote: >>> >>> We also have a src/common/process_utils.hpp which contains only >>> mesos::internal::utils::process::killtree() at the moment. >>> >>> >>> On Thu, Apr 25, 2013 at 11:37 AM, Yan Xu <[email protected]> wrote: >>> >>>> I guess os:: is fine, but in a separate file? >>>> >>>> -- >>>> Jiang Yan Xu <[email protected]> @xujyan <http://twitter.com/xujyan> >>>> >>>> >>>> On Thu, Apr 25, 2013 at 11:29 AM, Vinod Kone <[email protected]> wrote: >>>> >>>>> I don't like process:: because it conflicts with the libprocess >>>>> namespace as you mentioned. >>>>> >>>>> I still like proc:: but clearly BenH doesn't like it. I'm ok with os:: >>>>> namespace. >>>>> >>>>> >>>>> @vinodkone >>>>> >>>>> >>>>> On Thu, Apr 25, 2013 at 11:19 AM, Benjamin Mahler < >>>>> [email protected]> wrote: >>>>> >>>>>> Is there any consensus on how to place process utilities in stout? I >>>>>> would expect this to be in a process:: namespace but of course that is >>>>>> confusing because we use libprocess, which should perhaps have a >>>>>> libprocess:: namespace instead.. >>>>>> >>>>>> I'll be moving process utilities etc into stout, hopefully with the >>>>>> same calls for linux and OSX but I'm not yet certain if that is >>>>>> possible. I >>>>>> would like to place these in a process.hpp file inside a process:: >>>>>> namespace. >>>>>> >>>>>> I think these read very nicely: >>>>>> process::alive(pid_t) >>>>>> process::children(pid_t) >>>>>> process::stat(pid_t) >>>>>> >>>>>> Thoughts? >>>>>> >>>>>> >>>>>> On Tue, Apr 23, 2013 at 6:29 PM, Yan Xu <[email protected]> wrote: >>>>>> >>>>>>> This batch of commits changed the reaper to use "Future" as the >>>>>>> notification mechanism. >>>>>>> >>>>>>> Sequence: >>>>>>> https://reviews.apache.org/r/10744/ >>>>>>> https://reviews.apache.org/r/10745/ >>>>>>> https://reviews.apache.org/r/10746/ >>>>>>> https://reviews.apache.org/r/10747/ >>>>>>> >>>>>>> Best, >>>>>>> Yan >>>>>>> -- >>>>>>> Jiang Yan Xu <[email protected]> @xujyan <http://twitter.com/xujyan> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >
