-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55239/#review161016
-----------------------------------------------------------
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?
src/launcher/fetcher.cpp (line 67)
<https://reviews.apache.org/r/55239/#comment232335>
`stringify` on `vector` puts `", "` between items. We can use
`strings::join()`.
src/launcher/fetcher.cpp (line 85)
<https://reviews.apache.org/r/55239/#comment232293>
This is the default so unnecessary?
src/launcher/fetcher.cpp (line 95)
<https://reviews.apache.org/r/55239/#comment232295>
`stringify` on `vector` puts `", "` between items.
- Jiang Yan Xu
On Jan. 5, 2017, 5:13 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55239/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2017, 5:13 p.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
>
>
