----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66002/#review199206 -----------------------------------------------------------
Ship it! I think this approach is reasonable. `files` is the "public" API where we're getting (essentially) user input (i.e. things from frameworks and other external code beyond our control). This API had previously always expected `/` as the path separator, I think this is the correct place to "sanitize user input" and convert to the OS-specific path separator. This way, our OS layer (the file functions in stout) are left OS-specific, but the public API will work as expected. - Andrew Schwartzmeyer On March 9, 2018, 12:17 p.m., John Kordich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66002/ > ----------------------------------------------------------- > > (Updated March 9, 2018, 12:17 p.m.) > > > Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, > and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Many mesos frameworks assume that path separators are forward slashes, > like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to > this patch, if a forward slash was given in the path variable to an HTTP > API function call to the mesos agent on a Windows system, the Windows > API would fail at recognizing the path, because the Windows API accepts > only backslashes as path separators. To remedy this issue, we now > convert all forward slashes passed as a path to the HTTP API to an agent > to back slashes on Windows agents by using the path::from_uri function. > > > Diffs > ----- > > src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 > src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 > > > Diff: https://reviews.apache.org/r/66002/diff/2/ > > > Testing > ------- > > Tested on both Windows and Linux, all tests pass, including the two newly > enabled tests on Windows. > > > Thanks, > > John Kordich > >
