* [PATCH] kernel-shark: Do not hardcode /usr prefix for polkit policies @ 2021-03-11 13:12 Michal Sojka 2021-03-11 14:09 ` Steven Rostedt 0 siblings, 1 reply; 11+ messages in thread From: Michal Sojka @ 2021-03-11 13:12 UTC (permalink / raw) To: linux-trace-devel; +Cc: Michal Sojka --- kernel-shark/src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt index 457c100..687e150 100644 --- a/kernel-shark/src/CMakeLists.txt +++ b/kernel-shark/src/CMakeLists.txt @@ -92,7 +92,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND) DESTINATION ${_INSTALL_PREFIX}/share/icons/${KS_APP_NAME}) install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy" - DESTINATION /usr/share/polkit-1/actions/) + DESTINATION ${_INSTALL_PREFIX}/share/polkit-1/actions/) install(PROGRAMS "${KS_DIR}/bin/kshark-su-record" DESTINATION ${_INSTALL_PREFIX}/bin/) -- 2.30.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 13:12 [PATCH] kernel-shark: Do not hardcode /usr prefix for polkit policies Michal Sojka @ 2021-03-11 14:09 ` Steven Rostedt 2021-03-11 14:48 ` Michal Sojka 0 siblings, 1 reply; 11+ messages in thread From: Steven Rostedt @ 2021-03-11 14:09 UTC (permalink / raw) To: Michal Sojka; +Cc: linux-trace-devel On Thu, 11 Mar 2021 14:12:21 +0100 Michal Sojka <michal.sojka@cvut.cz> wrote: Hi Michal, Thanks for the fix! Note, even when submitting an "obvious" patch, it's always good to add some content to the change log body. Basically, why it shouldn't be hard coded. That is, what failed because of it. I can think of a few things, but we try to have content in the change log body for all commits, even obvious ones. The only exception is spelling fixes don't need content. Thanks! -- Steve > --- > kernel-shark/src/CMakeLists.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt > index 457c100..687e150 100644 > --- a/kernel-shark/src/CMakeLists.txt > +++ b/kernel-shark/src/CMakeLists.txt > @@ -92,7 +92,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND) > DESTINATION ${_INSTALL_PREFIX}/share/icons/${KS_APP_NAME}) > > install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy" > - DESTINATION /usr/share/polkit-1/actions/) > + DESTINATION ${_INSTALL_PREFIX}/share/polkit-1/actions/) > > install(PROGRAMS "${KS_DIR}/bin/kshark-su-record" > DESTINATION ${_INSTALL_PREFIX}/bin/) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 14:09 ` Steven Rostedt @ 2021-03-11 14:48 ` Michal Sojka 2021-03-11 14:50 ` [PATCH v2] " Michal Sojka 0 siblings, 1 reply; 11+ messages in thread From: Michal Sojka @ 2021-03-11 14:48 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel Hi Steven, On Thu, Mar 11 2021, Steven Rostedt wrote: > On Thu, 11 Mar 2021 14:12:21 +0100 > Michal Sojka <michal.sojka@cvut.cz> wrote: > > Hi Michal, > > Thanks for the fix! > > Note, even when submitting an "obvious" patch, it's always good to add > some content to the change log body. Understood. Sending v2... -Michal ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 14:48 ` Michal Sojka @ 2021-03-11 14:50 ` Michal Sojka 2021-03-11 14:54 ` Steven Rostedt 2021-03-11 17:01 ` Yordan Karadzhov (VMware) 0 siblings, 2 replies; 11+ messages in thread From: Michal Sojka @ 2021-03-11 14:50 UTC (permalink / raw) To: Michal Sojka, Steven Rostedt; +Cc: linux-trace-devel When one wants to install kernel-shark to a non-standard location, e.g., by configuring it as follows: cmake -D_INSTALL_PREFIX=$HOME .. then "make install" fails, with the following error: CMake Error at src/cmake_install.cmake:225 (file): file INSTALL cannot copy file "/home/user/src/trace-cmd/kernel-shark/org.freedesktop.kshark-record.policy" to "/usr/share/polkit-1/actions/org.freedesktop.kshark-record.policy". This commit fixes that by ensuring that even the org.freedesktop.kshark-record.policy file is installed to the user-specified prefix and not to /usr where the user has no write permission. --- kernel-shark/src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt index 457c100..687e150 100644 --- a/kernel-shark/src/CMakeLists.txt +++ b/kernel-shark/src/CMakeLists.txt @@ -92,7 +92,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND) DESTINATION ${_INSTALL_PREFIX}/share/icons/${KS_APP_NAME}) install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy" - DESTINATION /usr/share/polkit-1/actions/) + DESTINATION ${_INSTALL_PREFIX}/share/polkit-1/actions/) install(PROGRAMS "${KS_DIR}/bin/kshark-su-record" DESTINATION ${_INSTALL_PREFIX}/bin/) -- 2.30.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 14:50 ` [PATCH v2] " Michal Sojka @ 2021-03-11 14:54 ` Steven Rostedt 2021-03-11 17:01 ` Yordan Karadzhov (VMware) 1 sibling, 0 replies; 11+ messages in thread From: Steven Rostedt @ 2021-03-11 14:54 UTC (permalink / raw) To: Michal Sojka; +Cc: linux-trace-devel On Thu, 11 Mar 2021 15:50:59 +0100 Michal Sojka <michal.sojka@cvut.cz> wrote: > When one wants to install kernel-shark to a non-standard location, > e.g., by configuring it as follows: > > cmake -D_INSTALL_PREFIX=$HOME .. > > then "make install" fails, with the following error: > > CMake Error at src/cmake_install.cmake:225 (file): > file INSTALL cannot copy file > "/home/user/src/trace-cmd/kernel-shark/org.freedesktop.kshark-record.policy" > to "/usr/share/polkit-1/actions/org.freedesktop.kshark-record.policy". > > This commit fixes that by ensuring that even the > org.freedesktop.kshark-record.policy file is installed to the > user-specified prefix and not to /usr where the user has no write > permission. Thanks for the updated change log Michal! Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> -- Steve ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 14:50 ` [PATCH v2] " Michal Sojka 2021-03-11 14:54 ` Steven Rostedt @ 2021-03-11 17:01 ` Yordan Karadzhov (VMware) 2021-03-11 17:57 ` Michal Sojka 1 sibling, 1 reply; 11+ messages in thread From: Yordan Karadzhov (VMware) @ 2021-03-11 17:01 UTC (permalink / raw) To: Michal Sojka, Steven Rostedt; +Cc: linux-trace-devel Hi Michal, Thank you very much for helping us improving kernelshark! On 11.03.21 г. 16:50, Michal Sojka wrote: > When one wants to install kernel-shark to a non-standard location, > e.g., by configuring it as follows: > > cmake -D_INSTALL_PREFIX=$HOME .. > > then "make install" fails, with the following error: > > CMake Error at src/cmake_install.cmake:225 (file): > file INSTALL cannot copy file > "/home/user/src/trace-cmd/kernel-shark/org.freedesktop.kshark-record.policy" > to "/usr/share/polkit-1/actions/org.freedesktop.kshark-record.policy". > > This commit fixes that by ensuring that even the > org.freedesktop.kshark-record.policy file is installed to the > user-specified prefix and not to /usr where the user has no write > permission. In your case the installation fails to install the policy file used by Polkit. Note that this doesn't mean that the kernelshark installation itself fails. As far as I know the policy file can only go to a special locations so that Polkit can find it. Otherwise it will have no effect (I may be wrong on this). If you know how to tell Polkit to search in an arbitrary location for those files, please let me know. Thanks! Yordan > --- > kernel-shark/src/CMakeLists.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt > index 457c100..687e150 100644 > --- a/kernel-shark/src/CMakeLists.txt > +++ b/kernel-shark/src/CMakeLists.txt > @@ -92,7 +92,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND) > DESTINATION ${_INSTALL_PREFIX}/share/icons/${KS_APP_NAME}) > > install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy" > - DESTINATION /usr/share/polkit-1/actions/) > + DESTINATION ${_INSTALL_PREFIX}/share/polkit-1/actions/) > > install(PROGRAMS "${KS_DIR}/bin/kshark-su-record" > DESTINATION ${_INSTALL_PREFIX}/bin/) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 17:01 ` Yordan Karadzhov (VMware) @ 2021-03-11 17:57 ` Michal Sojka 2021-03-12 6:46 ` Yordan Karadzhov (VMware) 0 siblings, 1 reply; 11+ messages in thread From: Michal Sojka @ 2021-03-11 17:57 UTC (permalink / raw) To: Yordan Karadzhov (VMware), Steven Rostedt; +Cc: linux-trace-devel Hi Yordan, On Thu, Mar 11 2021, Yordan Karadzhov (VMware) wrote: > In your case the installation fails to install the policy file used by > Polkit. Note that this doesn't mean that the kernelshark installation > itself fails. Yes, I know, but when something fails, users are confused ;-) > As far as I know the policy file can only go to a special locations so > that Polkit can find it. Otherwise it will have no effect (I may be > wrong on this). You're right. I looked at polkit sources and it really seems that only one location is supported. It is determined at configuration time so on some systems it may be different from /usr/share/... I'm talking specifically about NixOS, but it already has the patch I sent. So I leave it up to you whether to apply the patch or not. I think that supporting seamless installation into $HOME is useful if one wants to quickly use a newer version not available in their distribution. BTW, thanks for great tool! -Michal ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-11 17:57 ` Michal Sojka @ 2021-03-12 6:46 ` Yordan Karadzhov (VMware) 2021-03-12 11:47 ` Dario Faggioli 2021-03-17 12:59 ` Michal Sojka 0 siblings, 2 replies; 11+ messages in thread From: Yordan Karadzhov (VMware) @ 2021-03-12 6:46 UTC (permalink / raw) To: Michal Sojka, Steven Rostedt; +Cc: linux-trace-devel Hi Michal, On 11.03.21 г. 19:57, Michal Sojka wrote: > Hi Yordan, > > On Thu, Mar 11 2021, Yordan Karadzhov (VMware) wrote: >> In your case the installation fails to install the policy file used by >> Polkit. Note that this doesn't mean that the kernelshark installation >> itself fails. > > Yes, I know, but when something fails, users are confused ;-) You are right here. We have to think how to make this message look more like a warning. > >> As far as I know the policy file can only go to a special locations so >> that Polkit can find it. Otherwise it will have no effect (I may be >> wrong on this). > > You're right. I looked at polkit sources and it really seems that only > one location is supported. It is determined at configuration time so on > some systems it may be different from /usr/share/... I'm talking > specifically about NixOS, but it already has the patch I sent. > > So I leave it up to you whether to apply the patch or not. I think that > supporting seamless installation into $HOME is useful if one wants to > quickly use a newer version not available in their distribution. Building, installing as root and testing the latest version shouldn't cause any problems/conflicts on your system. Note that there is a script in kernel-shark/build called "cmake_uninstall.sh". It is guaranteed that this script removes every single file that has been installed. And BTW from the patch I see that you still use the old version that comes together with trace-cmd. If you are keen to try the latest version, checkout this one: https://git.kernel.org/pub/scm/utils/trace-cmd/kernel-shark.git/ This is a brand new version that has a lot of changes under the hood and needs user testing. We will be extremely happy to receive bug reports ;) or patches for this new version. Thanks! Yordan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-12 6:46 ` Yordan Karadzhov (VMware) @ 2021-03-12 11:47 ` Dario Faggioli 2021-03-12 13:02 ` Yordan Karadzhov (VMware) 2021-03-17 12:59 ` Michal Sojka 1 sibling, 1 reply; 11+ messages in thread From: Dario Faggioli @ 2021-03-12 11:47 UTC (permalink / raw) To: Yordan Karadzhov (VMware), Michal Sojka, Steven Rostedt; +Cc: linux-trace-devel [-- Attachment #1: Type: text/plain, Size: 2112 bytes --] On Fri, 2021-03-12 at 08:46 +0200, Yordan Karadzhov (VMware) wrote: > Hi Michal, > Hey Michal! Nice to see you here too... well, it's a small world, I guess? :-P > On 11.03.21 г. 19:57, Michal Sojka wrote: > > You're right. I looked at polkit sources and it really seems that > > only > > one location is supported. It is determined at configuration time > > so on > > some systems it may be different from /usr/share/... I'm talking > > specifically about NixOS, but it already has the patch I sent. > > > > So I leave it up to you whether to apply the patch or not. I think > > that > > supporting seamless installation into $HOME is useful if one wants > > to > > quickly use a newer version not available in their distribution. > > Building, installing as root and testing the latest version shouldn't > cause any problems/conflicts on your system. Note that there is a > script > in kernel-shark/build called "cmake_uninstall.sh". It is guaranteed > that > this script removes every single file that has been installed. > Not completely sure, but since NixOS is being mentioned, I think at least part of the point here is making sure that a development/testing version of KS can be installed in those "special" systems that have read-only filesystems (or similar configurations). In such a system, even if you are root, if the makefile tries to put stuff in a part of the fs which is not writable, it's pretty much game-over. In fact, I don't know much about NixOS, but I'm on one of those "immutable" systems myself (openSUSE MicroOS, FTR) so I can relate. :-) So, although it's definitely a niche use-case (for now!! :-P), I think that either this patch, or at least not making the above issue fatal (as you're saying yourself) would make the life of some users easier. Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ ------------------------------------------------------------------- <<This happens because _I_ choose it to happen!>> (Raistlin Majere) [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-12 11:47 ` Dario Faggioli @ 2021-03-12 13:02 ` Yordan Karadzhov (VMware) 0 siblings, 0 replies; 11+ messages in thread From: Yordan Karadzhov (VMware) @ 2021-03-12 13:02 UTC (permalink / raw) To: Dario Faggioli, Michal Sojka, Steven Rostedt; +Cc: linux-trace-devel Hi Dario and Michal, On 12.03.21 г. 13:47, Dario Faggioli wrote: > On Fri, 2021-03-12 at 08:46 +0200, Yordan Karadzhov (VMware) wrote: >> Hi Michal, >> > Hey Michal! Nice to see you here too... well, it's a small world, I > guess? :-P > >> On 11.03.21 г. 19:57, Michal Sojka wrote: >>> You're right. I looked at polkit sources and it really seems that >>> only >>> one location is supported. It is determined at configuration time >>> so on >>> some systems it may be different from /usr/share/... I'm talking >>> specifically about NixOS, but it already has the patch I sent. >>> >>> So I leave it up to you whether to apply the patch or not. I think >>> that >>> supporting seamless installation into $HOME is useful if one wants >>> to >>> quickly use a newer version not available in their distribution. >> >> Building, installing as root and testing the latest version shouldn't >> cause any problems/conflicts on your system. Note that there is a >> script >> in kernel-shark/build called "cmake_uninstall.sh". It is guaranteed >> that >> this script removes every single file that has been installed. >> > Not completely sure, but since NixOS is being mentioned, I think at > least part of the point here is making sure that a development/testing > version of KS can be installed in those "special" systems that have > read-only filesystems (or similar configurations). > > In such a system, even if you are root, if the makefile tries to put > stuff in a part of the fs which is not writable, it's pretty much > game-over. > > In fact, I don't know much about NixOS, but I'm on one of those > "immutable" systems myself (openSUSE MicroOS, FTR) so I can relate. :-) OK, now I see your problem. Please excuse my ignorance. Here's the deal. What you want can be achieved with an almost trivial patch in kernelshark 2. Have a look here: https://git.kernel.org/pub/scm/utils/trace-cmd/kernel-shark.git/tree/src/CMakeLists.txt#n129 and make the installation of the policy file to be a separate component. Make the installation of "kshark-su-record" part of the new component as well since it relies on the policy file. I guess "kernelshark.desktop" must be there as well. Then you can edit the helper script "install_gui.sh" and make it install your new component as well. You can also add another helper script that install only the part you want. Send me a patch and I will take it. Thanks! Yordan > So, although it's definitely a niche use-case (for now!! :-P), I think > that either this patch, or at least not making the above issue fatal > (as you're saying yourself) would make the life of some users easier. > > Regards > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] kernel-shark: Do not hardcode /usr prefix for polkit policies 2021-03-12 6:46 ` Yordan Karadzhov (VMware) 2021-03-12 11:47 ` Dario Faggioli @ 2021-03-17 12:59 ` Michal Sojka 1 sibling, 0 replies; 11+ messages in thread From: Michal Sojka @ 2021-03-17 12:59 UTC (permalink / raw) To: Yordan Karadzhov (VMware), Steven Rostedt, Dario Faggioli Cc: linux-trace-devel Hi Yordan, On Fri, Mar 12 2021, Yordan Karadzhov (VMware) wrote: > If you are keen to try the latest version, checkout this one: > > https://git.kernel.org/pub/scm/utils/trace-cmd/kernel-shark.git/ > > This is a brand new version that has a lot of changes under the hood and > needs user testing. We will be extremely happy to receive bug reports ;) > or patches for this new version. Great! I've compiled this version and so far everything works. But I use just basic features - opening the trace, searching and zooming. On Fri, Mar 12 2021, Yordan Karadzhov (VMware) wrote: > OK, now I see your problem. Please excuse my ignorance. > > Here's the deal. What you want can be achieved with an almost trivial > patch in kernelshark 2. Have a look here: > > https://git.kernel.org/pub/scm/utils/trace-cmd/kernel-shark.git/tree/src/CMakeLists.txt#n129 > > and make the installation of the policy file to be a separate component. > Make the installation of "kshark-su-record" part of the new component as > well since it relies on the policy file. Sounds good. I'm working on the patch. I'll also send other changes that will make my life easier. > I guess "kernelshark.desktop" must be there as well. I don't think so. Desktop files are looked for at multiple places, including ~/.local/share/application or $XDG_<SOMETHING>. -Michal ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-03-17 13:00 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 13:12 [PATCH] kernel-shark: Do not hardcode /usr prefix for polkit policies Michal Sojka 2021-03-11 14:09 ` Steven Rostedt 2021-03-11 14:48 ` Michal Sojka 2021-03-11 14:50 ` [PATCH v2] " Michal Sojka 2021-03-11 14:54 ` Steven Rostedt 2021-03-11 17:01 ` Yordan Karadzhov (VMware) 2021-03-11 17:57 ` Michal Sojka 2021-03-12 6:46 ` Yordan Karadzhov (VMware) 2021-03-12 11:47 ` Dario Faggioli 2021-03-12 13:02 ` Yordan Karadzhov (VMware) 2021-03-17 12:59 ` Michal Sojka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).