> On Aug. 31, 2015, 5:50 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/backends/copy.cpp, line 19
> > <https://reviews.apache.org/r/37921/diff/1/?file=1059333#file1059333line19>
> >
> >     Move this below.
> >     
> >     ```
> >     #include "common/status_utils.hpp"
> >     
> >     #include "slave/containerizer/provisioners/backends/copy.hpp"
> >     ```

I think our newest style guide points that we include the cpp's main header 
file on the top, but I'm fine moving this down.


> On Aug. 31, 2015, 5:50 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/backends/copy.cpp, lines 127-132
> > <https://reviews.apache.org/r/37921/diff/1/?file=1059333#file1059333line127>
> >
> >     Using shell expansion here has others issues:
> >     
> >     1) Spaces (or other $IFS) in the paths
> >     2) Not sure if filename expansions works the same way across shells.
> >     
> >     GNU `cp` has a `--no-target-directory` flag that's handy in this 
> > situation but Mac `cp` doesn't have it.
> >     
> >     So I think we should probably just rely on a precondition check: return 
> > Failure if os::exists(rootfs) already exists and remove the mkdir in 
> > `provision()`. This allows us to just use the *argv* form of `subprocess` 
> > with `{"cp", "-a", layer, rootfs}`.
> >     
> >     AppcProvisioner already expects the Backend to create the `rootfs` and 
> > DockerProvisioner can do the same. 
> >     
> >     We can emphaize this by adding a note to `Backend::provision()` 
> > documentation.
> >     
> >     Thoughts?

Ok as long we can make sure provisioners output the same layout I definitely 
prefer just doing a straight copy!
I'll make sure the basename of all layers are the same.


- Timothy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37921/#review97009
-----------------------------------------------------------


On Aug. 29, 2015, 7:59 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37921/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2015, 7:59 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add Copy backend for provisioners.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioners/backend.cpp 
> 2f7c335f62fdeb27526ab9a38a07c097422ae92b 
>   src/slave/containerizer/provisioners/backends/copy.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/copy.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d321850613223a2357ca1646a9d988d05171772c 
> 
> Diff: https://reviews.apache.org/r/37921/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Reply via email to