You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/01/06 01:18:16 UTC

Review Request 55242: Stop using os::system to chown a directory hierarchy.

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

Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Bugs: MESOS-6862
    https://issues.apache.org/jira/browse/MESOS-6862


Repository: mesos


Description
-------

Stop using os::system to chown a directory hierarchy.


Diffs
-----

  3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 

Diff: https://reviews.apache.org/r/55242/diff/


Testing
-------

`sudo make check` (Fedora 25)


Thanks,

James Peach


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review161199
-----------------------------------------------------------



If we implement the recursive chown this way, I feel that we need some unit tests, I don't think higher-level tests have enough coverage for this method. If we replace it with `os::spawn()` it's more trivial that I think we can skip the test for now. What do you prefer?

- Jiang Yan Xu


On Jan. 5, 2017, 5:18 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Stop using os::system to chown a directory hierarchy.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review161344
-----------------------------------------------------------



Patch looks great!

Reviews applied: [55238, 55239, 55240, 55241, 55242]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 12, 2017, 12:29 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by James Peach <jp...@apache.org>.

> On Jan. 13, 2017, 8:27 a.m., Jiang Yan Xu wrote:
> > 3rdparty/stout/tests/os_tests.cpp, line 751
> > <https://reviews.apache.org/r/55242/diff/2/?file=1603253#file1603253line751>
> >
> >     s/tree/subtree/?

This commend looks correct to me.


- James


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


On Jan. 13, 2017, 11:12 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by James Peach <jp...@apache.org>.

> On Jan. 13, 2017, 8:27 a.m., Jiang Yan Xu wrote:
> > 3rdparty/stout/tests/os_tests.cpp, line 751
> > <https://reviews.apache.org/r/55242/diff/2/?file=1603253#file1603253line751>
> >
> >     s/tree/subtree/?

This commend looks correct to me.


- James


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


On Jan. 13, 2017, 11:12 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review161413
-----------------------------------------------------------




3rdparty/stout/include/stout/os/posix/chown.hpp (lines 36 - 42)
<https://reviews.apache.org/r/55242/#comment232712>

    We are handling the situation where we can't `stat` the root. Is it a file/dir permission concern? 
    
    What if we can't `stat` the subdirs and files? 
    
    I seems to me that for the two things we are doing:
    
    1) Only descend if the root is a directory and `recursive` is true: 
    
    If we just put a 
    ```
    if (!recursive && node->fts_level != FTS_ROOTLEVEL) {
      break;
    }
    ```
    
    inside the while loop so we only need to call `lchown` from one place.
    
    2) Dealing with issues with `stat` (and other errros)
    
    We need to handle then inside the loop too. Even though `chown` requires a "privledged" user (unless if you chown to yourself I think). I guess we shouldn't assume this while traversing the tree and should let the system call fail themselves at appropriate places.



3rdparty/stout/include/stout/os/posix/chown.hpp (line 43)
<https://reviews.apache.org/r/55242/#comment232693>

    s/char */char*/
    
    s/paths_/paths/ since there's no `paths`?



3rdparty/stout/include/stout/os/posix/chown.hpp (lines 52 - 53)
<https://reviews.apache.org/r/55242/#comment232694>

    `node` should align with `FTSENT` and we should put each expression on a new line.
    
    However the following calls `node = fts_read(tree)` from just one place which I think is nicer?
    
    ```
    FTSENT* node;
    while ((node = ::fts_read(tree)) != nullptr) {
    
    }
    ```



3rdparty/stout/include/stout/os/posix/chown.hpp (lines 54 - 56)
<https://reviews.apache.org/r/55242/#comment232695>

    Will the inability to `stat` the file result in `FTS_NS`? Do we not need to handle other fts errors?
    
    If we don't, our `if` condition is going to silently ignore these files and report success.



3rdparty/stout/tests/os_tests.cpp (line 728)
<https://reviews.apache.org/r/55242/#comment232789>

    Unfortunately it's a bit tricky to test the permission stuff I guess.



3rdparty/stout/tests/os_tests.cpp (line 746)
<https://reviews.apache.org/r/55242/#comment232783>

    s/link/symlink/



3rdparty/stout/tests/os_tests.cpp (line 751)
<https://reviews.apache.org/r/55242/#comment232786>

    s/tree/subtree/?



3rdparty/stout/tests/os_tests.cpp (line 752)
<https://reviews.apache.org/r/55242/#comment232787>

    Comment that `9` is just an arbitrary uid that we picked?


- Jiang Yan Xu


On Jan. 11, 2017, 4:29 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review161659
-----------------------------------------------------------


Fix it, then Ship it!




Just some style nits.


3rdparty/stout/include/stout/os/posix/chown.hpp (line 48)
<https://reviews.apache.org/r/55242/#comment232929>

    We rarely use C-style comments unless there's code following it on the same line.
    
    We also use complete sentences (capitalization and punctuations, etc.) even though these are incomplete sentences.
    
    e.g.,
    
    ```
    // Preorder directory.
    ```
    
    Here and below.



3rdparty/stout/include/stout/os/posix/chown.hpp (line 55)
<https://reviews.apache.org/r/55242/#comment232932>

    When cases blocks are are not single line, use `{}` to enclose them.



3rdparty/stout/include/stout/os/posix/chown.hpp (line 74)
<https://reviews.apache.org/r/55242/#comment232931>

    Use a `default` statement to be explict about ignoring other values (we are not capturing all possible fts cases and we are switching on an integer): https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements
    
    ```
    default:
      break
    ```



3rdparty/stout/tests/os_tests.cpp (lines 753 - 755)
<https://reviews.apache.org/r/55242/#comment232928>

    About the previous comment: s/tree/subtree/
    
    "the top of the tree" is `chown` right? Here you are making a symlink "two.link" to "chown/one/two", which is at most the top of a subtree. Of course the subtree is still a "tree" but "subtree" is at least bit more clear?


- Jiang Yan Xu


On Jan. 13, 2017, 3:12 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 3:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review162009
-----------------------------------------------------------



Patch looks great!

Reviews applied: [55238, 55239, 55240, 55241, 55242]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos ReviewBot


On Jan. 17, 2017, 10:06 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 10:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/
-----------------------------------------------------------

(Updated Jan. 17, 2017, 10:06 p.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
-------

Review.


Bugs: MESOS-6862
    https://issues.apache.org/jira/browse/MESOS-6862


Repository: mesos


Description
-------

Reimplement os::chown() to use fts(3) rather than sometimes spawning
chown(1). This removes the use of the shell and the corresponding
need to sanitize path arguments.  It also enables us to provide
consistent handling of symbolic links for the recursive and
non-recursive cases. We ensure that symlinks are never followed
and that we always change the ownership of the link itself, not
its referent.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
  3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
  3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 

Diff: https://reviews.apache.org/r/55242/diff/


Testing
-------

`sudo make check` (Fedora 25)


Thanks,

James Peach


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review161636
-----------------------------------------------------------



Bad patch!

Reviews applied: [55238, 55239, 55240, 55241, 55242]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh 2>&1 | tee build_55242"]

Error:
bash: ./support/docker_build.sh: No such file or directory

Full log: https://builds.apache.org/job/Mesos-Reviewbot/16720/console

- Mesos ReviewBot


On Jan. 13, 2017, 11:12 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Reimplement os::chown() to use fts(3) rather than sometimes spawning
> chown(1). This removes the use of the shell and the corresponding
> need to sanitize path arguments.  It also enables us to provide
> consistent handling of symbolic links for the recursive and
> non-recursive cases. We ensure that symlinks are never followed
> and that we always change the ownership of the link itself, not
> its referent.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
>   3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/
-----------------------------------------------------------

(Updated Jan. 13, 2017, 11:12 p.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
-------

Rebased.


Bugs: MESOS-6862
    https://issues.apache.org/jira/browse/MESOS-6862


Repository: mesos


Description
-------

Reimplement os::chown() to use fts(3) rather than sometimes spawning
chown(1). This removes the use of the shell and the corresponding
need to sanitize path arguments.  It also enables us to provide
consistent handling of symbolic links for the recursive and
non-recursive cases. We ensure that symlinks are never followed
and that we always change the ownership of the link itself, not
its referent.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
  3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
  3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 

Diff: https://reviews.apache.org/r/55242/diff/


Testing
-------

`sudo make check` (Fedora 25)


Thanks,

James Peach


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/
-----------------------------------------------------------

(Updated Jan. 12, 2017, 12:29 a.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
-------

Rebase and address review feedback.


Bugs: MESOS-6862
    https://issues.apache.org/jira/browse/MESOS-6862


Repository: mesos


Description (updated)
-------

Reimplement os::chown() to use fts(3) rather than sometimes spawning
chown(1). This removes the use of the shell and the corresponding
need to sanitize path arguments.  It also enables us to provide
consistent handling of symbolic links for the recursive and
non-recursive cases. We ensure that symlinks are never followed
and that we always change the ownership of the link itself, not
its referent.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
  3rdparty/stout/include/stout/os/posix/stat.hpp 1ab20e75fc18b336162b62e2f4f23b68f6685183 
  3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 

Diff: https://reviews.apache.org/r/55242/diff/


Testing
-------

`sudo make check` (Fedora 25)


Thanks,

James Peach


Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55242/#review160699
-----------------------------------------------------------



Patch looks great!

Reviews applied: [55238, 55239, 55240, 55241, 55242]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 6, 2017, 1:18 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55242/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 1:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6862
>     https://issues.apache.org/jira/browse/MESOS-6862
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Stop using os::system to chown a directory hierarchy.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/posix/chown.hpp c82e2e574019c5ee5f17ea105a6d225006388a45 
> 
> Diff: https://reviews.apache.org/r/55242/diff/
> 
> 
> Testing
> -------
> 
> `sudo make check` (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>