Re: [ABRT PATCH] return the right exit code for user cancellation

[Denys Vlasenko]

On 04/11/2013 11:20 AM, Jakub Filak wrote:
>  Let a frontend know that the error is not fatal and the termination
>  was driven by user request.
>  
>  Related to abrt/libreport#146
>  
>  Signed-off-by: Jakub Filak <jfilak(a)redhat.com&gt;
>  ---
>   src/plugins/abrt-action-install-debuginfo.in     |  4 ++--
>   src/plugins/abrt-action-perform-ccpp-analysis.in | 22 +++++++++++++++-------
>   src/plugins/abrt-retrace-client.c                |  3 +++
>   3 files changed, 20 insertions(+), 9 deletions(-)
>  
>  diff --git a/src/plugins/abrt-action-install-debuginfo.in
> b/src/plugins/abrt-action-install-debuginfo.in
>  index a061fa6..8969b86 100644
>  --- a/src/plugins/abrt-action-install-debuginfo.in
>  +++ b/src/plugins/abrt-action-install-debuginfo.in
>  @@ -197,7 +197,7 @@ if __name__ == "__main__":
>               error_msg("Can't execute abrt-action-trim-files: %s", e);
>   
>           missing = filter_installed_debuginfos(b_ids, cachedirs)
>  -
>  +    result = RETURN_OK
>       if missing:
>           log2("%s", missing)
>           print _("Coredump references {0} debuginfo files, {1} of them are not
> installed").format(len(b_ids), len(missing))
>  @@ -216,4 +216,4 @@ if __name__ == "__main__":
>       if not missing:
>           print _("All debuginfo files are available")
>   
>  -    exit(RETURN_OK)
>  +    exit(result)
>  diff --git a/src/plugins/abrt-action-perform-ccpp-analysis.in
> b/src/plugins/abrt-action-perform-ccpp-analysis.in
>  index c844fbd..14f5049 100644
>  --- a/src/plugins/abrt-action-perform-ccpp-analysis.in
>  +++ b/src/plugins/abrt-action-perform-ccpp-analysis.in
>  @@ -11,8 +11,12 @@ import gettext
>   from subprocess import Popen
>   
>   import report
>  +from report import EXIT_CANCELED_BY_USER as RETURN_CANCELED_BY_USER
>   from reportclient import ask_yes_no_yesforever
>   
>  +# everything was ok
>  +RETURN_OK = 0
>  +
>   GETTEXT_PROGNAME = "abrt"
>   
>   _ = gettext.lgettext
>  @@ -34,8 +38,9 @@ def handle_event(event_name, problem_dir):
>   
>       if ret == 0 and state.children_count == 0:
>           print "No actions are found for event '%s'" % event_name
>  +        return RETURN_FAILURE
>   
>  -    return ret == 0 and state.children_count != 0
>  +    return ret
>   
>   
>   def run_analyze_smart(problem_dir):
>  @@ -53,14 +58,14 @@ def run_analyze_smart(problem_dir):
>   
>       """
>   
>  -    analyzed = False
>  +    retval = RETURN_CANCELED_BY_USER
>       allowed = ask_yes_no_yesforever("abrt_analyze_smart_ask_upload_coredump",
>                   _("Ok to upload core dump? (It may contain sensitive data).
> "\
>                     "If your answer is 'No', a stack trace will be generated
> locally. "\
>                     "(It may download a huge amount of data)."))
>   
>       if allowed:
>  -        analyzed = handle_event("analyze_RetraceServer", problem_dir)
>  +        retval = handle_event("analyze_RetraceServer", problem_dir)
>   
>       # temporary helper variables for better readability
>       option = "abrt_analyze_smart_ask_perform_local_analysis"
>  @@ -70,10 +75,13 @@ def run_analyze_smart(problem_dir):
>   
>       # run local GDB if the retrace event was dismissed
>       # or if the retrace event failed and user gave us permission to run local GDB
>  -    if not allowed or (not analyzed and ask_yes_no_yesforever(option, question)):
>  -        analyzed = handle_event("analyze_LocalGDB", problem_dir)
>  +    #
>  +    # don't change the retval to RETURN_CANCELED_BY_USER if ask_yes_no_yesforever()
> is False
>  +    # we want to catch errors produced by Retrace Server
>  +    if not allowed or (retval != RETURN_OK and ask_yes_no_yesforever(option,
> question)):
>  +        retval = handle_event("analyze_LocalGDB", problem_dir)
>   
>  -    return analyzed
>  +    return retval
>   
>   
>   if __name__ == "__main__":
>  @@ -99,4 +107,4 @@ if __name__ == "__main__":
>   
>       OPTIONS = CMDARGS.parse_args()
>   
>  -    sys.exit(0 if run_analyze_smart(vars(OPTIONS)['problem_dir']) else 1)
>  +    sys.exit(run_analyze_smart(vars(OPTIONS)['problem_dir']))
>  diff --git a/src/plugins/abrt-retrace-client.c b/src/plugins/abrt-retrace-client.c
>  index 781a815..6071528 100644
>  --- a/src/plugins/abrt-retrace-client.c
>  +++ b/src/plugins/abrt-retrace-client.c
>  @@ -619,7 +619,10 @@ static int create(bool delete_temp_archive,
>           free(question);
>   
>           if (!response)
>  +        {
>  +            set_xfunc_error_retval(EXIT_CANCELED_BY_USER);
>               error_msg_and_die(_("Cancelled by user"));
>  +        }
>       }
>   
>       PRFileDesc *tcp_sock, *ssl_sock;
>   
Looks good to me.