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 <[email protected]> Authored: Thu Oct 16 11:48:37 2014 -0700 Committer: Niklas Q. Nielsen <[email protected]> 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(); }
