You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2017/03/09 21:30:33 UTC

[1/5] mesos git commit: CMake: Changed usage of `get_filename_component` function.

Repository: mesos
Updated Branches:
  refs/heads/master eb08f0895 -> de89ac845


CMake: Changed usage of `get_filename_component` function.

The built-in function `get_filename_component` has a legacy argument
'PATH' which is used for CMake versions <= 2.8.11.  The argument is
renamed to 'DIRECTORY' in later versions of CMake.

Since we support CMake down to 2.8.10, we need to use the legacy
argument instead of the newer one.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0eee031f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0eee031f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0eee031f

Branch: refs/heads/master
Commit: 0eee031f90e8896f297bfe96751868dee61930ef
Parents: a95abd0
Author: Joseph Wu <jo...@apache.org>
Authored: Fri Mar 3 14:01:20 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Mar 9 11:14:29 2017 -0800

----------------------------------------------------------------------
 3rdparty/stout/cmake/GroupSource.cmake | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0eee031f/3rdparty/stout/cmake/GroupSource.cmake
----------------------------------------------------------------------
diff --git a/3rdparty/stout/cmake/GroupSource.cmake b/3rdparty/stout/cmake/GroupSource.cmake
index 3cf6a90..b0b2b1d 100644
--- a/3rdparty/stout/cmake/GroupSource.cmake
+++ b/3rdparty/stout/cmake/GroupSource.cmake
@@ -71,10 +71,15 @@ function(GROUP_SOURCE GROUP_NAME ROOT_DIRECTORY RELATIVE_TO SOURCE_PATTERN)
     # Get relative directory of source file, use to generate a source group
     # that resembles the directory tree.
     get_filename_component(SOURCE_FILE_REALPATH ${SOURCE_FILE} REALPATH)
-    file(RELATIVE_PATH SOURCE_FILE_RELATIVE ${RELATIVE_TO}
-         ${SOURCE_FILE_REALPATH})
-    get_filename_component(SOURCE_DIRECTORY_RELATIVE ${SOURCE_FILE_RELATIVE}
-                           DIRECTORY)
+
+    file(RELATIVE_PATH SOURCE_FILE_RELATIVE
+      ${RELATIVE_TO} ${SOURCE_FILE_REALPATH})
+
+    get_filename_component(
+      SOURCE_DIRECTORY_RELATIVE
+      ${SOURCE_FILE_RELATIVE}
+      PATH)
+
     set(SOURCE_GROUP "${GROUP_NAME}\\${SOURCE_DIRECTORY_RELATIVE}")
 
     # This is necessary because source group names require '\' rather than '/'
@@ -91,7 +96,8 @@ function(GROUP_SOURCE GROUP_NAME ROOT_DIRECTORY RELATIVE_TO SOURCE_PATTERN)
     get_filename_component(
       SOURCE_DIRECTORY
       "${CMAKE_CURRENT_SOURCE_DIR}/${SOURCE_FILE}"
-      DIRECTORY)
+      PATH)
+
     file(
       GLOB
       SOURCE_GROUP_FILES


[5/5] mesos git commit: Windows: Prevented crash if sandbox `stdout` file already exists.

Posted by jo...@apache.org.
Windows: Prevented crash if sandbox `stdout` file already exists.

Subprocess currently provides a cross-platform API that allows the child
process to redirect `stdin`, `stdout`, and `stderr`, with support for
file paths, file descriptors, pipes, or Windows file `HANDLE`s.

Currently, in the case of file paths, the Windows code will use
`CreateFile` to open the file, with the semantics that it will error out
if the file already exists (because of the `CREATE_NEW` flag), and
explicitly specifying that it is ok to have multiple processes writing
to the file (because of the `FILE_SHARE_WRITE` flag).

The former of these is undesirable; libprocess should be able to proceed
regardless of whether the file already exists. But this also causes bugs
to the downstream, namely Mesos, in which a Fetcher might create the
sandbox `stdout` folder, and then subsequently crash when it tries to
open the `stdout` folder in the executor.

In this commit, we change the `CREATE_NEW` flag to `OPEN_ALWAYS`, which
has the semantics that we will create the file if it does not exist, and
open it if it does.

Additionally, since the documentation does not specify whether
`FILE_SHARE_WRITE` also allows other processes to have read access, we
additionally specify the flag `FILE_SHARE_READ` just to be sure we make
no assumptions about who is able to read and write to these files. See
comments for additional context on this point.

Review: https://reviews.apache.org/r/56689/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/de89ac84
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/de89ac84
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/de89ac84

Branch: refs/heads/master
Commit: de89ac84523eb254164acd81cfeb8797439a16fa
Parents: 7bd4349
Author: Alex Clemmer <cl...@gmail.com>
Authored: Thu Mar 9 12:25:49 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Mar 9 13:29:02 2017 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/subprocess_windows.cpp | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/de89ac84/3rdparty/libprocess/src/subprocess_windows.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess_windows.cpp b/3rdparty/libprocess/src/subprocess_windows.cpp
index 1b5ae5e..839212b 100644
--- a/3rdparty/libprocess/src/subprocess_windows.cpp
+++ b/3rdparty/libprocess/src/subprocess_windows.cpp
@@ -106,17 +106,30 @@ static Try<HANDLE> getHandleFromFileDescriptor(
 }
 
 
+// Creates a file for a subprocess's stdin, stdout, or stderr.
+//
+// NOTE: For portability, Libprocess implements POSIX-style semantics for these
+// files, and make no assumptions about who has access to them. For example, we
+// do not enforce a process-level write lock on stdin, and we do not enforce a
+// similar read lock from stdout.
+//
 // TODO(hausdorff): Rethink name here, write a comment about this function.
 static Try<HANDLE> createIoPath(const string& path, DWORD accessFlags)
 {
-  // The `TRUE` in the last field makes this duplicate handle inheritable.
+  // Per function comment, the flags `FILE_SHARE_READ`, `FILE_SHARE_WRITE`, and
+  // `OPEN_ALWAYS` are all important. The former two make sure we do not
+  // acquire a process-level lock on reading/writing the file, and the last one
+  // ensures that we open the file if it exists, and create it if it does not.
+  // Note that we specify both `FILE_SHARE_READ` and `FILE_SHARE_WRITE` because
+  // the documentation is not clear about whether `FILE_SHARE_WRITE` also
+  // ensures we don't take a read lock out.
   SECURITY_ATTRIBUTES sa = { sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE };
   const HANDLE handle = ::CreateFile(
       path.c_str(),
       accessFlags,
-      FILE_SHARE_READ,
+      FILE_SHARE_READ | FILE_SHARE_WRITE,
       &sa,
-      CREATE_NEW,
+      OPEN_ALWAYS,
       FILE_ATTRIBUTE_NORMAL,
       nullptr);
 


[2/5] mesos git commit: CMake: Removed unused FindCurl.cmake file.

Posted by jo...@apache.org.
CMake: Removed unused FindCurl.cmake file.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a95abd0e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a95abd0e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a95abd0e

Branch: refs/heads/master
Commit: a95abd0e61e37c4db6d84616c0fef51c9fa201fe
Parents: 5d1df5d
Author: Joseph Wu <jo...@apache.org>
Authored: Thu Mar 2 14:04:53 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Mar 9 11:14:29 2017 -0800

----------------------------------------------------------------------
 src/slave/cmake/FindCurl.cmake | 103 ------------------------------------
 1 file changed, 103 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a95abd0e/src/slave/cmake/FindCurl.cmake
----------------------------------------------------------------------
diff --git a/src/slave/cmake/FindCurl.cmake b/src/slave/cmake/FindCurl.cmake
deleted file mode 100644
index 63818d3..0000000
--- a/src/slave/cmake/FindCurl.cmake
+++ /dev/null
@@ -1,103 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-###############################################################
-# **NOTE:** I (darroyo) have copied a *lot* of stuff in this
-# file from code I found at in the FindAPR.cmake[1] project. The
-# original version[2] of this file was also released under the
-# Apache 2.0 license, and licensed to the ASF, making it fully
-# compatible with the Mesos project.
-#  [1] 3rdparty/stout/cmake/FindApr.cmake
-#  [2] https://github.com/akumuli/Akumuli/blob/master/cmake/FindAPR.cmake
-
-# This module helps to find the cURL library.
-#
-# USAGE: to use this module, add the following line to your CMake project:
-#   find_package(Curl)
-#
-# To make linking to cURL simple, this module defines:
-#   * CURL_FOUND            (if false, linking build will fail)
-#   * CURL_INCLUDE_DIR      (defineswhere to find apr.h, etc.)
-#   * CURL_LIBS             (binaries for cURL and friends)
-#
-# Also defined, but not for general use are:
-#   * CURL_LIB        (where to find the cURL library)
-
-# CONFIGURE THE CURL SEARCH. Specify what we're looking for, and which
-# directories we're going to look through to find them.
-#############################################################################
-set(POSSIBLE_CURL_INCLUDE_DIRS
-  ${POSSIBLE_CURL_INCLUDE_DIRS}
-  /usr/include/curl
-  )
-
-find_path(CURL_INCLUDE_DIR curl.h ${POSSIBLE_CURL_INCLUDE_DIRS})
-
-# SEARCH FOR CURL LIBRARIES.
-############################
-set(POSSIBLE_CURL_LIB_DIRS
-  ${POSSIBLE_CURL_LIB_DIRS}
-  /usr/lib
-  /usr/lib/x86_64-linux-gnu
-  /usr/local/lib
-  )
-
-find_library(
-  CURL_LIB
-  NAMES curl
-  PATHS ${POSSIBLE_CURL_LIB_DIRS}
-  )
-
-# Did we find the include directory?
-string(
-  COMPARE NOTEQUAL
-  "CURL_INCLUDE_DIR-NOTFOUND"
-  ${CURL_INCLUDE_DIR} # Value set to CURL_INCLUDE_DIR-NOTFOUND if not found.
-  CURL_INCLUDE_DIR_FOUND
-  )
-
-# Did we find the library?
-string(
-  COMPARE NOTEQUAL
-  "CURL_LIB-NOTFOUND"
-  ${CURL_LIB} # Value set to `CURL_LIB-NOTFOUND` if not found.
-  CURL_LIB_FOUND
-  )
-
-# cURL is considered "found" if we've both an include directory and a cURL
-# binary.
-if ("${CURL_LIB_FOUND}" AND "${CURL_INCLUDE_DIR_FOUND}")
-  set(CURL_LIBS ${CURL_LIB})
-  set(CURL_FOUND 1)
-else ("${CURL_LIB_FOUND}" AND "${CURL_INCLUDE_DIR_FOUND}")
-  set(CURL_FOUND 0)
-endif ("${CURL_LIB_FOUND}" AND "${CURL_INCLUDE_DIR_FOUND}")
-
-# Results.
-if (CURL_FOUND)
-  if (NOT Curl_FIND_QUIETLY)
-    message(STATUS "Found cURL headers: ${CURL_INCLUDE_DIR}")
-    message(STATUS "Found cURL library: ${CURL_LIBS}")
-  endif (NOT Curl_FIND_QUIETLY)
-else (CURL_FOUND)
-  if (Curl_FIND_REQUIRED)
-    message(FATAL_ERROR "Could not find cURL library")
-  endif (Curl_FIND_REQUIRED)
-endif (CURL_FOUND)
-
-# (Deprecated declarations.)
-set(NATIVE_CURL_INCLUDE_PATH ${CURL_INCLUDE_DIR})
-get_filename_component(NATIVE_CURL_LIB_PATH ${CURL_LIB} PATH)


[3/5] mesos git commit: CMake: Normalized some function capitalization.

Posted by jo...@apache.org.
CMake: Normalized some function capitalization.

CMake isn't case sensitive about many of its functions,
but our codebase generally uses lowercase when calling
built-in CMake functions.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5d1df5d9
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5d1df5d9
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5d1df5d9

Branch: refs/heads/master
Commit: 5d1df5d98e9faea542ca95f51fe1a3ccc2da6457
Parents: eb08f08
Author: Joseph Wu <jo...@apache.org>
Authored: Thu Mar 2 14:03:50 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Mar 9 11:14:29 2017 -0800

----------------------------------------------------------------------
 CMakeLists.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5d1df5d9/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8e492cf..0d1e17b 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -127,7 +127,7 @@ option(CPACK_BINARY_TGZ          "Enable to build TGZ packages"             OFF)
 option(CPACK_BINARY_TXZ          "Enable to build TXZ packages"             OFF)
 
 # Output a gzip'd tarball for the `package_source` target.
-SET(CPACK_SOURCE_TGZ ON)
+set(CPACK_SOURCE_TGZ ON)
 
 # By default, other forms of source packages are disabled.
 option(CPACK_SOURCE_TBZ2 "Enable to build TBZ2 source packages" OFF)
@@ -135,4 +135,4 @@ option(CPACK_SOURCE_TXZ  "Enable to build TXZ source packages"  OFF)
 option(CPACK_SOURCE_TZ   "Enable to build TZ source packages"   OFF)
 option(CPACK_SOURCE_ZIP  "Enable to build ZIP source packages"  OFF)
 
-INCLUDE(CPack)
+include(CPack)


[4/5] mesos git commit: Windows: Reworked error messages in subprocess.

Posted by jo...@apache.org.
Windows: Reworked error messages in subprocess.

This commit clarifies a few error messages when launching subprocesses
on Windows (via libprocess).  The most substantial change is the
addition of the `command` string to each of the possible error messages.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7bd4349d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7bd4349d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7bd4349d

Branch: refs/heads/master
Commit: 7bd4349d48704782e1fecac81aa7ff9e09774610
Parents: 0eee031
Author: Joseph Wu <jo...@apache.org>
Authored: Mon Mar 6 11:03:32 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Mar 9 13:29:01 2017 -0800

----------------------------------------------------------------------
 .../include/process/windows/subprocess.hpp           | 15 ++++++---------
 3rdparty/libprocess/src/subprocess.cpp               |  2 +-
 2 files changed, 7 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7bd4349d/3rdparty/libprocess/include/process/windows/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/windows/subprocess.hpp b/3rdparty/libprocess/include/process/windows/subprocess.hpp
index 0e652cc..1d93b08 100644
--- a/3rdparty/libprocess/include/process/windows/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/windows/subprocess.hpp
@@ -207,7 +207,8 @@ inline Try<PROCESS_INFORMATION> createChildProcess(
       &processInfo);           // PROCESS_INFORMATION pointer.
 
   if (!createProcessResult) {
-    return WindowsError("createChildProcess: failed to call 'CreateProcess'");
+    return WindowsError(
+        "Failed to call CreateProcess on command '" + command + "'");
   }
 
   // Run the parent hooks.
@@ -218,10 +219,6 @@ inline Try<PROCESS_INFORMATION> createChildProcess(
     // If the hook callback fails, we shouldn't proceed with the
     // execution and hence the child process should be killed.
     if (parentSetup.isError()) {
-      LOG(WARNING)
-        << "Failed to execute Subprocess::ParentHook in parent for child '"
-        << pid << "': " << parentSetup.error();
-
       // Attempt to kill the process. Since it is still in suspended state, we
       // do not need to kill any descendents. We also can't use `os::kill_job`
       // because this process is not in a Job Object unless one of the parent
@@ -229,15 +226,15 @@ inline Try<PROCESS_INFORMATION> createChildProcess(
       ::TerminateProcess(processInfo.hProcess, 1);
 
       return Error(
-          "Failed to execute Subprocess::ParentHook in parent for child '" +
-          stringify(pid) + "': " + parentSetup.error());
+          "Failed to execute Parent Hook in child '" + stringify(pid) +
+          "' with command '" + command + "': " + parentSetup.error());
     }
   }
 
   // Start child process.
   if (::ResumeThread(processInfo.hThread) == -1) {
-    return WindowsError("process::createChildProcess: Could not spawn child "
-                        "process");
+    return WindowsError(
+        "Failed to resume child process with command '" + command + "'");
   }
 
   return processInfo;

http://git-wip-us.apache.org/repos/asf/mesos/blob/7bd4349d/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index bc1b99b..6dfb939 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -411,7 +411,7 @@ Try<Subprocess> subprocess(
     if (processInformation.isError()) {
       process::internal::close(stdinfds, stdoutfds, stderrfds);
       return Error(
-          "Could not launch child process" + processInformation.error());
+          "Could not launch child process: " + processInformation.error());
     }
 
     if (processInformation.get().dwProcessId == -1) {