----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54415/#review158163 -----------------------------------------------------------
Patch looks great! Reviews applied: [54324, 53550, 53551, 53552, 54395, 54415] 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 Dec. 6, 2016, 11:29 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54415/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2016, 11:29 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu. > > > Bugs: MESOS-6697 > https://issues.apache.org/jira/browse/MESOS-6697 > > > Repository: mesos > > > Description > ------- > > Currently there are two bugs in the Windows implementation of > `os::mkdtemp`, which together cause all agent tests to fail. We describe > these bugs below and explain the steps this commit will take to address > them. > > The first concerns a mismatch between the POSIX specification of > `mkdtemp` and the Windows implementation of `os::mkdtemp`. Specifically, > the POSIX specification indicates that, if any component of the path > prefix in the directory pattern passed to `mkdtemp` does not exist, we > are to set `errno` to `ENOENT` and return a null pointer. In the Stout > POSIX wrapper, `os::mkdtemp`, we will return an `ErrnoError`. In > contrast, the Windows implementation will erroneously recursively create > all components of the path prefix. This commit will address this by > explicitly setting the `recursive` flag of the call to `os::mkdir` to > `false`. > > The second concerns a failure to parse Unix-like paths when creating the > temporary directory. Specifically, both the POSIX and Windows > implementations of `os::mkdtemp` have a default argument: `/tmp/XXXXXX`. > When we replace the 'X' characters with random alphanumeric characters > and pass the result to `os::mkdir`, and when the `recursive` flag is > errorneously set to `true`, we will fail to parse the path on Windows, > because `os::mkdir` can only pass Windows-style paths, with the '\' > character. This causes all Agent tests to fail, e.g., when we try to > create sandbox directories. > > Technically, this second issue is actually resolved by setting the > `recursive` flag to `false` in the call to `os::mkdir`, but here we > observe that `/tmp` is not the "right place" to put temporary files on > Windows anyway, and so we take the time to clean it up here, by > replacing the default string literal path `/tmp/XXXXXX` with the > platform-agnostic `path::join(os::temp(), "XXXXXX)`. > > > Diffs > ----- > > 3rdparty/stout/include/Makefile.am d5bc728ce378586e84173b98499b0d52047a83e1 > 3rdparty/stout/include/stout/os.hpp > bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba > 3rdparty/stout/include/stout/os/posix/mkdtemp.hpp > 90866dba8e6be206c64f21204d936c1bed808c9a > 3rdparty/stout/include/stout/os/posix/temp.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/temp.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/windows/mkdtemp.hpp > 2cb73bbb996775e3764ad852ccda5076b41aef41 > 3rdparty/stout/include/stout/os/windows/temp.hpp PRE-CREATION > 3rdparty/stout/include/stout/posix/os.hpp > 1f4d155e4399b955d0ca884f5336c78d8ceb56b5 > 3rdparty/stout/include/stout/windows/os.hpp > de9b04ad82443038a0f4408bc72cae1540a1beaf > > Diff: https://reviews.apache.org/r/54415/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
