You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/11/30 03:28:20 UTC
[01/13] mesos git commit: Windows: Fixed `fs::symlink` to support
relative and broken paths.
Repository: mesos
Updated Branches:
refs/heads/master 24550e7b8 -> 5dd3b2900
Windows: Fixed `fs::symlink` to support relative and broken paths.
This resolves MESOS-5881 and simplifies `create_symbolic_link()`. Links
to non-existent targets are allowed on Windows. Relative paths for links
and targets are also valid.
This enables two tests which had been disabled due to the previous
implementation not allowing links to non-existent files. The `OsTest`
uncovered more uses of CRT APIs in the Windows `stat` implementation.
Review: https://reviews.apache.org/r/64180
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0f544edb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0f544edb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0f544edb
Branch: refs/heads/master
Commit: 0f544edb0aef4281a27244138faf7bce904ebaaa
Parents: e1d7c6d
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 29 13:04:14 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:10 2017 -0800
----------------------------------------------------------------------
.../stout/internal/windows/reparsepoint.hpp | 55 ++++----------------
.../stout/include/stout/os/windows/stat.hpp | 6 +++
3rdparty/stout/tests/os/rmdir_tests.cpp | 5 +-
3rdparty/stout/tests/os_tests.cpp | 5 +-
4 files changed, 18 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0f544edb/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
index 0ff5d68..fff2cbb 100644
--- a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
@@ -20,7 +20,6 @@
#include <stout/windows.hpp>
#include <stout/os/mkdir.hpp>
-#include <stout/os/realpath.hpp>
#include <stout/internal/windows/attributes.hpp>
#include <stout/internal/windows/longpath.hpp>
@@ -263,52 +262,20 @@ inline Try<Nothing> create_symbolic_link(
const std::string& target,
const std::string& reparse_point)
{
- // Normalize input paths.
- const Result<std::string> real_reparse_point_path =
- os::realpath(reparse_point);
-
- const Result<std::string> real_target_path = os::realpath(target);
-
- if (!real_reparse_point_path.isSome()) {
- return Error(
- "Failed to get realpath for '" +
- reparse_point + "': " + (real_reparse_point_path.isError() ?
- real_reparse_point_path.error() : "No such directory"));
- }
-
- if (!real_target_path.isSome()) {
- return Error(
- "Failed to get realpath for '" +
- target + "': " + (real_target_path.isError() ?
- real_target_path.error() : "No such directory"));
- }
-
- const std::string& absolute_target_path = real_target_path.get();
-
// Determine if target is a folder or a file. This makes a difference
// in the way we call `create_symbolic_link`.
- const Try<DWORD> attributes = get_file_attributes(
- longpath(absolute_target_path));
+ const Try<DWORD> attributes = get_file_attributes(longpath(target));
- if (attributes.isError()) {
- return Error(attributes.error());
+ bool target_is_folder = false;
+ if (attributes.isSome()) {
+ target_is_folder = attributes.get() & FILE_ATTRIBUTE_DIRECTORY;
}
- const bool target_is_folder = attributes.get() & FILE_ATTRIBUTE_DIRECTORY;
-
// Bail out if target is already a reparse point.
- Try<bool> attribute_set = reparse_point_attribute_set(
- longpath(absolute_target_path));
-
- if (!attribute_set.isSome()) {
+ Try<bool> attribute_set = reparse_point_attribute_set(longpath(target));
+ if (attribute_set.isSome() && attribute_set.get()) {
return Error(
- "Could not get reparse point attribute for '" + absolute_target_path +
- "'" + (attribute_set.isError() ? ": " + attribute_set.error() : ""));
- }
-
- if (attribute_set.get()) {
- return Error(
- "Path '" + absolute_target_path + "' is already a reparse point");
+ "Path '" + target + "' is already a reparse point");
}
// Avoid requiring administrative privileges to create symbolic links.
@@ -322,13 +289,11 @@ inline Try<Nothing> create_symbolic_link(
// above flag.
if (!::CreateSymbolicLinkW(
// Path to link.
- longpath(real_reparse_point_path.get()).data(),
+ longpath(reparse_point).data(),
// Path to target.
- longpath(real_target_path.get()).data(),
+ longpath(target).data(),
flags)) {
- return WindowsError(
- "'internal::windows::create_symbolic_link': 'CreateSymbolicLink' "
- "call failed");
+ return WindowsError();
}
return Nothing();
http://git-wip-us.apache.org/repos/asf/mesos/blob/0f544edb/3rdparty/stout/include/stout/os/windows/stat.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/stat.hpp b/3rdparty/stout/include/stout/os/windows/stat.hpp
index 0db6d48..2ba6969 100644
--- a/3rdparty/stout/include/stout/os/windows/stat.hpp
+++ b/3rdparty/stout/include/stout/os/windows/stat.hpp
@@ -97,6 +97,8 @@ inline bool islink(const std::string& path)
// applied to a symbolic link with `follow` set to
// `DO_NOT_FOLLOW_SYMLINK`, this will return the length of the entry
// name (strlen).
+//
+// TODO(andschwa): Replace `::_stat`. See MESOS-8275.
inline Try<Bytes> size(
const std::string& path,
const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK)
@@ -129,6 +131,7 @@ inline Try<Bytes> size(
}
+// TODO(andschwa): Replace `::_stat`. See MESOS-8275.
inline Try<long> mtime(
const std::string& path,
const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK)
@@ -161,6 +164,7 @@ inline Try<long> mtime(
}
+// TODO(andschwa): Replace `::_stat`. See MESOS-8275.
inline Try<mode_t> mode(
const std::string& path,
const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK)
@@ -179,6 +183,7 @@ inline Try<mode_t> mode(
}
+// TODO(andschwa): Replace `::_stat`. See MESOS-8275.
inline Try<dev_t> dev(
const std::string& path,
const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK)
@@ -197,6 +202,7 @@ inline Try<dev_t> dev(
}
+// TODO(andschwa): Replace `::_stat`. See MESOS-8275.
inline Try<ino_t> inode(
const std::string& path,
const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK)
http://git-wip-us.apache.org/repos/asf/mesos/blob/0f544edb/3rdparty/stout/tests/os/rmdir_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os/rmdir_tests.cpp b/3rdparty/stout/tests/os/rmdir_tests.cpp
index 86ec623..2fd24a8 100644
--- a/3rdparty/stout/tests/os/rmdir_tests.cpp
+++ b/3rdparty/stout/tests/os/rmdir_tests.cpp
@@ -280,12 +280,9 @@ TEST_F(RmdirTest, RemoveDirectoryWithDeviceFile)
#endif // __WINDOWS__
-// TODO(hausdorff): Look into enabling this test on Windows. Currently it is
-// not possible to create a symlink on Windows unless the target exists. See
-// MESOS-5881.
// This test verifies that `rmdir` can remove a directory with a
// symlink that has no target.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(RmdirTest, SYMLINK_RmDirNoTargetSymbolicLink)
+TEST_F(RmdirTest, SYMLINK_RmDirNoTargetSymbolicLink)
{
const string newDirectory = path::join(os::getcwd(), "newDirectory");
ASSERT_SOME(os::mkdir(newDirectory));
http://git-wip-us.apache.org/repos/asf/mesos/blob/0f544edb/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp
index 959b02d..26514f3 100644
--- a/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/stout/tests/os_tests.cpp
@@ -995,10 +995,7 @@ TEST_F(OsTest, Mknod)
#endif // __WINDOWS__
-// TODO(hausdorff): Look into enabling this test on Windows. Currently it is
-// not possible to create a symlink on Windows unless the target exists. See
-// MESOS-5881.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, SYMLINK_Realpath)
+TEST_F(OsTest, SYMLINK_Realpath)
{
// Create a file.
const Try<string> _testFile = os::mktemp();
[11/13] mesos git commit: Windows: Fixed MESOS-6816 to enable
`ExecutorEnvironmentVariables`.
Posted by an...@apache.org.
Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.
By removing the explicit system environment override code, and instead
setting the system environment as the default in the `CreateProcess`
wrapper, the `SlaveTest.ExecutorEnvironmentVariables` can be enabled.
Note that this also required fixing `os::host_default_path()` and using
it, because the test used `PATH = /bin`, which breaks on Windows.
Review: https://reviews.apache.org/r/63816
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0dee8b1c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0dee8b1c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0dee8b1c
Branch: refs/heads/master
Commit: 0dee8b1c96b94edd39c6fe4eed68371423c435ff
Parents: 76ce648
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Nov 3 12:36:22 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 14 --------------
src/slave/containerizer/mesos/launch.cpp | 14 --------------
src/tests/slave_tests.cpp | 22 +++++++++++++++-------
3 files changed, 15 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 9918d83..d837442 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1459,20 +1459,6 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
environment["PATH"] = os::host_default_path();
}
-#ifdef __WINDOWS__
- // TODO(dpravat): (MESOS-6816) We should allow system environment variables to
- // be overwritten if they are specified by the framework. This might cause
- // applications to not work, but upon overriding system defaults, it becomes
- // the overidder's problem.
- Option<map<std::wstring, std::wstring>> systemEnvironment =
- ::internal::windows::get_system_env();
- foreachpair(const std::wstring& key,
- const std::wstring& value,
- systemEnvironment.get()) {
- environment[stringify(key)] = stringify(value);
- }
-#endif // __WINDOWS__
-
vector<string> argv;
argv.push_back(MESOS_DOCKER_EXECUTOR);
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 4fabf92..b1065be 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -962,20 +962,6 @@ int MesosContainerizerLaunch::execute()
environment["PATH"] = os::host_default_path();
}
-#ifdef __WINDOWS__
- // TODO(dpravat): (MESOS-6816) We should allow system environment variables
- // to be overwritten if they are specified by the framework. This might
- // cause applications to not work, but upon overriding system defaults, it
- // becomes the overidder's problem.
- Option<std::map<std::wstring, std::wstring>> systemEnvironment =
- ::internal::windows::get_system_env();
- foreachpair (const std::wstring& key,
- const std::wstring& value,
- systemEnvironment.get()) {
- environment[stringify(key)] = stringify(value);
- }
-#endif // __WINDOWS__
-
envp = os::raw::Envp(environment);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/0dee8b1c/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index b35497b..6e4a860 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -5222,8 +5222,8 @@ TEST_F(SlaveTest, TaskStatusContainerStatus)
// Test that we can set the executors environment variables and it
-// won't inhert the slaves.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
+// won't inherit the slaves.
+TEST_F(SlaveTest, ExecutorEnvironmentVariables)
{
Try<Owned<cluster::Master>> master = StartMaster();
ASSERT_SOME(master);
@@ -5231,11 +5231,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
// Need flags for 'executor_environment_variables'.
slave::Flags flags = CreateSlaveFlags();
- Try<JSON::Object> parse = JSON::parse<JSON::Object>("{\"PATH\": \"/bin\"}");
+ const std::string path = os::host_default_path();
- ASSERT_SOME(parse);
-
- flags.executor_environment_variables = parse.get();
+ flags.executor_environment_variables = JSON::Object{{"PATH", path}};
Owned<MasterDetector> detector = master.get()->createDetector();
Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
@@ -5266,8 +5264,18 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ExecutorEnvironmentVariables)
// Command executor will run as user running test.
CommandInfo command;
+#ifdef __WINDOWS__
+ command.set_shell(false);
+ command.set_value("powershell.exe");
+ command.add_arguments("powershell.exe");
+ command.add_arguments("-NoProfile");
+ command.add_arguments("-Command");
+ command.add_arguments(
+ "if ($env:PATH -eq '" + path + "') { exit 0 } else { exit 1 }");
+#else
command.set_shell(true);
- command.set_value("test $PATH = /bin");
+ command.set_value("test $PATH = " + path);
+#endif // __WINDOWS__
task.mutable_command()->MergeFrom(command);
[12/13] mesos git commit: Windows: Cleaned up `os::exists`.
Posted by an...@apache.org.
Windows: Cleaned up `os::exists`.
More const, RAII, and comments.
Review: https://reviews.apache.org/r/64182
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/adad9948
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/adad9948
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/adad9948
Branch: refs/heads/master
Commit: adad99482cb565fd7cd10e710ed7355b7569ef23
Parents: 1f37e11
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 29 14:18:25 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
.../stout/include/stout/os/windows/exists.hpp | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/adad9948/3rdparty/stout/include/stout/os/windows/exists.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/exists.hpp b/3rdparty/stout/include/stout/os/windows/exists.hpp
index 7c8c85c..50bb92a 100644
--- a/3rdparty/stout/include/stout/os/windows/exists.hpp
+++ b/3rdparty/stout/include/stout/os/windows/exists.hpp
@@ -37,12 +37,15 @@ inline bool exists(const std::string& path)
::internal::windows::longpath(path).data());
if (attributes == INVALID_FILE_ATTRIBUTES) {
- DWORD error = GetLastError();
+ const DWORD error = ::GetLastError();
if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND) {
return false;
}
}
+ // Note that `ERROR_ACCESS_DENIED` etc. indicates the path does exist, but
+ // `INVALID_FILE_ATTRIBUTES` would still be returned.
+
return true;
}
@@ -52,16 +55,11 @@ inline bool exists(const std::string& path)
// os::process(pid) to get details of a process.
inline bool exists(pid_t pid)
{
- HANDLE handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid);
-
- bool has_handle = false;
-
- if (handle != nullptr) {
- has_handle = true;
- CloseHandle(handle);
- }
+ SharedHandle handle(
+ ::OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid),
+ ::CloseHandle);
- return has_handle;
+ return handle.get_handle() != nullptr;
}
[04/13] mesos git commit: Windows: Fixed `os::rmdir` bugs.
Posted by an...@apache.org.
Windows: Fixed `os::rmdir` bugs.
The biggest bug with `os::rmdir` was the race condition that appeared on
Windows. There is now method to delete a file on Windows synchronously.
Furthermore, the `RemoveDirectory` API requires that the directory
actually be empty; that is, it does contain files that are marked for
deletion but not yet deleted. Avoiding this race condition requires
waiting for the file to be deleted, not just marked for deletion.
This was accomplished by simplifying the `recursive_remove_directory`
code so that the base recursion case deletes files and symlinks, and
adding a wait in the depth-first search after each recursion.
Furthermore, `os::rmdir` was incorrectly calling `os::realpath`. The
specification of `os::rmdir` states that it expects an absolute path.
The error condition is that the path is not a directory. A symlink to a
directory is not a directory, so do not follow semantics are required.
In the non-recursive case, a stray ANSI CRT function was still in-use,
instead of a long-path aware Unicode Windows API.
Review: https://reviews.apache.org/r/64181
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1f37e11b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1f37e11b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1f37e11b
Branch: refs/heads/master
Commit: 1f37e11bad1c13a140008c47a33ee58fa710be70
Parents: 453d631
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Tue Nov 28 16:00:59 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
.../include/stout/internal/windows/longpath.hpp | 6 +
3rdparty/stout/include/stout/os/windows/rm.hpp | 48 ++++-
.../stout/include/stout/os/windows/rmdir.hpp | 177 +++++++------------
3 files changed, 121 insertions(+), 110 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f37e11b/3rdparty/stout/include/stout/internal/windows/longpath.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/longpath.hpp b/3rdparty/stout/include/stout/internal/windows/longpath.hpp
index eb62dd6..499eef3 100644
--- a/3rdparty/stout/include/stout/internal/windows/longpath.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/longpath.hpp
@@ -47,6 +47,12 @@ inline std::wstring longpath(const std::string& path)
}
}
+
+inline std::wstring longpath(const std::wstring& path)
+{
+ return longpath(stringify(path));
+}
+
} // namespace windows {
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f37e11b/3rdparty/stout/include/stout/os/windows/rm.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/rm.hpp b/3rdparty/stout/include/stout/os/windows/rm.hpp
index 73ebcec..1c66e3e 100644
--- a/3rdparty/stout/include/stout/os/windows/rm.hpp
+++ b/3rdparty/stout/include/stout/os/windows/rm.hpp
@@ -22,10 +22,46 @@
#include <stout/windows.hpp>
#include <stout/os/stat.hpp>
+#include <stout/windows/os.hpp>
#include <stout/internal/windows/longpath.hpp>
+namespace internal {
+namespace windows {
+
+// NOTE: File and directory deletion on Windows is an asynchronous operation,
+// and there is no built-in way to "wait" on the deletion. So we wait by
+// checking for the path's existence until there is a "file not found" error.
+// Until the file is actually deleted, this will loop on an access denied error
+// (the file exists but has been marked for deletion).
+inline Try<Nothing> wait_on_delete(const std::string& path)
+{
+ // Try for 1 second in 10 intervals of 100 ms.
+ for (int i = 0; i < 10; ++i) {
+ // This should always fail if the file has been marked for deletion.
+ const DWORD attributes =
+ ::GetFileAttributesW(::internal::windows::longpath(path).data());
+ CHECK_EQ(attributes, INVALID_FILE_ATTRIBUTES);
+ const DWORD error = ::GetLastError();
+
+ if (error == ERROR_ACCESS_DENIED) {
+ LOG(WARNING) << "Waiting for file " << path << " to be deleted";
+ os::sleep(Milliseconds(100));
+ } else if (error == ERROR_FILE_NOT_FOUND) {
+ // The file is truly gone, stop waiting.
+ return Nothing();
+ } else {
+ return WindowsError(error);
+ }
+ }
+
+ return Error("Timed out when waiting for file " + path + " to be deleted");
+}
+
+} // namespace windows {
+} // namespace internal {
+
namespace os {
inline Try<Nothing> rm(const std::string& path)
@@ -51,7 +87,17 @@ inline Try<Nothing> rm(const std::string& path)
: ::DeleteFileW(longpath.data());
if (!result) {
- return WindowsError("`os::rm` could not remove '" + path + "'");
+ return WindowsError();
+ }
+
+ // This wait is necessary because the `RemoveDirectory` API does not know to
+ // wait for pending deletions of files in the directory, and can otherwise
+ // immediately fail with "directory not empty" if there still exists a marked
+ // for deletion but not yet deleted file. By making waiting synchronously, we
+ // gain the behavior of the POSIX API.
+ Try<Nothing> deleted = ::internal::windows::wait_on_delete(path);
+ if (deleted.isError()) {
+ return Error("wait_on_delete failed " + deleted.error());
}
return Nothing();
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f37e11b/3rdparty/stout/include/stout/os/windows/rmdir.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/rmdir.hpp b/3rdparty/stout/include/stout/os/windows/rmdir.hpp
index 76b74f8..52e7d86 100644
--- a/3rdparty/stout/include/stout/os/windows/rmdir.hpp
+++ b/3rdparty/stout/include/stout/os/windows/rmdir.hpp
@@ -13,21 +13,22 @@
#ifndef __STOUT_OS_WINDOWS_RMDIR_HPP__
#define __STOUT_OS_WINDOWS_RMDIR_HPP__
+#include <string>
+
#include <glog/logging.h>
+#include <stout/error.hpp>
#include <stout/nothing.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
#include <stout/windows.hpp>
-#include <stout/os/realpath.hpp>
#include <stout/os/rm.hpp>
#include <stout/os/stat.hpp>
#include <stout/windows/error.hpp>
#include <stout/internal/windows/longpath.hpp>
-#include <stout/internal/windows/reparsepoint.hpp>
namespace os {
@@ -45,119 +46,85 @@ namespace internal {
inline Try<Nothing> recursive_remove_directory(
const std::string& path, bool removeRoot, bool continueOnError)
{
- // NOTE: Special case required to match the semantics of POSIX. See comment
- // above. As below, this also handles symlinks correctly, i.e., given a path
- // to a symlink, we delete the symlink rather than the target.
- if (os::stat::isfile(path)) {
+ // Base recursion case to delete a symlink or file.
+ //
+ // We explicitly delete symlinks here to handle hanging symlinks. Note that
+ // `os::rm` will correctly delete the symlink, not the target.
+ if (os::stat::islink(path) || os::stat::isfile(path)) {
return os::rm(path);
}
+ // Recursion case to delete all files and subdirectories of a directory.
+
// Appending a slash here if the path doesn't already have one simplifies
// path join logic later, because (unlike Unix) Windows doesn't like double
// slashes in paths.
- std::string current_path;
+ const std::string current_path =
+ strings::endsWith(path, "\\") ? path : path + "\\";
- if (!strings::endsWith(path, "\\")) {
- current_path = path + "\\";
- } else {
- current_path = path;
- }
const std::wstring long_current_path =
- ::internal::windows::longpath(current_path);
-
- // Get first file matching pattern `X:\path\to\wherever\*`.
- WIN32_FIND_DATAW found;
- const std::wstring search_pattern = long_current_path + L"*";
- const SharedHandle search_handle(
- ::FindFirstFileW(search_pattern.data(), &found),
- ::FindClose);
-
- if (search_handle.get() == INVALID_HANDLE_VALUE) {
- return WindowsError(
- "`os::internal::recursive_remove_directory` failed when searching "
- "for files with pattern '" + stringify(search_pattern) + "'");
- }
-
- do {
- // NOTE: do-while is appropriate here because folder is guaranteed to have
- // at least a file called `.` (and probably also one called `..`).
- const std::wstring current_file(found.cFileName);
+ ::internal::windows::longpath(current_path);
+
+ // Scope the `search_handle` so that it is closed before we delete the current
+ // directory.
+ {
+ // Get first file matching pattern `X:\path\to\wherever\*`.
+ WIN32_FIND_DATAW found;
+ const std::wstring search_pattern = long_current_path + L"*";
+ const SharedHandle search_handle(
+ ::FindFirstFileW(search_pattern.data(), &found), ::FindClose);
+
+ if (search_handle.get() == INVALID_HANDLE_VALUE) {
+ return WindowsError(
+ "FindFirstFile failed for pattern " + stringify(search_pattern));
+ }
- const bool is_current_directory = current_file.compare(L".") == 0;
- const bool is_parent_directory = current_file.compare(L"..") == 0;
+ do {
+ // NOTE: do-while is appropriate here because folder is guaranteed to have
+ // at least a file called `.` (and probably also one called `..`).
+ const std::wstring current_file(found.cFileName);
- // Don't try to delete `.` and `..` files in directory.
- if (is_current_directory || is_parent_directory) {
- continue;
- }
+ const bool is_current_directory = current_file.compare(L".") == 0;
+ const bool is_parent_directory = current_file.compare(L"..") == 0;
- // Path to remove.
- const std::wstring current_absolute_path = long_current_path + current_file;
-
- Try<bool> is_reparse_point =
- ::internal::windows::reparse_point_attribute_set(current_absolute_path);
-
- // Delete current path, whether it's a symlink, directory, or file.
- if (!is_reparse_point.isError() && is_reparse_point.get()) {
- // NOTE: This is a best-effort attempt to delete symlinks even when they
- // are "hanging" (i.e., when the target has since been deleted). We call
- // both `RemoveDirectory` and `DeleteFile` here because we are not sure
- // whether the deleted target was a directory or a file, which in general
- // is hard to determine on Windows.
- //
- // If either `RemoveDirectory` or `DeleteFile` succeeds, the reparse
- // point has been successfully removed, and we report success.
- const BOOL rmdir = ::RemoveDirectoryW(current_absolute_path.data());
-
- if (rmdir == FALSE) {
- const BOOL rm = ::DeleteFileW(current_absolute_path.data());
-
- if (rm == FALSE) {
- return WindowsError(
- "Failed to remove reparse point at '" +
- stringify(current_absolute_path) + "'");
- }
+ // Don't try to delete `.` and `..` files in directory.
+ if (is_current_directory || is_parent_directory) {
+ continue;
}
- } else if (os::stat::isdir(stringify(current_absolute_path))) {
+
+ // Path to remove, note that recursion will call `longpath`.
+ const std::wstring current_absolute_path =
+ long_current_path + current_file;
+
+ // Depth-first search, deleting files and directories.
Try<Nothing> removed = recursive_remove_directory(
stringify(current_absolute_path), true, continueOnError);
if (removed.isError()) {
if (continueOnError) {
- LOG(WARNING) << "Failed to delete directory "
- << stringify(current_absolute_path)
- << " with error " << removed.error();
+ LOG(WARNING) << "Failed to delete path "
+ << stringify(current_absolute_path) << " with error "
+ << removed.error();
} else {
return Error(removed.error());
}
}
- } else {
- if (::DeleteFileW(current_absolute_path.data()) == 0) {
- if (continueOnError) {
- LOG(WARNING)
- << "`os::internal::recursive_remove_directory`"
- << " attempted to delete file '"
- << stringify(current_absolute_path) << "', but failed";
- } else {
- return WindowsError(
- "`os::internal::recursive_remove_directory` attempted to delete "
- "file '" + stringify(current_absolute_path) + "', but failed");
- }
- }
+ } while (::FindNextFileW(search_handle.get(), &found));
+
+ // Check that this loop ended for the right reason.
+ const DWORD error = ::GetLastError();
+ if (error != ERROR_NO_MORE_FILES) {
+ return WindowsError(error);
}
- } while (::FindNextFileW(search_handle.get(), &found));
+ } // Search Handle is closed when this scope is exited.
// Finally, remove current directory unless `removeRoot` is disabled.
- if (removeRoot && ::RemoveDirectoryW(long_current_path.data()) == FALSE) {
- if (continueOnError) {
- LOG(WARNING) << "`os::internal::recursive_remove_directory`"
- << " attempted to delete directory '"
- << current_path << "', but failed";
- return ErrnoError("rmdir failed in 'continueOnError' mode");
+ if (removeRoot) {
+ if (!os::stat::isdir(current_path,
+ os::stat::FollowSymlink::DO_NOT_FOLLOW_SYMLINK)) {
+ return Error("Refusing to rmdir non-directory " + current_path);
} else {
- return ErrnoError(
- "`os::internal::recursive_remove_directory` attempted to delete "
- "directory '" + current_path + "', but failed");
+ return os::rm(current_path);
}
}
@@ -182,29 +149,21 @@ inline Try<Nothing> rmdir(
bool removeRoot = true,
bool continueOnError = false)
{
- // Canonicalize the path to Windows style for the call to
- // `recursive_remove_directory`.
- Result<std::string> root = os::realpath(directory);
-
- if (root.isError()) {
- return Error(root.error());
- } else if (root.isNone()) {
- return Error(
- "Argument to `os::rmdir` is not a valid directory or file: '" +
- directory + "'");
+ // The API of this function also deletes files symlinks according to the tests.
+ if (!os::exists(directory)) {
+ return WindowsError(ERROR_FILE_NOT_FOUND);
}
- if (!recursive) {
- if (::_rmdir(directory.c_str()) < 0) {
- return ErrnoError();
+ if (recursive) {
+ return os::internal::recursive_remove_directory(
+ directory, removeRoot, continueOnError);
+ } else {
+ if (!os::stat::isdir(directory,
+ os::stat::FollowSymlink::DO_NOT_FOLLOW_SYMLINK)) {
+ return Error("Refusing to rmdir non-directory " + directory);
} else {
- return Nothing();
+ return os::rm(directory);
}
- } else {
- return os::internal::recursive_remove_directory(
- root.get(),
- removeRoot,
- continueOnError);
}
}
[02/13] mesos git commit: Windows: Fixed `fs::symlink` to not need
admin privileges.
Posted by an...@apache.org.
Windows: Fixed `fs::symlink` to not need admin privileges.
This new feature of Windows 10 allows us to make symlinks without being
an administrator. In the future, we may want to do a version check, as
this is likely to fail on versions of Windows which don't support this
flag. Resolves MESOS-7370.
Review: https://reviews.apache.org/r/63809
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e1d7c6dd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e1d7c6dd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e1d7c6dd
Branch: refs/heads/master
Commit: e1d7c6dddd1701579a10e0f408b06adfb75e4a93
Parents: 24550e7
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Thu Oct 26 11:22:17 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:10 2017 -0800
----------------------------------------------------------------------
.../stout/internal/windows/reparsepoint.hpp | 145 +++++++------------
1 file changed, 56 insertions(+), 89 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e1d7c6dd/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
index e3d0448..0ff5d68 100644
--- a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
@@ -13,11 +13,9 @@
#ifndef __STOUT_INTERNAL_WINDOWS_REPARSEPOINT_HPP__
#define __STOUT_INTERNAL_WINDOWS_REPARSEPOINT_HPP__
-#include <mutex>
#include <string>
#include <stout/nothing.hpp>
-#include <stout/synchronized.hpp>
#include <stout/try.hpp>
#include <stout/windows.hpp>
@@ -45,6 +43,9 @@ enum class FollowSymlink
} // namespace os {
+namespace internal {
+namespace windows {
+
// We pass this struct to `DeviceIoControl` to get information about a reparse
// point (including things like whether it's a symlink). It is normally part of
// the Device Driver Kit (DDK), specifically `nitfs.h`, but rather than taking
@@ -55,69 +56,44 @@ enum class FollowSymlink
// [1] http://www.boost.org/doc/libs/1_46_1/libs/filesystem/v3/src/operations.cpp
// [2] https://msdn.microsoft.com/en-us/library/cc232007.aspx
// [3] https://msdn.microsoft.com/en-us/library/cc232005.aspx
-//
-// We are declaring this structure (and the REPARSE_DATA_BUFFER_HEADER_SIZE
-// macro right below it in the global namespace, to be consistent with the
-// original Windows DDK declarations.
typedef struct _REPARSE_DATA_BUFFER
{
// Describes, among other things, which type of reparse point this is (e.g.,
// a symlink).
- ULONG ReparseTag;
+ ULONG ReparseTag;
// Size in bytes of common portion of the `REPARSE_DATA_BUFFER`.
- USHORT ReparseDataLength;
+ USHORT ReparseDataLength;
// Unused. Ignore.
- USHORT Reserved;
- union
+ USHORT Reserved;
+ // Holds symlink data.
+ struct
{
- // Holds symlink data.
- struct
- {
- // Byte offset in `PathBuffer` where the substitute name begins.
- // Calculated as an offset from 0.
- USHORT SubstituteNameOffset;
- // Length in bytes of the substitute name.
- USHORT SubstituteNameLength;
- // Byte offset in `PathBuffer` where the print name begins. Calculated as
- // an offset from 0.
- USHORT PrintNameOffset;
- // Length in bytes of the print name.
- USHORT PrintNameLength;
- // Indicates whether symlink is absolute or relative. If flags containing
- // `SYMLINK_FLAG_RELATIVE`, then the substitute name is a relative
- // symlink.
- ULONG Flags;
- // The first byte of the path string -- according to the documentation[1],
- // this is followed in memory by the rest of the path string. The "path
- // string" itself is a unicode char array containing both substitute name
- // and print name. They can occur in any order. Use the offset and length
- // of each in this struct to calculate where each starts and ends.
- //
- // [1] https://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx
- WCHAR PathBuffer[1];
- } SymbolicLinkReparseBuffer;
-
- // Unused: holds mount point data.
- struct
- {
- USHORT SubstituteNameOffset;
- USHORT SubstituteNameLength;
- USHORT PrintNameOffset;
- USHORT PrintNameLength;
- WCHAR PathBuffer[1];
- } MountPointReparseBuffer;
- struct
- {
- UCHAR DataBuffer[1];
- } GenericReparseBuffer;
- };
+ // Byte offset in `PathBuffer` where the substitute name begins.
+ // Calculated as an offset from 0.
+ USHORT SubstituteNameOffset;
+ // Length in bytes of the substitute name.
+ USHORT SubstituteNameLength;
+ // Byte offset in `PathBuffer` where the print name begins. Calculated as
+ // an offset from 0.
+ USHORT PrintNameOffset;
+ // Length in bytes of the print name.
+ USHORT PrintNameLength;
+ // Indicates whether symlink is absolute or relative. If flags containing
+ // `SYMLINK_FLAG_RELATIVE`, then the substitute name is a relative
+ // symlink.
+ ULONG Flags;
+ // The path string; the Windows declaration is the first byte, but we
+ // declare a suitably sized array so we can use this struct more easily. The
+ // "path string" itself is a Unicode `wchar` array containing both
+ // substitute name and print name. They can occur in any order. Use the
+ // offset and length of each in this struct to calculate where each starts
+ // and ends.
+ //
+ // https://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx
+ WCHAR PathBuffer[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
+ } SymbolicLinkReparseBuffer;
} REPARSE_DATA_BUFFER;
-#define REPARSE_DATA_BUFFER_HEADER_SIZE \
- FIELD_OFFSET(REPARSE_DATA_BUFFER, GenericReparseBuffer)
-
-namespace internal {
-namespace windows {
// Convenience struct for holding symlink data, meant purely for internal use.
// We pass this around instead of the `REPARSE_DATA_BUFFER` struct, simply
@@ -254,11 +230,7 @@ inline Try<SymbolicLink> get_symbolic_link_data(const HANDLE handle)
// [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364571(v=vs.85).aspx
// [2] https://svn.boost.org/trac/boost/ticket/4663
// [3] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363216(v=vs.85).aspx
- const size_t reparse_point_data_size = MAXIMUM_REPARSE_DATA_BUFFER_SIZE;
- BYTE buffer[reparse_point_data_size];
- REPARSE_DATA_BUFFER* reparse_point_data =
- reinterpret_cast<REPARSE_DATA_BUFFER*>(buffer);
-
+ REPARSE_DATA_BUFFER buffer;
DWORD ignored = 0;
// The semantics of this function are: get the reparse data associated with
@@ -269,8 +241,8 @@ inline Try<SymbolicLink> get_symbolic_link_data(const HANDLE handle)
FSCTL_GET_REPARSE_POINT, // Gets reparse point data for file/folder.
nullptr, // Ignored.
0, // Ignored.
- reparse_point_data,
- reparse_point_data_size,
+ &buffer,
+ sizeof(buffer),
&ignored, // Ignored.
nullptr); // Ignored.
@@ -280,18 +252,13 @@ inline Try<SymbolicLink> get_symbolic_link_data(const HANDLE handle)
"failed");
}
- return build_symbolic_link(*reparse_point_data);
+ return build_symbolic_link(buffer);
}
// Creates a reparse point with the specified target. The target can be either
// a file (in which case a junction is created), or a folder (in which case a
// mount point is created).
-//
-// Calling this function results in a temporary elevation of the process token's
-// privileges (if those privileges are not already held), to allow for junction
-// or mount point creation. This operation is gated by a static mutex, which
-// makes it thread-safe.
inline Try<Nothing> create_symbolic_link(
const std::string& target,
const std::string& reparse_point)
@@ -320,8 +287,8 @@ inline Try<Nothing> create_symbolic_link(
// Determine if target is a folder or a file. This makes a difference
// in the way we call `create_symbolic_link`.
- const Try<DWORD> attributes = ::internal::windows::get_file_attributes(
- ::internal::windows::longpath(absolute_target_path));
+ const Try<DWORD> attributes = get_file_attributes(
+ longpath(absolute_target_path));
if (attributes.isError()) {
return Error(attributes.error());
@@ -344,24 +311,24 @@ inline Try<Nothing> create_symbolic_link(
"Path '" + absolute_target_path + "' is already a reparse point");
}
- // `CreateSymbolicLink` adjusts the process token's privileges to allow for
- // symlink creation. MSDN[1] makes no guarantee when it comes to the thread
- // safety of this operation, so we are making use of a mutex to prevent
- // multiple concurrent calls.
- //
- // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363866(v=vs.85).aspx
- static std::mutex adjust_privileges_mutex;
- synchronized(adjust_privileges_mutex) {
- if (!::CreateSymbolicLinkW(
- // Path to link.
- longpath(real_reparse_point_path.get()).data(),
- // Path to target.
- longpath(real_target_path.get()).data(),
- target_is_folder ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0)) {
- return WindowsError(
- "'internal::windows::create_symbolic_link': 'CreateSymbolicLink' "
- "call failed");
- }
+ // Avoid requiring administrative privileges to create symbolic links.
+ DWORD flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
+ if (target_is_folder) {
+ flags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
+ }
+
+ // `CreateSymbolicLink` normally adjusts the process token's privileges to
+ // allow for symlink creation; however, we explicitly avoid this with the
+ // above flag.
+ if (!::CreateSymbolicLinkW(
+ // Path to link.
+ longpath(real_reparse_point_path.get()).data(),
+ // Path to target.
+ longpath(real_target_path.get()).data(),
+ flags)) {
+ return WindowsError(
+ "'internal::windows::create_symbolic_link': 'CreateSymbolicLink' "
+ "call failed");
}
return Nothing();
[03/13] mesos git commit: Windows: Fixed `os::host_default_path()`.
Posted by an...@apache.org.
Windows: Fixed `os::host_default_path()`.
This adds the necessary and expected default paths to the default `PATH`
value represented by this function. Especially needed was the
`WindowsPowerShell` folder so that tasks using PowerShell can run.
Review: https://reviews.apache.org/r/63814
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/58322265
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/58322265
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/58322265
Branch: refs/heads/master
Commit: 583222650a078a9233919e2645080aeaa6507c74
Parents: 3b130a4
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Nov 3 12:27:02 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/windows/os.hpp | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/58322265/3rdparty/stout/include/stout/windows/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/os.hpp b/3rdparty/stout/include/stout/windows/os.hpp
index a24bad9..09469e9 100644
--- a/3rdparty/stout/include/stout/windows/os.hpp
+++ b/3rdparty/stout/include/stout/windows/os.hpp
@@ -801,13 +801,17 @@ inline std::string host_default_path()
{
// NOTE: On Windows, this code must run on the host where we are
// expecting to `exec` the task, because the value of
- // `%SYSTEMROOT%` is not identical on all platforms.
- const Option<std::string> system_root_env = os::getenv("SYSTEMROOT");
+ // `%SystemRoot%` is not identical on all platforms.
+ const Option<std::string> system_root_env = os::getenv("SystemRoot");
const std::string system_root = system_root_env.isSome()
? system_root_env.get()
- : "C:\\WINDOWS";
+ : path::join("C:", "Windows");
- return strings::join(";", system_root, path::join(system_root, "system32"));
+ return strings::join(";",
+ path::join(system_root, "System32"),
+ system_root,
+ path::join(system_root, "System32", "Wbem"),
+ path::join(system_root, "System32", "WindowsPowerShell", "v1.0"));
}
} // namespace os {
[06/13] mesos git commit: Windows: Added internal `fullpath` API to
normalize paths.
Posted by an...@apache.org.
Windows: Added internal `fullpath` API to normalize paths.
This explicitly does not check for existence, nor does it follow
symlinks. It simply normalizes a given path to an absolute path. This
removes the `reparsepoint.hpp` and `symlink.hpp` dependency os
`os::realpath`.
Review: https://reviews.apache.org/r/63810
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b64ff888
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b64ff888
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b64ff888
Branch: refs/heads/master
Commit: b64ff8888bbcafe40e07fafa80e8bcfa2b8fd267
Parents: 0f544ed
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 1 12:28:54 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
.../include/stout/internal/windows/symlink.hpp | 33 ++++++++++++++++++--
3rdparty/stout/include/stout/windows/fs.hpp | 2 ++
2 files changed, 32 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b64ff888/3rdparty/stout/include/stout/internal/windows/symlink.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/symlink.hpp b/3rdparty/stout/include/stout/internal/windows/symlink.hpp
index b9cf122..def5515 100644
--- a/3rdparty/stout/include/stout/internal/windows/symlink.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/symlink.hpp
@@ -21,12 +21,39 @@
#include <stout/internal/windows/longpath.hpp>
#include <stout/internal/windows/reparsepoint.hpp>
-#include <stout/os/realpath.hpp>
-
namespace internal {
namespace windows {
+// Get the full / absolute path. Does not check for existence, and does not
+// resolve symlinks.
+inline Result<std::string> fullpath(const std::string& path)
+{
+ // First query for the buffer size required.
+ const DWORD length =
+ ::GetFullPathNameW(longpath(path).data(), 0, nullptr, nullptr);
+
+ if (length == 0) {
+ return WindowsError("Failed to retrieve fullpath buffer size");
+ }
+
+ std::vector<wchar_t> buffer;
+ buffer.reserve(static_cast<size_t>(length));
+
+ const DWORD result =
+ ::GetFullPathNameW(longpath(path).data(), length, buffer.data(), nullptr);
+
+ if (result == 0) {
+ return WindowsError("Failed to determine fullpath");
+ }
+
+ return strings::remove(
+ stringify(std::wstring(buffer.data())),
+ os::LONGPATH_PREFIX,
+ strings::Mode::PREFIX);
+}
+
+
// Gets symlink data for a given path, if it exists.
//
// This turns out to be a very complicated task on Windows. The gist of it is
@@ -52,7 +79,7 @@ namespace windows {
inline Try<SymbolicLink> query_symbolic_link_data(const std::string& path)
{
// Convert to absolute path because Windows APIs expect it.
- const Result<std::string> absolute_path = os::realpath(path);
+ const Result<std::string> absolute_path = fullpath(path);
if (!absolute_path.isSome()) {
return Error(absolute_path.error());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/b64ff888/3rdparty/stout/include/stout/windows/fs.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/fs.hpp b/3rdparty/stout/include/stout/windows/fs.hpp
index d7b883c..bc7cf02 100644
--- a/3rdparty/stout/include/stout/windows/fs.hpp
+++ b/3rdparty/stout/include/stout/windows/fs.hpp
@@ -21,6 +21,8 @@
#include <stout/try.hpp>
#include <stout/windows.hpp>
+#include <stout/os/realpath.hpp>
+
#include <stout/internal/windows/longpath.hpp>
#include <stout/internal/windows/symlink.hpp>
[10/13] mesos git commit: Windows: Fixed name of default executor.
Posted by an...@apache.org.
Windows: Fixed name of default executor.
This path is checked with `os::realpath`, which now correctly errors if
the path does not exist. Therefore the name must be correct.
Review: https://reviews.apache.org/r/63813
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3b130a4d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3b130a4d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3b130a4d
Branch: refs/heads/master
Commit: 3b130a4d7ab7ebcf6029fd04d58723f716225392
Parents: 4f01558
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 1 16:36:19 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
src/slave/constants.hpp | 4 ++++
1 file changed, 4 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3b130a4d/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index 4015906..2d07bce 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -162,7 +162,11 @@ constexpr Bytes DEFAULT_FETCHER_CACHE_SIZE = Gigabytes(2);
Duration DEFAULT_MASTER_PING_TIMEOUT();
// Name of the executable for default executor.
+#ifdef __WINDOWS__
+constexpr char MESOS_DEFAULT_EXECUTOR[] = "mesos-default-executor.exe";
+#else
constexpr char MESOS_DEFAULT_EXECUTOR[] = "mesos-default-executor";
+#endif // __WINDOWS__
// Virtual path on which agent logs are mounted in `/files/` endpoint.
constexpr char AGENT_LOG_VIRTUAL_PATH[] = "/slave/log";
[07/13] mesos git commit: Windows: Added internal `get_handle_follow`
which follows symlinks.
Posted by an...@apache.org.
Windows: Added internal `get_handle_follow` which follows symlinks.
This is the opposite of `get_handle_no_follow`. They both use the
inappropriately named Windows API `CreateFile` to retrieve a `HANDLE` to
an existing file or directory. This existing `get_handle_no_follow`
explicitly gets a handle to the symlink itself, and the new
`get_handle_follow` explicitly resolves symlinks and gets a handle to
the target.
This may fail if the target doesn't exist.
Review: https://reviews.apache.org/r/63811
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f1229891
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f1229891
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f1229891
Branch: refs/heads/master
Commit: f1229891d36825806464413bf037464c8cca606b
Parents: b64ff88
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 1 12:30:33 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
.../stout/internal/windows/reparsepoint.hpp | 78 +++++++++++++++++---
1 file changed, 66 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f1229891/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
index fff2cbb..158f445 100644
--- a/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
@@ -52,7 +52,7 @@ namespace windows {
// well-worn path used by Boost FS[1], among others. See documentation
// here[2][3].
//
-// [1] http://www.boost.org/doc/libs/1_46_1/libs/filesystem/v3/src/operations.cpp
+// [1] http://www.boost.org/doc/libs/1_46_1/libs/filesystem/v3/src/operations.cpp // NOLINT(whitespace/line_length)
// [2] https://msdn.microsoft.com/en-us/library/cc232007.aspx
// [3] https://msdn.microsoft.com/en-us/library/cc232005.aspx
typedef struct _REPARSE_DATA_BUFFER
@@ -88,7 +88,7 @@ typedef struct _REPARSE_DATA_BUFFER
// offset and length of each in this struct to calculate where each starts
// and ends.
//
- // https://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx
+ // https://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx // NOLINT(whitespace/line_length)
WCHAR PathBuffer[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
} SymbolicLinkReparseBuffer;
} REPARSE_DATA_BUFFER;
@@ -149,13 +149,69 @@ inline Try<SymbolicLink> build_symbolic_link(const REPARSE_DATA_BUFFER& data)
}
+// Attempts to get a file or folder handle for an absolute path, and follows
+// symlinks. That is, if the path points at a symlink, the handle will refer to
+// the file or folder the symlink points at, rather than the symlink itself.
+inline Try<SharedHandle> get_handle_follow(const std::string& absolute_path)
+{
+ const Try<DWORD> attributes = get_file_attributes(longpath(absolute_path));
+
+ if (attributes.isError()) {
+ return Error(attributes.error());
+ }
+
+ bool resolved_path_is_directory = attributes.get() & FILE_ATTRIBUTE_DIRECTORY;
+
+ // NOTE: The name of `CreateFile` is misleading: it is also used to retrieve
+ // handles to existing files or directories as if it were actually `OpenPath`
+ // (which does not exist). We use `OPEN_EXISTING` but not
+ // `FILE_FLAG_OPEN_REPARSE_POINT` to explicitily follow (resolve) symlinks in
+ // the path to the file or directory.
+ //
+ // Note also that `CreateFile` will appropriately generate a handle for
+ // either a folder or a file, as long as the appropriate flag is being set:
+ // `FILE_FLAG_BACKUP_SEMANTICS`.
+ //
+ // The `FILE_FLAG_BACKUP_SEMANTICS` flag is being set whenever the target is
+ // a directory. According to MSDN[1]: "You must set this flag to obtain a
+ // handle to a directory. A directory handle can be passed to some functions
+ // instead of a file handle". More `FILE_FLAG_BACKUP_SEMANTICS` documentation
+ // can be found in MSDN[2].
+ //
+ // The `GENERIC_READ` flag is being used because it's the most common way of
+ // opening a file for reading only. The `FILE_SHARE_READ` allows other
+ // processes to read the file at the same time. MSDN[1] provides a more
+ // detailed explanation of these flags.
+ //
+ // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx // NOLINT(whitespace/line_length)
+ // [2] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364399(v=vs.85).aspx // NOLINT(whitespace/line_length)
+ const DWORD access_flags = resolved_path_is_directory
+ ? FILE_FLAG_BACKUP_SEMANTICS
+ : FILE_ATTRIBUTE_NORMAL;
+
+ const HANDLE handle = ::CreateFileW(
+ longpath(absolute_path).data(),
+ GENERIC_READ, // Open the file for reading only.
+ FILE_SHARE_READ, // Just reading this file, allow others to do the same.
+ nullptr, // Ignored.
+ OPEN_EXISTING, // Open existing file.
+ access_flags, // Open file, not the symlink itself.
+ nullptr); // Ignored.
+
+ if (handle == INVALID_HANDLE_VALUE) {
+ return WindowsError();
+ }
+
+ return SharedHandle(handle, ::CloseHandle);
+}
+
+
// Attempts to get a file or folder handle for an absolute path, and does not
// follow symlinks. That is, if the path points at a symlink, the handle will
// refer to the symlink rather than the file or folder the symlink points at.
inline Try<SharedHandle> get_handle_no_follow(const std::string& absolute_path)
{
- const Try<DWORD> attributes = ::internal::windows::get_file_attributes(
- ::internal::windows::longpath(absolute_path));
+ const Try<DWORD> attributes = get_file_attributes(longpath(absolute_path));
if (attributes.isError()) {
return Error(attributes.error());
@@ -182,8 +238,8 @@ inline Try<SharedHandle> get_handle_no_follow(const std::string& absolute_path)
// processes to read the file at the same time. MSDN[1] provides a more
// detailed explanation of these flags.
//
- // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx
- // [2] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364399(v=vs.85).aspx
+ // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx // NOLINT(whitespace/line_length)
+ // [2] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364399(v=vs.85).aspx // NOLINT(whitespace/line_length)
const DWORD access_flags = resolved_path_is_directory
? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
: FILE_FLAG_OPEN_REPARSE_POINT;
@@ -198,12 +254,10 @@ inline Try<SharedHandle> get_handle_no_follow(const std::string& absolute_path)
nullptr); // Ignored.
if (handle == INVALID_HANDLE_VALUE) {
- return WindowsError(
- "'internal::windows::get_handle_no_follow': 'CreateFile' call failed "
- "at path '" + absolute_path + "'");
+ return WindowsError();
}
- return SharedHandle(handle, CloseHandle);
+ return SharedHandle(handle, ::CloseHandle);
}
@@ -226,9 +280,9 @@ inline Try<SymbolicLink> get_symbolic_link_data(const HANDLE handle)
// Finally, for context, it may be worth looking at the MSDN
// documentation[3] for `DeviceIoControl` itself.
//
- // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364571(v=vs.85).aspx
+ // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364571(v=vs.85).aspx // NOLINT(whitespace/line_length)
// [2] https://svn.boost.org/trac/boost/ticket/4663
- // [3] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363216(v=vs.85).aspx
+ // [3] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363216(v=vs.85).aspx // NOLINT(whitespace/line_length)
REPARSE_DATA_BUFFER buffer;
DWORD ignored = 0;
[13/13] mesos git commit: Windows: Enabled more agent tests.
Posted by an...@apache.org.
Windows: Enabled more agent tests.
These tests "just worked" once the isolators and command was fixed to be
Windows-compatible. Particularly, `/bin/echo --author` was replaced with
an equivalent PowerShell command, and `posix/cpu,posix/mem` was replaced
with `windows/cpu`.
Review: https://reviews.apache.org/r/63817
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/453d631c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/453d631c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/453d631c
Branch: refs/heads/master
Commit: 453d631cf116275f567e222c54481b614d1bb965
Parents: 0dee8b1
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Nov 3 12:42:35 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
src/tests/slave_tests.cpp | 85 ++++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/453d631c/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 6e4a860..823bd44 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -151,7 +151,35 @@ namespace tests {
// that seem vaguely more slave than master-related are in this file.
// The others are in "master_tests.cpp".
-class SlaveTest : public MesosTest {};
+class SlaveTest : public MesosTest
+{
+public:
+ const std::string defaultIsolators{
+#ifdef __WINDOWS__
+ "windows/cpu"
+#else
+ "posix/cpu,posix/mem"
+#endif // __WINDOWS__
+ };
+
+ CommandInfo echoAuthorCommand()
+ {
+ CommandInfo command;
+ command.set_shell(false);
+#ifdef __WINDOWS__
+ command.set_value("powershell.exe");
+ command.add_arguments("powershell.exe");
+ command.add_arguments("-NoProfile");
+ command.add_arguments("-Command");
+ command.add_arguments("echo --author");
+#else
+ command.set_value("/bin/echo");
+ command.add_arguments("/bin/echo");
+ command.add_arguments("--author");
+#endif // __WINDOWS__
+ return command;
+ };
+};
// This test ensures that when a slave shuts itself down, it
@@ -327,7 +355,7 @@ TEST_F(SlaveTest, DuplicateTerminalUpdateBeforeAck)
}
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ShutdownUnregisteredExecutor)
+TEST_F(SlaveTest, ShutdownUnregisteredExecutor)
{
Try<Owned<cluster::Master>> master = StartMaster();
ASSERT_SOME(master);
@@ -336,7 +364,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ShutdownUnregisteredExecutor)
slave::Flags flags = CreateSlaveFlags();
// Set the isolation flag so we know a MesosContainerizer will
// be created.
- flags.isolation = "posix/cpu,posix/mem";
+ flags.isolation = defaultIsolators;
Fetcher fetcher(flags);
@@ -616,14 +644,14 @@ TEST_F(SlaveTest, RemoveUnregisteredTerminatedExecutor)
// mesos-executor args. For more details of this see MESOS-1873.
//
// This assumes the ability to execute '/bin/echo --author'.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, CommandTaskWithArguments)
+TEST_F(SlaveTest, CommandTaskWithArguments)
{
Try<Owned<cluster::Master>> master = StartMaster();
ASSERT_SOME(master);
// Need flags for 'executor_registration_timeout'.
slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "posix/cpu,posix/mem";
+ flags.isolation = defaultIsolators;
Fetcher fetcher(flags);
@@ -663,13 +691,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, CommandTaskWithArguments)
task.mutable_resources()->MergeFrom(offers.get()[0].resources());
// Command executor will run as user running test.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/echo");
- command.add_arguments("/bin/echo");
- command.add_arguments("--author");
-
- task.mutable_command()->MergeFrom(command);
+ task.mutable_command()->MergeFrom(echoAuthorCommand());
Future<TaskStatus> statusStarting;
Future<TaskStatus> statusRunning;
@@ -783,7 +805,7 @@ TEST_F(SlaveTest, CommandTaskWithKillPolicy)
// Don't let args from the CommandInfo struct bleed over into
// mesos-executor forking. For more details of this see MESOS-1873.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfo)
+TEST_F(SlaveTest, GetExecutorInfo)
{
TestContainerizer containerizer;
StandaloneMasterDetector detector;
@@ -813,14 +835,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfo)
task.mutable_slave_id()->set_value(
"20141010-221431-251662764-60288-32120-0001");
task.mutable_resources()->MergeFrom(taskResources);
-
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/echo");
- command.add_arguments("/bin/echo");
- command.add_arguments("--author");
-
- task.mutable_command()->MergeFrom(command);
+ task.mutable_command()->MergeFrom(echoAuthorCommand());
DiscoveryInfo* info = task.mutable_discovery();
info->set_visibility(DiscoveryInfo::EXTERNAL);
@@ -852,7 +867,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfo)
// Ensure getExecutorInfo for mesos-executor gets the ContainerInfo,
// if present. This ensures the MesosContainerizer can get the
// NetworkInfo even when using the command executor.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfoForTaskWithContainer)
+TEST_F(SlaveTest, GetExecutorInfoForTaskWithContainer)
{
TestContainerizer containerizer;
StandaloneMasterDetector detector;
@@ -881,14 +896,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfoForTaskWithContainer)
task.mutable_slave_id()->set_value(
"20141010-221431-251662764-60288-12345-0001");
task.mutable_resources()->MergeFrom(taskResources);
-
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/echo");
- command.add_arguments("/bin/echo");
- command.add_arguments("--author");
-
- task.mutable_command()->MergeFrom(command);
+ task.mutable_command()->MergeFrom(echoAuthorCommand());
ContainerInfo* container = task.mutable_container();
container->set_type(ContainerInfo::MESOS);
@@ -924,14 +932,14 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, GetExecutorInfoForTaskWithContainer)
// MesosContainerizer would fail the launch.
//
// TODO(jieyu): Move this test to the mesos containerizer tests.
-TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ROOT_LaunchTaskInfoWithContainerInfo)
+TEST_F(SlaveTest, ROOT_LaunchTaskInfoWithContainerInfo)
{
Try<Owned<cluster::Master>> master = StartMaster();
ASSERT_SOME(master);
// Need flags for 'executor_registration_timeout'.
slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "posix/cpu,posix/mem";
+ flags.isolation = defaultIsolators;
Fetcher fetcher(flags);
@@ -967,14 +975,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, ROOT_LaunchTaskInfoWithContainerInfo)
task.mutable_slave_id()->set_value(
"20141010-221431-251662764-60288-12345-0001");
task.mutable_resources()->MergeFrom(taskResources);
-
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/echo");
- command.add_arguments("/bin/echo");
- command.add_arguments("--author");
-
- task.mutable_command()->MergeFrom(command);
+ task.mutable_command()->MergeFrom(echoAuthorCommand());
ContainerID containerId;
containerId.set_value(UUID::random().toString());
@@ -3651,6 +3652,8 @@ TEST_F(SlaveTest, HealthCheckUnregisterRace)
// This test verifies that when an unreachable agent reregisters after
// master failover, the master consults and updates the registrar for
// re-admitting the agent.
+//
+// TODO(andschwa): Enable when Windows supports replicated log. See MESOS-5932.
TEST_F_TEMP_DISABLED_ON_WINDOWS(
SlaveTest, UnreachableAgentReregisterAfterFailover)
{
@@ -3749,6 +3752,8 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
// This test verifies that when a registered agent restarts and reregisters
// after master failover, the master does not consult the registrar in
// deciding to re-admit the agent.
+//
+// TODO(andschwa): Enable when Windows supports replicated log. See MESOS-5932.
TEST_F_TEMP_DISABLED_ON_WINDOWS(
SlaveTest, RegisteredAgentReregisterAfterFailover)
{
[05/13] mesos git commit: Windows: Fixed environment priorities in
`shell.hpp`.
Posted by an...@apache.org.
Windows: Fixed environment priorities in `shell.hpp`.
Legacy code had caused the system environment to explicitly override the
supplied environment, but this is incorrect. The system environment
should be the default, with any supplied environment variables
overriding those.
Review: https://reviews.apache.org/r/63815
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/76ce648b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/76ce648b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/76ce648b
Branch: refs/heads/master
Commit: 76ce648b56e18f8c28dc163b7e0d10f5b16ee6f0
Parents: 5832226
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Nov 3 12:31:47 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/os/windows/shell.hpp | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/76ce648b/3rdparty/stout/include/stout/os/windows/shell.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/shell.hpp b/3rdparty/stout/include/stout/os/windows/shell.hpp
index 5c71183..542039c 100644
--- a/3rdparty/stout/include/stout/os/windows/shell.hpp
+++ b/3rdparty/stout/include/stout/os/windows/shell.hpp
@@ -116,21 +116,20 @@ inline Option<std::wstring> create_process_env(
std::map<std::wstring, std::wstring> combined_env;
- // Populate the combined environment first with the given environment
- // converted to UTF-16 for Windows.
- foreachpair (const std::string& key,
- const std::string& value,
- env.get()) {
- combined_env[wide_stringify(key)] = wide_stringify(value);
- }
-
- // Add the system environment variables, overwriting the previous.
+ // Populate the combined environment first with the system environment.
foreachpair (const std::wstring& key,
const std::wstring& value,
system_env.get()) {
combined_env[key] = value;
}
+ // Now override with the supplied environment.
+ foreachpair (const std::string& key,
+ const std::string& value,
+ env.get()) {
+ combined_env[wide_stringify(key)] = wide_stringify(value);
+ }
+
std::wstring env_string;
foreachpair (const std::wstring& key,
const std::wstring& value,
[08/13] mesos git commit: Windows: Fixed `os::realpath` to behave
like POSIX version.
Posted by an...@apache.org.
Windows: Fixed `os::realpath` to behave like POSIX version.
The existing implementation (1) did not resolve symlinks and (2) did not
check for existence of the path. This resulted in unexpected behaviors
by users of the function.
Review: https://reviews.apache.org/r/63812
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4f015587
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4f015587
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4f015587
Branch: refs/heads/master
Commit: 4f01558778e27ec1687fa8d336c0d8bef2372109
Parents: f122989
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 1 12:46:03 2017 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
.../stout/include/stout/os/windows/realpath.hpp | 21 +++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4f015587/3rdparty/stout/include/stout/os/windows/realpath.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/realpath.hpp b/3rdparty/stout/include/stout/os/windows/realpath.hpp
index 81a33bd..c6bad50 100644
--- a/3rdparty/stout/include/stout/os/windows/realpath.hpp
+++ b/3rdparty/stout/include/stout/os/windows/realpath.hpp
@@ -18,21 +18,28 @@
#include <stout/result.hpp>
#include <stout/stringify.hpp>
#include <stout/strings.hpp>
+#include <stout/windows.hpp>
#include <stout/internal/windows/longpath.hpp>
+#include <stout/internal/windows/reparsepoint.hpp>
namespace os {
+// This should behave like the POSIX `realpath` API: specifically it should
+// resolve symlinks in the path, and succeed only if the target file exists.
+// This requires that the user has permissions to resolve each component of the
+// path.
inline Result<std::string> realpath(const std::string& path)
{
- // TODO(andschwa): Test the existence of `path` to be consistent with POSIX
- // `::realpath`.
-
- std::wstring longpath = ::internal::windows::longpath(path);
+ const Try<SharedHandle> handle = ::internal::windows::get_handle_follow(path);
+ if (handle.isError()) {
+ return Error(handle.error());
+ }
// First query for the buffer size required.
- DWORD length = GetFullPathNameW(longpath.data(), 0, nullptr, nullptr);
+ DWORD length = ::GetFinalPathNameByHandleW(
+ handle.get().get_handle(), nullptr, 0, FILE_NAME_NORMALIZED);
if (length == 0) {
return WindowsError("Failed to retrieve realpath buffer size");
}
@@ -40,8 +47,8 @@ inline Result<std::string> realpath(const std::string& path)
std::vector<wchar_t> buffer;
buffer.reserve(static_cast<size_t>(length));
- DWORD result =
- GetFullPathNameW(longpath.data(), length, buffer.data(), nullptr);
+ DWORD result = ::GetFinalPathNameByHandleW(
+ handle.get().get_handle(), buffer.data(), length, FILE_NAME_NORMALIZED);
if (result == 0) {
return WindowsError("Failed to determine realpath");
[09/13] mesos git commit: Included `stout/os/realpath.hpp` in
`flags.hpp`.
Posted by an...@apache.org.
Included `stout/os/realpath.hpp` in `flags.hpp`.
This was used but not included, and broke on Windows because no other
headers included it.
Review: https://reviews.apache.org/r/64187
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5dd3b290
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5dd3b290
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5dd3b290
Branch: refs/heads/master
Commit: 5dd3b290032f54e3ad6e842b45975990515bdaa9
Parents: adad994
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Wed Nov 29 15:45:10 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Nov 29 19:24:11 2017 -0800
----------------------------------------------------------------------
src/tests/flags.hpp | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5dd3b290/src/tests/flags.hpp
----------------------------------------------------------------------
diff --git a/src/tests/flags.hpp b/src/tests/flags.hpp
index 0050fc9..df4d458 100644
--- a/src/tests/flags.hpp
+++ b/src/tests/flags.hpp
@@ -25,6 +25,8 @@
#include <stout/flags.hpp>
#include <stout/os.hpp>
+#include <stout/os/realpath.hpp>
+
#include "common/parse.hpp"
#include "logging/logging.hpp"