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/11 00:32:31 UTC
git commit: Changed modules to accept relative path or file name for
library name.
Repository: mesos
Updated Branches:
refs/heads/master 29a82c97c -> e8e8adf7e
Changed modules to accept relative path or file name for library name.
Rename "path" in Modules protobuf to "file" since a "path" always
referes to an absolute or a relative path, whereas "file" may refer to a
path or a file name. If "file" contains a slash ("/"), it is considered
an absolute or relative path, otherwise if is considered a file name.
For file name, dlopen() automatically does a standard library path
search to find the absolute path.
Also, return a better error message if an empty module name is provided.
Review: https://reviews.apache.org/r/26529
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e8e8adf7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e8e8adf7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e8e8adf7
Branch: refs/heads/master
Commit: e8e8adf7e277e1b5c8eed90d505d32b875315130
Parents: 29a82c9
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Fri Oct 10 15:07:51 2014 -0700
Committer: Niklas Q. Nielsen <ni...@mesosphere.io>
Committed: Fri Oct 10 15:29:31 2014 -0700
----------------------------------------------------------------------
include/mesos/module.hpp | 5 ++-
src/messages/messages.proto | 9 ++--
src/module/manager.cpp | 12 ++++--
src/tests/module_tests.cpp | 92 ++++++++++++++++++++++++++++++++++------
4 files changed, 95 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e8e8adf7/include/mesos/module.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/module.hpp b/include/mesos/module.hpp
index 733b150..5205e30 100644
--- a/include/mesos/module.hpp
+++ b/include/mesos/module.hpp
@@ -27,8 +27,9 @@
// takes a list of libraries with contained modules as command line
// argument (in JSON format or a path to a JSON file).
//
-// JSON := [library, ...]
-// library := {"path": <library path>, "modules": [<module name>, ...]}
+// JSON := {"libraries": [library, ...]}
+// library := {"file": <library path>,
+// "modules": [<module name>, ...]}
//
// How to write a module library:
// 1. Define a create() function that returns a pointer to an object
http://git-wip-us.apache.org/repos/asf/mesos/blob/e8e8adf7/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index 77515d9..8de7f96 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -413,11 +413,10 @@ message TaskHealthStatus {
// Collection of Modules.
message Modules {
message Library {
- // TODO(karya): Add a "name" field that allows specifying library
- // name instead of an absolute path.
- //optional string name = 1;
-
- optional string path = 1;
+ // If "file" contains a slash ("/"), then it is interpreted as a
+ // (relative or absolute) pathname. Otherwise a standard library
+ // search is performed.
+ optional string file = 1;
repeated string modules = 2;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e8e8adf7/src/module/manager.cpp
----------------------------------------------------------------------
diff --git a/src/module/manager.cpp b/src/module/manager.cpp
index 265f934..8cc7995 100644
--- a/src/module/manager.cpp
+++ b/src/module/manager.cpp
@@ -169,14 +169,13 @@ Try<Nothing> ModuleManager::load(const Modules& modules)
initialize();
foreach (const Modules::Library& library, modules.libraries()) {
- if (!library.has_path()) {
+ if (!library.has_file()) {
return Error("Library path not provided");
}
-
Owned<DynamicLibrary> dynamicLibrary(new DynamicLibrary());
- Try<Nothing> result = dynamicLibrary->open(library.path());
+ Try<Nothing> result = dynamicLibrary->open(library.file());
if (!result.isSome()) {
- return Error("Error opening library: '" + library.path() + "'");
+ return Error("Error opening library: '" + library.file() + "'");
}
// Currently we never delete the DynamicLibrary instance nor do we
@@ -188,6 +187,11 @@ Try<Nothing> ModuleManager::load(const Modules& modules)
// Load module manifests.
foreach (const string& moduleName, library.modules()) {
+ if (moduleName.empty()) {
+ return Error(
+ "Error: module name not provided with library '" +
+ library.file() + "'");
+ }
// Check for possible duplicate module names.
if (moduleBases.contains(moduleName)) {
return Error("Error loading duplicate module '" + moduleName + "'");
http://git-wip-us.apache.org/repos/asf/mesos/blob/e8e8adf7/src/tests/module_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/module_tests.cpp b/src/tests/module_tests.cpp
index 5a902e6..71e7ef9 100644
--- a/src/tests/module_tests.cpp
+++ b/src/tests/module_tests.cpp
@@ -17,6 +17,7 @@
*/
#include <stout/dynamiclibrary.hpp>
+#include <stout/os.hpp>
#include <examples/test_module.hpp>
@@ -27,6 +28,12 @@
#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;
@@ -36,19 +43,17 @@ using namespace mesos::internal::tests;
class ModuleTest : public MesosTest {};
+static const string getLibraryDirectory()
+{
+ return path::join(tests::flags.build_dir, "src", ".libs");
+}
+
+
static const string getLibraryPath(const string& libraryName)
{
return path::join(
- tests::flags.build_dir,
- "src",
- ".libs",
- "lib" + libraryName +
-#ifdef __linux__
- ".so"
-#else
- ".dylib"
-#endif
- );
+ getLibraryDirectory(),
+ "lib" + libraryName + libraryExtension);
}
@@ -56,7 +61,7 @@ static Modules getModules(const string& libraryName, const string& moduleName)
{
Modules modules;
Modules::Library* library = modules.add_libraries();
- library->set_path(getLibraryPath(libraryName));
+ library->set_file(getLibraryPath(libraryName));
library->add_modules(moduleName);
return modules;
}
@@ -110,6 +115,69 @@ TEST_F(ModuleTest, AuthorInfoTest)
}
+// Test that a module library gets loaded with just library name if
+// found in LD_LIBRARY_PATH.
+TEST_F(ModuleTest, NoAbsoluteLibraryPath)
+{
+ const string libraryName = stringify("libexamplemodule") + libraryExtension;
+ 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_file(libraryName);
+ library->add_modules(moduleName);
+
+ 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.
+ os::setenv(ldLibraryPath, oldLdLibraryPath);
+ ModuleManager::unloadAll();
+}
+
+
+// Test that module library loading fails when filename is empty.
+TEST_F(ModuleTest, EmptyLibraryFilename)
+{
+ const string libraryName = "";
+ const string moduleName = "org_apache_mesos_TestModule";
+
+ Modules modules = getModules(libraryName, moduleName);
+
+ EXPECT_ERROR(ModuleManager::load(modules));
+
+ ModuleManager::unloadAll();
+}
+
+
+// Test that module library loading fails when module name is empty.
+TEST_F(ModuleTest, EmptyModuleName)
+{
+ const string libraryName = "examplemodule";
+ const string moduleName = "";
+
+ Modules modules = getModules(libraryName, moduleName);
+ EXPECT_ERROR(ModuleManager::load(modules));
+
+ ModuleManager::unloadAll();
+}
+
+
// Test that module library loading fails when given an unknown path.
TEST_F(ModuleTest, UnknownLibraryTest)
{
@@ -177,7 +245,7 @@ TEST_F(ModuleTest, DuplicateModule)
// Add duplicate module.
Modules::Library* library = modules.add_libraries();
- library->set_path(getLibraryPath(libraryName));
+ library->set_file(getLibraryPath(libraryName));
library->add_modules(moduleName);
EXPECT_ERROR(ModuleManager::load(modules));