mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anindya Sinha <anindya_si...@apple.com>
Subject Re: Review Request 52980: Updated os::mkdir() to take the mode as an input.
Date Tue, 15 Nov 2016 18:25:56 GMT


> On Oct. 18, 2016, 7:58 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/os/mkdir.hpp, line 43
> > <https://reviews.apache.org/r/52980/diff/1/?file=1540377#file1540377line43>
> >
> >     Permissions is a bit of a subtle issue in Mesos, which we've largely punted
on. I'm a bit worried that adding a pluggable `mode` here will cause us to add code that only
works on Unix. How do you see this API evolving to accommodate both?
> 
> Alex Clemmer wrote:
>     Also, sorry, I should have been more clear: I mean that permissions _on Windows_
is a subtle issue in Mesos. My worry is rooted in the fact that this API also has to support
Windows.
> 
> Anindya Sinha wrote:
>     So, er either extend `mkdir()` to include a `mode` (which as you indicated would
need a change in the windows api also), OR call `chmod()` recursively after the call to the
existing `mkdir()` [ie. without `mode`]. Currently, linux implementation in stout does not
have a recursive `chmod()` which needs to be added, and I think windows just punts on `chmod()`
calls.
> 
> Anindya Sinha wrote:
>     s/er/we. So to restate my comments once again:
>     
>     So, we either extend `mkdir()` to include a `mode` (which as you indicated would
need a change in the windows api also), OR call `chmod()` recursively after the call to the
existing `mkdir()` [ie. without `mode`]. Currently, linux implementation in stout does not
have a recursive `chmod()` which needs to be added, and I think windows just punts on chmod()
calls.
> 
> Greg Mann wrote:
>     Anindya, do you see a downside to the recursive `chmod` approach? Going with that
would allow us to avoid adding the posix-specific `mode` to another function signature.
>     
>     Alex, do you have any thoughts at this time on how file permissions support for Windows
might evolve? If we think that we'll end up defining a set of generic file permission states
that can be converted to either posix-style or Windows-style permissions, then perhaps adding
the `mode` parameter here isn't such a big deal; we could simply convert it to some generic
`Mode` type that we introduce later on.

No downside functionally to use a recursive `chmod()` over a mode-specific `mkdir()`, but
we would need to add support for a recursive `chmod()` in that case also the current `chmod()`
does not handle recursive processing. So, that would be a change to the existing api nevertheless.
Also, `chmod()` for windows is also punted currently.

So, I am not sure if one approach is better than the other.


- Anindya


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


On Oct. 18, 2016, 7:43 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52980/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
>     https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mode still defaults to 755, but we now enable the caller to specify
> a different mode for creation of directories.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/mkdir.hpp fe86864c8b480993c8f052f39b2fd3ece23798da

> 
> Diff: https://reviews.apache.org/r/52980/diff/
> 
> 
> Testing
> -------
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message