| Message ID | 20190419135036.19340-5-ykaradzhov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Various modifications and fixes toward KS 1.0 | expand |
On 19.04.19 г. 20:28 ч., Steven Rostedt wrote: > On Fri, 19 Apr 2019 16:50:32 +0300 > Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > >> +char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n) >> +{ >> + QString path = QCoreApplication::applicationFilePath(); >> + std::string pluginStr = plugin.toStdString(); >> + char *lib; >> + >> + if (path.contains(KS_DIR)) { > > I'd rather not use the hardcoded path. If I build the code on one > machine, tarball it up and move it to another machine and extract it, > and then run that code from that machine, I want it to still use the > plugins for that machine. > > I was hoping to test: > > string = cmdline_path() + "../../kernel-shark/lib/"; > > If that exists, then we know that we are in the source directory. I don't thing this is a good idea. If we search for plugins in a path that is defined like this: cmdline_path() + "/something/hard/coded/lib/" Then the GUI will do one thing when started like this: ./kernelshark and anther thing when started like this: bin/kernelshark and this can be very surprising behavior for the user The other solution has it own weaknesses, but at least it sounds like a simple rule: "If you want to use the compiled version of the plugins you have to start the GUI from the source code directory used to build. Otherwise the installed version of the plugins will be used." Thanks! Yordan > > -- Steve > >> + n = asprintf(&lib, "%s/lib/plugin-%s.so", >> + KS_DIR, pluginStr.c_str()); >> + } else { >> + n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so", >> + _INSTALL_PREFIX, pluginStr.c_str()); >> + } >> + >> + return lib; >> +} >> +
On Mon, 22 Apr 2019 14:29:49 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > > I was hoping to test: > > > > string = cmdline_path() + "../../kernel-shark/lib/"; > > > > If that exists, then we know that we are in the source directory. > > I don't thing this is a good idea. If we search for plugins in a path > that is defined like this: > cmdline_path() + "/something/hard/coded/lib/" Your missing the "../.." part. > > Then the GUI will do one thing when started like this: > ./kernelshark > > and anther thing when started like this: > bin/kernelshark No it shouldn't, unless you moved the binary. I said to add "../../plugins" to the path that the kernelshark binary is executed from. If you were to move kernelshark, then yes. it would not longer give you the same result. If the binary is in: kernelshark/bin/kernelshark and you ran it as: kernelshark/bin/kernelshark using the path "kernelshark/bin" and adding "../lib" would give you "kernelshark/lib" directory. "kernelshark/bin/../lib" == "kernelshark/lib" Also, if you were to cd to kernelshark and run "bin/kernelshark" the path would be "bin/../lib" which would be equal to "lib" and being in the kernelshark directory, would give you the plugins directory that is the same as the previous command. If you cd to kernelshark/bin and ran "./kernelshark" we would then use "./../lib" which is the same as "../lib" which is still the same path as the other two. But last call we discussed a way to find the full path name of the binary being executed. And if that's the case, we could not only add "../lib" we could also check that the binary being executed is also in a "kernelshark/bin" first. > > and this can be very surprising behavior for the user > > The other solution has it own weaknesses, but at least it sounds like a > simple rule: "If you want to use the compiled version of the plugins you > have to start the GUI from the source code directory used to build. > Otherwise the installed version of the plugins will be used." Building from source, then moving that source tree to another path shouldn't cause the binary to act differently. -- Steve
On 22.04.19 г. 14:50 ч., Steven Rostedt wrote: > On Mon, 22 Apr 2019 14:29:49 +0300 > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > > >>> I was hoping to test: >>> >>> string = cmdline_path() + "../../kernel-shark/lib/"; >>> >>> If that exists, then we know that we are in the source directory. >> >> I don't thing this is a good idea. If we search for plugins in a path >> that is defined like this: >> cmdline_path() + "/something/hard/coded/lib/" > > Your missing the "../.." part. > >> >> Then the GUI will do one thing when started like this: >> ./kernelshark >> >> and anther thing when started like this: >> bin/kernelshark > > No it shouldn't, unless you moved the binary. > > I said to add "../../plugins" to the path that the kernelshark binary > is executed from. If you were to move kernelshark, then yes. it would > not longer give you the same result. > > If the binary is in: > > kernelshark/bin/kernelshark > > and you ran it as: kernelshark/bin/kernelshark using the path > "kernelshark/bin" and adding "../lib" would give you "kernelshark/lib" > directory. > > "kernelshark/bin/../lib" == "kernelshark/lib" > > Also, if you were to cd to kernelshark and run > > "bin/kernelshark" the path would be "bin/../lib" which would be > equal to "lib" and being in the kernelshark directory, would > give you the plugins directory that is the same as the previous command. > > If you cd to kernelshark/bin and ran "./kernelshark" we would then use > "./../lib" which is the same as "../lib" which is still the same path > as the other two. > > But last call we discussed a way to find the full path name of the > binary being executed. And if that's the case, we could not only add > "../lib" we could also check that the binary being executed is also in > a "kernelshark/bin" first. > OK, now I think I understand what you want. Thanks! Yordan >> >> and this can be very surprising behavior for the user >> >> The other solution has it own weaknesses, but at least it sounds like a >> simple rule: "If you want to use the compiled version of the plugins you >> have to start the GUI from the source code directory used to build. >> Otherwise the installed version of the plugins will be used." > > Building from source, then moving that source tree to another path > shouldn't cause the binary to act differently. > > -- Steve >
diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake index 8041cfc..ba211f4 100644 --- a/kernel-shark/build/deff.h.cmake +++ b/kernel-shark/build/deff.h.cmake @@ -14,6 +14,9 @@ /** KernelShark source code path. */ #cmakedefine KS_DIR "@KS_DIR@" +/** KernelShark installation prefix path. */ +#cmakedefine _INSTALL_PREFIX "@_INSTALL_PREFIX@" + /** Location of the trace-cmd executable. */ #cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@" diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp index 6af0c66..b05c0dc 100644 --- a/kernel-shark/src/KsUtils.cpp +++ b/kernel-shark/src/KsUtils.cpp @@ -423,13 +423,12 @@ void KsPluginManager::_parsePluginList() */ void KsPluginManager::registerFromList(kshark_context *kshark_ctx) { - auto lamRegBuiltIn = [&kshark_ctx](const QString &plugin) + auto lamRegBuiltIn = [&kshark_ctx, this](const QString &plugin) { char *lib; int n; - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n <= 0) return; @@ -458,13 +457,12 @@ void KsPluginManager::registerFromList(kshark_context *kshark_ctx) */ void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx) { - auto lamUregBuiltIn = [&kshark_ctx](const QString &plugin) + auto lamUregBuiltIn = [&kshark_ctx, this](const QString &plugin) { char *lib; int n; - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n <= 0) return; @@ -487,6 +485,23 @@ void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx) lamUregUser); } +char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n) +{ + QString path = QCoreApplication::applicationFilePath(); + std::string pluginStr = plugin.toStdString(); + char *lib; + + if (path.contains(KS_DIR)) { + n = asprintf(&lib, "%s/lib/plugin-%s.so", + KS_DIR, pluginStr.c_str()); + } else { + n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so", + _INSTALL_PREFIX, pluginStr.c_str()); + } + + return lib; +} + /** * @brief Register a Plugin. * @@ -508,8 +523,7 @@ void KsPluginManager::registerPlugin(const QString &plugin) * The argument is the name of the plugin. From the * name get the library .so file. */ - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n > 0) { kshark_register_plugin(kshark_ctx, lib); _registeredKsPlugins[i] = true; @@ -570,8 +584,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin) * The argument is the name of the plugin. From the * name get the library .so file. */ - n = asprintf(&lib, "%s/lib/plugin-%s.so", KS_DIR, - plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n > 0) { kshark_unregister_plugin(kshark_ctx, lib); _registeredKsPlugins[i] = false; @@ -579,7 +592,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin) } return; - } else if (plugin.contains("/lib/plugin-" + + } else if (plugin.contains("/lib/plugin-" + _ksPluginList[i], Qt::CaseInsensitive)) { /* * The argument is the name of the library .so file. diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp index 77048ab..019bde8 100644 --- a/kernel-shark/src/KsUtils.hpp +++ b/kernel-shark/src/KsUtils.hpp @@ -238,6 +238,7 @@ signals: private: void _parsePluginList(); + char *_pluginLibFromName(const QString &plugin, int &n); template <class T> void _forEachInList(const QStringList &pl, diff --git a/kernel-shark/src/plugins/CMakeLists.txt b/kernel-shark/src/plugins/CMakeLists.txt index 6098275..64cf98d 100644 --- a/kernel-shark/src/plugins/CMakeLists.txt +++ b/kernel-shark/src/plugins/CMakeLists.txt @@ -29,6 +29,6 @@ BUILD_PLUGIN(NAME missed_events list(APPEND PLUGIN_LIST "missed_events default") # This plugin will be loaded by default install(TARGETS sched_events missed_events - LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/) + LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/plugins/) set(PLUGINS ${PLUGIN_LIST} PARENT_SCOPE)
If the application has been started from the source code directory, all build-in plugins will be loaded from kernel-shark/lib/ If the application has been started from its installation location, all build-in plugins will be loaded from INSTALL_PREFIX/lib/kshark/plugins/ Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> --- kernel-shark/build/deff.h.cmake | 3 +++ kernel-shark/src/KsUtils.cpp | 35 +++++++++++++++++-------- kernel-shark/src/KsUtils.hpp | 1 + kernel-shark/src/plugins/CMakeLists.txt | 2 +- 4 files changed, 29 insertions(+), 12 deletions(-)