[PATCH] move action helpers to /sur/bin

[Denys Vlasenko]

Hi,

When abrt-action-foo's were moved to /usr/libexec, they become
non-callable without tweaking $PATH.

Currently, we do it in abrt-handle-crashdump and abrtd.
I discover more and more cases where I need to tweak $PATH.
For example, both

int run_event_on_dir_name(struct run_event_state *state, const char *dump_dir_name, const char *event);
int run_event_on_crash_data(struct run_event_state *state, crash_data_t *data, const char *event);

need to tweak $PATH, or else any function which calls them will
get failures to find abrt-action-foo.

It seems to me that simpler solution would be to move
abrt-action-foo's to /usr/bin. They do not pollute namespace.
They aren't dangerous in any way for ordinary users to run -
in fact, users may legitimately want to run them.
Developers (we) definitely want to run them for testing,
and not having to set up $PATH or use full path
makes it a bit easier.

Any objections?

[Nikola Pajkovsky]

On Tue, 04 Jan 2011 14:29:09 +0100, Denys Vlasenko  wrote:
> Hi,
>
> When abrt-action-foo's were moved to /usr/libexec, they become
> non-callable without tweaking $PATH.
>
> Currently, we do it in abrt-handle-crashdump and abrtd.
> I discover more and more cases where I need to tweak $PATH.
> For example, both
>
> int run_event_on_dir_name(struct run_event_state *state, const char *dump_dir_name, const char *event);
> int run_event_on_crash_data(struct run_event_state *state, crash_data_t *data, const char *event);
>
> need to tweak $PATH, or else any function which calls them will
> get failures to find abrt-action-foo.
>
> It seems to me that simpler solution would be to move
> abrt-action-foo's to /usr/bin. They do not pollute namespace.
> They aren't dangerous in any way for ordinary users to run -
> in fact, users may legitimately want to run them.
> Developers (we) definitely want to run them for testing,
> and not having to set up $PATH or use full path
> makes it a bit easier.
>
> Any objections?
>
> -- 
> vda
>
> diff -x '*.po' -d -urpN abrt.1/abrt.spec abrt.2/abrt.spec
> --- abrt.1/abrt.spec	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/abrt.spec	2011-01-04 13:07:05.063130781 +0100
> @@ -349,7 +349,7 @@ fi
>  %{_sbindir}/abrt-server
>  %{_bindir}/abrt-handle-upload
>  %{_bindir}/abrt-handle-crashdump
> -%{_libexecdir}/abrt-action-save-package-data
> +%{_bindir}/abrt-action-save-package-data
>  %config(noreplace) %{_sysconfdir}/%{name}/abrt.conf
>  %config(noreplace) %{_sysconfdir}/%{name}/abrt_event.conf
>  %config(noreplace) %{_sysconfdir}/%{name}/gpg_keys
> @@ -416,9 +416,9 @@ fi
>  %dir %{_localstatedir}/cache/abrt-di
>  %{_libdir}/%{name}/libCCpp.so*
>  %{_libexecdir}/abrt-hook-ccpp
> -%{_libexecdir}/abrt-action-analyze-c
> -%{_libexecdir}/abrt-action-install-debuginfo.py*
> -%{_libexecdir}/abrt-action-generate-backtrace
> +%{_bindir}/abrt-action-analyze-c
> +%{_bindir}/abrt-action-install-debuginfo.py*
> +%{_bindir}/abrt-action-generate-backtrace
>
> %files addon-kerneloops
>  %defattr(-,root,root,-)
> @@ -428,48 +428,48 @@ fi
>  %{_mandir}/man7/abrt-KerneloopsScanner.7.gz
>  %{_libdir}/%{name}/KerneloopsReporter.glade
>  %{_mandir}/man7/abrt-KerneloopsReporter.7.gz
> -%{_libexecdir}/abrt-action-analyze-oops
> -%{_libexecdir}/abrt-action-kerneloops
> +%{_bindir}/abrt-action-analyze-oops
> +%{_bindir}/abrt-action-kerneloops
>
> %files plugin-logger
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/Logger.conf
>  %{_libdir}/%{name}/Logger.glade
>  %{_mandir}/man7/abrt-Logger.7.gz
> -%{_libexecdir}/abrt-action-print
> +%{_bindir}/abrt-action-print
>
> %files plugin-mailx
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/Mailx.conf
>  %{_libdir}/%{name}/Mailx.glade
>  %{_mandir}/man7/abrt-Mailx.7.gz
> -%{_libexecdir}/abrt-action-mailx
> +%{_bindir}/abrt-action-mailx
>
> %files plugin-bugzilla
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/Bugzilla.conf
>  %{_libdir}/%{name}/Bugzilla.glade
>  %{_mandir}/man7/abrt-Bugzilla.7.gz
> -%{_libexecdir}/abrt-action-bugzilla
> +%{_bindir}/abrt-action-bugzilla
>
> %files plugin-rhtsupport
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/RHTSupport.conf
>  %{_libdir}/%{name}/RHTSupport.glade
>  # {_mandir}/man7/abrt-RHTSupport.7.gz
> -%{_libexecdir}/abrt-action-rhtsupport
> +%{_bindir}/abrt-action-rhtsupport
>
> %files plugin-reportuploader
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/Upload.conf
>  %{_libdir}/%{name}/Upload.glade
>  %{_mandir}/man7/abrt-Upload.7.gz
> -%{_libexecdir}/abrt-action-upload
> +%{_bindir}/abrt-action-upload
>
> %files addon-python
>  %defattr(-,root,root,-)
>  %config(noreplace) %{_sysconfdir}/%{name}/plugins/Python.conf
> -%{_libexecdir}/abrt-action-analyze-python
> +%{_bindir}/abrt-action-analyze-python
>  %{python_site}/*.py*
>  %{python_site}/abrt.pth
>
> diff -x '*.po' -d -urpN abrt.1/src/daemon/abrt_event.conf abrt.2/src/daemon/abrt_event.conf
> --- abrt.1/src/daemon/abrt_event.conf	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/daemon/abrt_event.conf	2011-01-04 13:08:25.685130919 +0100
> @@ -52,7 +52,7 @@ EVENT=analyze analyzer=CCpp backtrace=  
>  # Same as "analyze", but executed when user requests "refresh" in GUI
>  #EVENT=reanalyze analyzer=CCpp  trim-debuginfo-cache /var/cache/abrt-di 4096m
>  EVENT=reanalyze analyzer=CCpp  abrt-action-install-debuginfo.py "--core=$DUMP_DIR/coredump" "--tmpdir=/var/run/abrt/$$-$RANDOM" --cache=/var/cache/abrt-di
> -EVENT=reanalyze analyzer=CCpp   abrt-action-generate-backtrace
> +EVENT=reanalyze analyzer=CCpp  abrt-action-generate-backtrace
>
> EVENT=report          analyzer=Kerneloops  abrt-action-kerneloops
>  EVENT=report_Bugzilla analyzer=CCpp        abrt-action-bugzilla -c /etc/abrt/plugins/Bugzilla.conf
> diff -x '*.po' -d -urpN abrt.1/src/daemon/abrt-handle-crashdump.c abrt.2/src/daemon/abrt-handle-crashdump.c
> --- abrt.1/src/daemon/abrt-handle-crashdump.c	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/daemon/abrt-handle-crashdump.c	2011-01-04 13:02:15.604880597 +0100
> @@ -87,13 +87,6 @@ int main(int argc, char **argv)
>  /* -e EVENT: run event */
>
> /* Need to add LIBEXEC_DIR to PATH, because otherwise abrt-action-*
> * are not found by exec()
> */
> const char *env_path = getenv("PATH");
> if (!env_path) env_path = "";
> putenv(xasprintf("PATH=%s%s%s", LIBEXEC_DIR, (!env_path[0] ? "" : ":"), env_path));
> struct run_event_state *run_state = new_run_event_state();
>    run_state->logging_callback = do_log;
>    int r = run_event_on_dir_name(run_state, dump_dir_name ? dump_dir_name : ".", event);
>
> diff -x '*.po' -d -urpN abrt.1/src/daemon/Daemon.cpp abrt.2/src/daemon/Daemon.cpp
> --- abrt.1/src/daemon/Daemon.cpp	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/daemon/Daemon.cpp	2011-01-04 13:03:42.003131574 +0100
> @@ -726,13 +726,9 @@ int main(int argc, char** argv)
>       * (I saw it set only DBUS_STARTER_ADDRESS and DBUS_STARTER_BUS_TYPE).
>       * In this case, set something sane:
>       */
> /* Need to add LIBEXEC_DIR to PATH, because otherwise abrt-action-*
> * are not found by exec()
> */
> const char *env_path = getenv("PATH");
>    if (!env_path || !env_path[0])
>    env_path = "/usr/sbin:/usr/bin:/sbin:/bin";
> putenv(xasprintf("PATH=%s:%s", LIBEXEC_DIR, env_path));
>    putenv((char*)"PATH=/usr/sbin:/usr/bin:/sbin:/bin");
>
> putenv(xasprintf("ABRT_VERBOSE=%u", g_verbose));
>
> diff -x '*.po' -d -urpN abrt.1/src/daemon/Makefile.am abrt.2/src/daemon/Makefile.am
> --- abrt.1/src/daemon/Makefile.am	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/daemon/Makefile.am	2011-01-04 13:04:37.394130947 +0100
> @@ -1,12 +1,12 @@
>  bin_SCRIPTS = \
>      abrt-handle-upload
> -libexec_PROGRAMS = \
> abrt-action-save-package-data
>
> bin_PROGRAMS = \
> abrt-handle-crashdump
> abrt-handle-crashdump \
> abrt-action-save-package-data
>
> -sbin_PROGRAMS = abrtd \
> +sbin_PROGRAMS = \
> abrtd \
> abrt-server
>
> abrtd_SOURCES = \
> diff -x '*.po' -d -urpN abrt.1/src/hooks/Makefile.am abrt.2/src/hooks/Makefile.am
> --- abrt.1/src/hooks/Makefile.am	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/hooks/Makefile.am	2011-01-04 13:05:40.345882960 +0100
> @@ -1,4 +1,5 @@
>  libexec_PROGRAMS = abrt-hook-ccpp
>
> bin_PROGRAMS = dumpoops
>
> # abrt-hook-ccpp
> diff -x '*.po' -d -urpN abrt.1/src/plugins/Makefile.am abrt.2/src/plugins/Makefile.am
> --- abrt.1/src/plugins/Makefile.am	2010-12-22 16:33:47.000000000 +0100
> +++ abrt.2/src/plugins/Makefile.am	2011-01-04 13:05:34.139130764 +0100
> @@ -1,8 +1,20 @@
>  pluginslibdir = $(PLUGINS_LIB_DIR)
>
> -libexec_SCRIPTS = \
> +bin_SCRIPTS = \
>      abrt-action-install-debuginfo.py
>
> +bin_PROGRAMS = \
> abrt-action-analyze-c \
> abrt-action-analyze-python \
> abrt-action-analyze-oops \
> abrt-action-generate-backtrace \
> abrt-action-bugzilla \
> abrt-action-rhtsupport \
> abrt-action-kerneloops \
> abrt-action-upload \
> abrt-action-mailx \
> abrt-action-print
>
> pluginslib_LTLIBRARIES = \
>      libCCpp.la \
>      libKerneloopsScanner.la
> @@ -78,18 +90,6 @@ libKerneloopsScanner_la_LDFLAGS = \
>      -avoid-version \
>      $(GLIB_LIBS)
>
> -libexec_PROGRAMS = \
> abrt-action-analyze-c \
> abrt-action-analyze-python \
> abrt-action-analyze-oops \
> abrt-action-generate-backtrace \
> abrt-action-bugzilla \
> abrt-action-rhtsupport \
> abrt-action-kerneloops \
> abrt-action-upload \
> abrt-action-mailx \
> abrt-action-print
>
> abrt_action_analyze_c_SOURCES = \
>      abrt-action-analyze-c.c
>  abrt_action_analyze_c_CPPFLAGS = \
> diff -x '*.po' -d -urpN abrt.1/src/report-python/test_setroubleshoot_example2 abrt.2/src/report-python/test_setroubleshoot_example2
> --- abrt.1/src/report-python/test_setroubleshoot_example2	1970-01-01 01:00:00.000000000 +0100
> +++ abrt.2/src/report-python/test_setroubleshoot_example2	2011-01-04 13:41:39.599543448 +0100
> @@ -0,0 +1,27 @@
> +#!/usr/bin/python
>
> +import report
> +import report.io
> +import report.io.GTKIO
> +import report.accountmanager
>
> +accounts = report.accountmanager.AccountManager()
>
> +signature = report.createAlertSignature("selinux-policy",
>                                    "setroubleshoot",
>                                    "self.siginfo.get_hash()",
>                                    "self.summary",
>                                    "content")
>
> +# Won't send log anywhere:
> +#rc = report.report(signature, report.io.GTKIO.GTKIO(accounts))
>
> +# report.report() + logging:
> +def logging_callback(line):
> print "LOG:", line
> return
>
> +state = report.run_event_state()
> +state.logging_callback = logging_callback
> +rc = state.run_event_on_crash_data(signature, "report")
>
> +print "rc:", rc

if it needs a lot of tweaking, we should move it to /usr/bin

[Jiri Moskovcak]

Seems ok and works ok, I just don't the actions executables will be run 
by users, so I would prefer if the stay in /usr/libexec and we require 
full path in event.conf if the executable is not in $PATH. But if this 
patch would make things easier and won't break anything I'm not against it.

J.

On 01/04/2011 02:29 PM, Denys Vlasenko wrote:
> Hi,
>
> When abrt-action-foo's were moved to /usr/libexec, they become
> non-callable without tweaking $PATH.
>
> Currently, we do it in abrt-handle-crashdump and abrtd.
> I discover more and more cases where I need to tweak $PATH.
> For example, both
>
> int run_event_on_dir_name(struct run_event_state *state, const char *dump_dir_name, const char *event);
> int run_event_on_crash_data(struct run_event_state *state, crash_data_t *data, const char *event);
>
> need to tweak $PATH, or else any function which calls them will
> get failures to find abrt-action-foo.
>
> It seems to me that simpler solution would be to move
> abrt-action-foo's to /usr/bin. They do not pollute namespace.
> They aren't dangerous in any way for ordinary users to run -
> in fact, users may legitimately want to run them.
> Developers (we) definitely want to run them for testing,
> and not having to set up $PATH or use full path
> makes it a bit easier.
>
> Any objections?