> On March 24, 2017, 10:18 p.m., Li Li wrote: > > 3rdparty/stout/include/stout/os/windows/stat.hpp > > Lines 66 (patched) > > <https://reviews.apache.org/r/57926/diff/1/?file=1674588#file1674588line66> > > > > At this case, should true or false be returned?
In POSIX systems, a symlink is traditionally a file semantically. On Windows, it is implemented as a reparse point instead of a file, but this MSDN documentation https://msdn.microsoft.com/en-us/library/windows/desktop/aa365680(v=vs.85).aspx states: > Symbolic links are designed to aid in migration and application compatibility > with UNIX operating systems. Microsoft has implemented its symbolic links to > function just like UNIX links. So I think it makes sense to maintain the POSIX semantics and state that a symlink is a file (i.e. return true). That said, I double checked our POSIX implementation, and `isdir` nor `isfile` respectively compare `S_IFDIR` and `S_IFREG`, so they would both return false (just tested `S_IFREG` as I wasn't sure). I don't _think_ this is correct semantically, unless we intend `isfile` to mean "is regular file" (i.e. explicitly not a link). I might have just talked myself into returning false here, but would appreciate further input. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57926/#review170059 ----------------------------------------------------------- On March 24, 2017, 10:14 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57926/ > ----------------------------------------------------------- > > (Updated March 24, 2017, 10:14 p.m.) > > > Review request for mesos, John Kordich, James Peach, Joseph Wu, Li Li, and > Michael Park. > > > Bugs: MESOS-7307 > https://issues.apache.org/jira/browse/MESOS-7307 > > > Repository: mesos > > > Description > ------- > > Commit 5f159cdcb introduced `return Error(...)` logic to functions > which return `bool`, not `Try<bool>`, which broke the Windows build. > > Furthermore, in the instances of `isdir` and `isfile`, erroring when > asked to not follow a symlink is not correct. The semantics of symlinks > provide clear answers to `isdir` and `isfile` when the target is a link, > and is not being followed (it is a file, but not a directory). > > For the functions `mode` and `dev`, which return types wrapped by `Try`, > we should only error if asked not to follow symlinks, and the target is > actually a symlink. If it is not a symlink to begin with, we should not > prematurely error. If it is a symlink, we should error because there is > no equivalent of `lstat` on Windows to obtain `st_mode` or `st_dev` of a > symlink itself. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/windows/stat.hpp > 8587341282ca2d596a2b6f23f84b84a00053c3d5 > > > Diff: https://reviews.apache.org/r/57926/diff/1/ > > > Testing > ------- > > Build on Windows and run stout-tests.exe > > > Thanks, > > Andrew Schwartzmeyer > >
