> On Jan. 11, 2017, 9:14 a.m., Jiang Yan Xu wrote:
> > I don't feel that we need to introduce these wrappers, which are either
> > single-use or don't provide a better abstration than the counterpart in
> > stout.
> >
> > Can we uniformly use `subprocess` in all cases (since `os::spawn()` doesn't
> > work in all cases) for consistency?
> >
> > To reuse code, we can do this
> >
> > ```
> > vector<string> command;
> >
> > // Potentially need to specify destination via stdout.
> > Option<Subprocess::IO> out = None();
> >
> > // if tar.
> > command = {"tar", "-C", destinationDirectory, "-xf", sourcePath};
> > // else if gzip.
> > command = {"gzip", "-dc", sourcePath};
> > out = Subprocess::PATH(destinationPath);
> > // else if zip.
> > command = {"unzip", "-o", "-d", destinationDirectory, sourcePath};
> > // else
> >
> > ...
> >
> > // Comment that we use the argv of subprocess to avoid injection.
> > Try<Subprocess> s = subprocess(
> > command[0],
> > command,
> > Subprocess::FD(STDIN_FILENO),
> > out.getOrElse(Subprocess::FD(STDOUT_FILENO)));
> > ```
> >
> > What do you think?
We could subprocess everything. Would need to think about how to format the
command message legibly.
> On Jan. 11, 2017, 9:14 a.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 85
> > <https://reviews.apache.org/r/55239/diff/1/?file=1598134#file1598134line85>
> >
> > This is the default so unnecessary?
However it makes it obvious to the reader, so it should be kept.
> On Jan. 11, 2017, 9:14 a.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 95
> > <https://reviews.apache.org/r/55239/diff/1/?file=1598134#file1598134line95>
> >
> > `stringify` on `vector` puts `", "` between items.
Yes, that's desirable so that you can see the whitespace in the command.
> On Jan. 11, 2017, 9:14 a.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 67
> > <https://reviews.apache.org/r/55239/diff/1/?file=1598134#file1598134line67>
> >
> > `stringify` on `vector` puts `", "` between items. We can use
> > `strings::join()`.
This is intended since it makes the actual arguments much clearer and more
obvious.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55239/#review161016
-----------------------------------------------------------
On Jan. 6, 2017, 1:13 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55239/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2017, 1:13 a.m.)
>
>
> Review request for mesos, Jie Yu and Jiang Yan Xu.
>
>
> Bugs: MESOS-6862
> https://issues.apache.org/jira/browse/MESOS-6862
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Stop using os::system to extract fetcher archives.
>
>
> Diffs
> -----
>
> src/launcher/fetcher.cpp 4456c28139966e42859cc6d2c79a1d90e83fb22f
>
> Diff: https://reviews.apache.org/r/55239/diff/
>
>
> Testing
> -------
>
> `sudo make check` (Fedora 25)
>
>
> Thanks,
>
> James Peach
>
>
