----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50673/#review144547 -----------------------------------------------------------
3rdparty/stout/tests/os/rmdir_tests.cpp (line 105) <https://reviews.apache.org/r/50673/#comment210564> Per the next comment (below this one), this line would become: ``` EXPECT_EQ(hashset<string>::EMPTY, listfiles(tmpdir)); ``` 3rdparty/stout/tests/os/rmdir_tests.cpp (lines 116 - 117) <https://reviews.apache.org/r/50673/#comment210562> Since these two sets are only modified once within the test, why not do: ``` const hashset<string> expectedRootListing = { newDirectoryName }; const hashset<string> expectedSubListing = { newFileName }; ``` I realize this is the pattern within the other tests in this file. So perhaps you can change the pattern in a subsequent review :) - Joseph Wu On Aug. 1, 2016, 2:24 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50673/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2016, 2:24 p.m.) > > > Review request for mesos, Daniel Pravat and Joseph Wu. > > > Bugs: MESOS-5942 > https://issues.apache.org/jira/browse/MESOS-5942 > > > Repository: mesos > > > Description > ------- > > This commit will fix 2 known bugs in the Windows implementation of > `os::rmdir`, as chronicled in MESOS-5942, namely: > > 1. Calling `os::rmdir` with a file argument (rather than a directory) > and the `recursive` parameter set to `true` will fail on Windows, > but succeed on POSIX. > > The POSIX semantics of `os::rmdir` are a union of `rm -r` and `::rmdir`, > the behavior of which depends on the arguments the caller passes in. If > the formal parameter `recursive` is set to `true`, then the semantics > are meant to be`rm -r`; if `false`, the semantics are meant to be > `::rmdir`. > > The implications of this are somewhat subtle: `::rmdir` will error out > if you try to delete (e.g.) regular files, while `rm -r` will happily > delete them. > > On Windows, we currently always have `::rmdir`-style semantics, in that > we if you pass a path that points at a file to `os::rmdir`, it will not > delete that file. > > This commit will reverse this, and move make the Windows implementation > semantically identical to the POSIX implementation (at least in this > regard). > > 2. Recursively deleting nested directories fails on windows when > `removeRoot` is set to `false`. > > Currently if you set the `removeRoot` parameter to `false`, the Windows > implementation of `os::rmdir` will fail to delete a directory inside a > directory. The reason is that we are propagating the `removeRoot` flag > to the recursive calls to `os::rmdir`. The implication of this is that > the recursive call will *not* delete the nested directory (since > `removeRoot` is `false`). > > This commit will fix this by setting `removeRoot` to `true` in recursive > calls to `os::rmdir`. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/windows/rmdir.hpp > b74bf7153a15435ce424880df84901c349dee216 > 3rdparty/stout/tests/os/rmdir_tests.cpp > ffe234baac305e26b5a29cffcdd310350d10167e > > Diff: https://reviews.apache.org/r/50673/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
