You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by nn...@apache.org on 2014/10/16 21:24:01 UTC
git commit: Added "name" to Modules protobuf.
Repository: mesos
Updated Branches:
refs/heads/master 0796c2b6a -> 18c929b42
Added "name" to Modules protobuf.
Allow the users to specify a bare library names.
For example, "foo-2.3" will expand to "libfoo-2.3.so" on Linux and
"libfoo-2.3.dylib" on OS X.
Review: https://reviews.apache.org/r/26674
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/18c929b4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/18c929b4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/18c929b4
Branch: refs/heads/master
Commit: 18c929b42399c20d90e4d1c858a0aece8460e2c0
Parents: 0796c2b
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Thu Oct 16 11:48:37 2014 -0700
Committer: Niklas Q. Nielsen <ni...@mesosphere.io>
Committed: Thu Oct 16 11:58:02 2014 -0700
----------------------------------------------------------------------
src/master/flags.hpp | 6 ++++
src/messages/messages.proto | 6 +++-
src/module/manager.cpp | 14 ++++++---
src/module/manager.hpp | 10 +++++++
src/slave/flags.hpp | 6 ++++
src/tests/module_tests.cpp | 61 +++++++++++++++++++++++++---------------
6 files changed, 75 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/master/flags.hpp
----------------------------------------------------------------------
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index c41af1f..9d06856 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -321,6 +321,12 @@ public:
" \"org_apache_mesos_bar\",\n"
" \"org_apache_mesos_baz\"\n"
" ]\n"
+ " },\n"
+ " {\n"
+ " \"name\": \"qux\",\n"
+ " \"modules\": [\n"
+ " \"org_apache_mesos_norf\",\n"
+ " ]\n"
" }\n"
" ]\n"
"}");
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index 8de7f96..0dfc1b7 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -418,7 +418,11 @@ message Modules {
// search is performed.
optional string file = 1;
- repeated string modules = 2;
+ // We will add the proper prefix ("lib") and suffix (".so" for
+ // Linux and ".dylib" for OS X) to the "name".
+ optional string name = 2;
+
+ repeated string modules = 3;
}
repeated Library libraries = 1;
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/module/manager.cpp
----------------------------------------------------------------------
diff --git a/src/module/manager.cpp b/src/module/manager.cpp
index 8cc7995..fdf0d33 100644
--- a/src/module/manager.cpp
+++ b/src/module/manager.cpp
@@ -169,13 +169,19 @@ Try<Nothing> ModuleManager::load(const Modules& modules)
initialize();
foreach (const Modules::Library& library, modules.libraries()) {
- if (!library.has_file()) {
- return Error("Library path not provided");
+ string libraryName;
+ if (library.has_file()) {
+ libraryName = library.file();
+ } else if (library.has_name()) {
+ libraryName = expandLibraryName(library.name());
+ } else {
+ return Error("Library name or path not provided");
}
+
Owned<DynamicLibrary> dynamicLibrary(new DynamicLibrary());
- Try<Nothing> result = dynamicLibrary->open(library.file());
+ Try<Nothing> result = dynamicLibrary->open(libraryName);
if (!result.isSome()) {
- return Error("Error opening library: '" + library.file() + "'");
+ return Error("Error opening library: '" + libraryName + "'");
}
// Currently we never delete the DynamicLibrary instance nor do we
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/module/manager.hpp
----------------------------------------------------------------------
diff --git a/src/module/manager.hpp b/src/module/manager.hpp
index 797728a..ecf98ad 100644
--- a/src/module/manager.hpp
+++ b/src/module/manager.hpp
@@ -89,6 +89,16 @@ public:
return kind;
}
+ static std::string expandLibraryName(const std::string& libraryName)
+ {
+#ifdef __linux__
+ const char* libraryExtension = ".so";
+#else
+ const char* libraryExtension = ".dylib";
+#endif
+ return "lib" + libraryName + libraryExtension;
+ }
+
// Exposed just for testing so that we can close all open dynamic
// libraries.
static void unloadAll();
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 159d4ef..03c62a2 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -348,6 +348,12 @@ public:
" \"org_apache_mesos_bar\",\n"
" \"org_apache_mesos_baz\"\n"
" ]\n"
+ " },\n"
+ " {\n"
+ " \"name\": \"qux\",\n"
+ " \"modules\": [\n"
+ " \"org_apache_mesos_norf\",\n"
+ " ]\n"
" }\n"
" ]\n"
"}");
http://git-wip-us.apache.org/repos/asf/mesos/blob/18c929b4/src/tests/module_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/module_tests.cpp b/src/tests/module_tests.cpp
index 71e7ef9..718fb7a 100644
--- a/src/tests/module_tests.cpp
+++ b/src/tests/module_tests.cpp
@@ -16,24 +16,18 @@
* limitations under the License.
*/
+#include <mesos/module.hpp>
+
#include <stout/dynamiclibrary.hpp>
#include <stout/os.hpp>
-#include <examples/test_module.hpp>
+#include "examples/test_module.hpp"
-#include <mesos/module.hpp>
-
-#include <module/manager.hpp>
+#include "module/manager.hpp"
#include "tests/flags.hpp"
#include "tests/mesos.hpp"
-#ifdef __linux__
-const char* libraryExtension = ".so";
-#else
-const char* libraryExtension = ".dylib";
-#endif
-
using std::string;
using namespace mesos;
@@ -43,6 +37,7 @@ using namespace mesos::internal::tests;
class ModuleTest : public MesosTest {};
+
static const string getLibraryDirectory()
{
return path::join(tests::flags.build_dir, "src", ".libs");
@@ -53,7 +48,7 @@ static const string getLibraryPath(const string& libraryName)
{
return path::join(
getLibraryDirectory(),
- "lib" + libraryName + libraryExtension);
+ ModuleManager::expandLibraryName(libraryName));
}
@@ -115,11 +110,39 @@ TEST_F(ModuleTest, AuthorInfoTest)
}
+// Test that a module library gets loaded when provided with a
+// library name without any extension and without the "lib" prefix.
+TEST_F(ModuleTest, LibraryNameWithoutExtension)
+{
+ const string libraryName = "examplemodule";
+ const string moduleName = "org_apache_mesos_TestModule";
+ const string ldLibraryPath = "LD_LIBRARY_PATH";
+
+ // Append our library path to LD_LIBRARY_PATH.
+ const string oldLdLibraryPath = os::getenv(ldLibraryPath, false);
+ const string newLdLibraryPath =
+ getLibraryDirectory() + ":" + oldLdLibraryPath;
+ os::setenv(ldLibraryPath, newLdLibraryPath);
+
+ Modules modules;
+ Modules::Library* library = modules.add_libraries();
+ library->set_name(libraryName);
+ library->add_modules(moduleName);
+
+ EXPECT_SOME(ModuleManager::load(modules));
+
+ // Reset LD_LIBRARY_PATH environment variable.
+ os::setenv(ldLibraryPath, oldLdLibraryPath);
+
+ ModuleManager::unloadAll();
+}
+
+
// Test that a module library gets loaded with just library name if
// found in LD_LIBRARY_PATH.
-TEST_F(ModuleTest, NoAbsoluteLibraryPath)
+TEST_F(ModuleTest, LibraryNameWithExtension)
{
- const string libraryName = stringify("libexamplemodule") + libraryExtension;
+ const string libraryName = ModuleManager::expandLibraryName("examplemodule");
const string moduleName = "org_apache_mesos_TestModule";
const string ldLibraryPath = "LD_LIBRARY_PATH";
@@ -136,17 +159,9 @@ TEST_F(ModuleTest, NoAbsoluteLibraryPath)
EXPECT_SOME(ModuleManager::load(modules));
- Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName);
- EXPECT_SOME(module);
-
- // The TestModuleImpl module's implementation of foo() returns
- // the sum of the passed arguments, whereas bar() returns the
- // product.
- EXPECT_EQ(module.get()->foo('A', 1024), 1089);
- EXPECT_EQ(module.get()->bar(0.5, 10.8), 5);
-
- // reset LD_LIBRARY_PATH environment variable.
+ // Reset LD_LIBRARY_PATH environment variable.
os::setenv(ldLibraryPath, oldLdLibraryPath);
+
ModuleManager::unloadAll();
}