Hi, The patch compiles and works, but it's not meant to be committed as-is, discussion is definitely needed. -- vda diff -urp libreport.7/libreport.spec.in libreport.9/libreport.spec.in --- libreport.7/libreport.spec.in 2012-10-25 11:11:06.000000000 +0200 +++ libreport.9/libreport.spec.in 2012-11-07 13:24:46.573794037 +0100 @@ -362,6 +362,7 @@ gtk-update-icon-cache %{_datadir}/icons/ %files plugin-bugzilla %defattr(-,root,root,-) %config(noreplace) %{_sysconfdir}/libreport/plugins/bugzilla.conf +%config(noreplace) %{_sysconfdir}/libreport/plugins/bugzilla_format.conf %{_sysconfdir}/libreport/events/report_Bugzilla.xml %config(noreplace) %{_sysconfdir}/libreport/events/report_Bugzilla.conf %config(noreplace) %{_sysconfdir}/libreport/events.d/bugzilla_event.conf diff -urp libreport.7/src/plugins/Makefile.am libreport.9/src/plugins/Makefile.am --- libreport.7/src/plugins/Makefile.am 2012-10-04 13:13:34.000000000 +0200 +++ libreport.9/src/plugins/Makefile.am 2012-11-07 13:25:25.591878189 +0100 @@ -23,7 +23,7 @@ reportpluginsconfdir = $(REPORT_PLUGINS_ reporters_plugin_conf = if BUILD_BUGZILLA -reporters_plugin_conf += bugzilla.conf +reporters_plugin_conf += bugzilla.conf bugzilla_format.conf endif dist_reportpluginsconf_DATA = $(reporters_plugin_conf) \ diff -urp libreport.7/src/plugins/reporter-bugzilla.c libreport.9/src/plugins/reporter-bugzilla.c --- libreport.7/src/plugins/reporter-bugzilla.c 2012-11-05 22:34:53.885310748 +0100 +++ libreport.9/src/plugins/reporter-bugzilla.c 2012-11-07 13:48:46.787219126 +0100 @@ -21,6 +21,266 @@ #include "abrt_xmlrpc.h" #include "rhbz.h" +struct section_t { + char *name; + GList *items; +}; +typedef struct section_t section_t; + +static +GList* split_string_on_char(const char *str, char ch) +{ + GList *list = NULL; + for (;;) + { + const char *delim = strchrnul(str, ch); + list = g_list_prepend(list, xstrndup(str, delim - str)); + if (*delim == '\0') + break; + str = delim + 1; + } + return g_list_reverse(list); +} + +static +GList* load_bzrep_conf_file(const char *path) +{ + FILE *fp = stdin; + if (strcmp(path, "-") != 0) + { + fp = fopen(path, "r"); + if (!fp) + return NULL; + } + + GList *sections = NULL; + + char *line; + while ((line = xmalloc_fgetline(fp)) != NULL) + { + /* Skip comments and empty lines */ + char first = *skip_whitespace(line); + if (first == '\0' || first == '#') + goto free_line; + + /* We are reusing line buffer to form temporary + * "key\0values\0..." in its beginning + */ + char *value = NULL; + char *src; + char *dst; + for (src = dst = line; *src; src++) + { + char c = *src; + /* did we reach the value list? */ + if (!value && c == ':') + { + *dst++ = '\0'; /* terminate key */ + value = dst; /* remember where value starts */ + continue; + } + /* skip whitespace in value list */ + if (value && isspace(c)) + continue; + *dst++ = c; /* store next key or value char */ + } + + /* Skip broken lines */ + if (!value) + goto free_line; + + *dst = '\0'; /* terminate value */ + + GList *item_list = split_string_on_char(value, ','); + + section_t *sec = xzalloc(sizeof(*sec)); + sec->name = xstrdup(line); + sec->items = item_list; + sections = g_list_prepend(sections, sec); + + free_line: + free(line); + } + + if (fp != stdin) + fclose(fp); + + return g_list_reverse(sections); +} + +static +bool is_explicit_or_forbidden(const char *name, GList *comment_fmt_spec) +{ + GList *l = comment_fmt_spec; + while (l) + { + section_t *sec = l->data; + GList *item = sec->items; + while (item) + { + const char *nm = item->data; + if (nm[0] == '-') + nm++; + if (strcmp(name, nm) == 0) + return true; /* we see "name" or "-name" */ + item = item->next; + } + l = l->next; + } + return false; +} + +static +int append_item(struct strbuf *result, const char *item_name, problem_data_t *pd, GList *comment_fmt_spec) +{ + bool print_item_name = (strncmp(item_name, "%bare_", strlen("%bare_")) != 0); + if (!print_item_name) + item_name += strlen("%bare_"); + + if (item_name[0] != '%') + { + struct problem_item *item = problem_data_get_item_or_NULL(pd, item_name); + if (!item) + return 0; /* "I did not print anything" */ + if (!(item->flags & CD_FLAG_TXT)) + return 0; /* "I did not print anything" */ + + char *formatted = problem_item_format(item); + char *content = formatted ? formatted : item->content; + char *eol = strchrnul(content, '\n'); + if (eol[0] == '\0' || eol[1] == '\0') + { + /* one-liner */ + int pad = 16 - (strlen(item_name) + 2); + if (pad < 0) + pad = 0; + if (print_item_name) + strbuf_append_strf(result, + eol[0] == '\0' ? "%s: %*s%s\n" : "%s: %*s%s", + item_name, pad, "", content + ); + else + strbuf_append_strf(result, + eol[0] == '\0' ? "%s\n" : "%s", + content + ); + } + else + { + /* multi-line item */ + if (print_item_name) + strbuf_append_strf(result, "%s:\n", item_name); + for (;;) + { + eol = strchrnul(content, '\n'); + strbuf_append_strf(result, + /* For %bare_multiline_item, we don't want to print colons */ + (print_item_name ? ":%.*s\n" : "%.*s\n"), + (int)(eol - content), content + ); + if (eol[0] == '\0' || eol[1] == '\0') + break; + content = eol + 1; + } + } + free(formatted); + return 1; /* "I printed something" */ + } + + /* Special item name */ + /* %oneline,%multiline,%text,%binary */ + bool oneline = (strcmp(item_name+1, "oneline" ) == 0); + bool multiline = (strcmp(item_name+1, "multiline") == 0); + bool text = (strcmp(item_name+1, "text" ) == 0); +// bool binary = (strcmp(item_name+1, "binary" ) == 0); + if (!oneline && !multiline && !text) + { + log("Unknown or unsupported element specifier '%s'", item_name); + return 0; /* "I did not print anything" */ + } + + int printed = 0; + { + /* Iterate over _sorted_ items */ + GList *sorted_names = g_hash_table_get_keys(pd); + sorted_names = g_list_sort(sorted_names, (GCompareFunc)strcmp); + + /* %text => do as if %oneline, then repeat as if %multiline */ + if (text) + oneline = 1; + again: ; + GList *l = sorted_names; + while (l) + { + const char *name = l->data; + l = l->next; + struct problem_item *item = g_hash_table_lookup(pd, name); + if (!item) + continue; /* paranoia, won't happen */ + + if (!(item->flags & CD_FLAG_TXT)) + continue; + + if (is_explicit_or_forbidden(name, comment_fmt_spec)) + continue; + + char *formatted = problem_item_format(item); + char *content = formatted ? formatted : item->content; + char *eol = strchrnul(content, '\n'); + bool is_oneline = (eol[0] == '\0' || eol[1] == '\0'); + if (oneline == is_oneline) + printed |= append_item(result, name, pd, comment_fmt_spec); + free(formatted); + } + if (text && oneline) + { + /* %text, and we just did %oneline. Repeat as if %multiline */ + oneline = 0; + /*multiline = 1; - not checked in fact, so why bother setting? */ + goto again; + } + + g_list_free(sorted_names); /* names themselves are not freed */ + + } + return printed; +} + +static +int generate_bz_comment(struct strbuf *result, problem_data_t *pd, GList *comment_fmt_spec) +{ + int printed = 0; + GList *l = comment_fmt_spec; + while (l) + { + struct strbuf *output = strbuf_new(); + section_t *sec = l->data; + GList *item = sec->items; + while (item) + { + const char *str = item->data; + if (str[0] == '-') /* "-name", ignore it */ + goto skip_item; + printed |= append_item(output, str, pd, comment_fmt_spec); + skip_item: + item = item->next; + } + + if (output->len != 0) + { + strbuf_append_strf(result, + (result->len == 0 ? "%s:\n%s" : "\n%s:\n%s"), + sec->name, + output->buf + ); + } + strbuf_free(output); + + l = l->next; + } + + return printed; +} struct bugzilla_struct { const char *b_login; @@ -103,12 +363,9 @@ int main(int argc, char **argv) textdomain(PACKAGE); #endif - const char *dump_dir_name = "."; - GList *conf_file = NULL; - /* Can't keep these strings/structs static: _() doesn't support that */ const char *program_usage_string = _( - "\n& [-vbf] [-g GROUP-NAME]... [-c CONFFILE]... -d DIR" + "\n& [-vbf] [-g GROUP-NAME]... [-c CONFFILE]... [-F FMTFILE] -d DIR" "\nor:" "\n& [-v] [-c CONFFILE]... [-d DIR] -t[ID] FILE..." "\nor:" @@ -142,16 +399,21 @@ int main(int argc, char **argv) "\nRecognized string parameters: BugzillaURL, Login, Password, OSRelease." "\nRecognized boolean parameter (VALUE should be 1/0, yes/no): SSLVerify." "\nParameters can be overridden via $Bugzilla_PARAM environment variables." + "\n" + "\nFMTFILE defaults to "CONF_DIR"/plugins/bugzilla_format.conf" ); enum { OPT_v = 1 << 0, OPT_d = 1 << 1, OPT_c = 1 << 2, - OPT_t = 1 << 3, - OPT_b = 1 << 4, - OPT_f = 1 << 5, + OPT_F = 1 << 3, + OPT_t = 1 << 4, + OPT_b = 1 << 5, + OPT_f = 1 << 6, }; - + const char *dump_dir_name = "."; + GList *conf_file = NULL; + const char *fmt_file = CONF_DIR"/plugins/bugzilla_format.conf"; char *ticket_no = NULL, *abrt_hash = NULL; GList *group = NULL; /* Keep enum above and order of options below in sync! */ @@ -159,6 +421,7 @@ int main(int argc, char **argv) OPT__VERBOSE(&g_verbose), OPT_STRING( 'd', NULL, &dump_dir_name , "DIR" , _("Problem directory")), OPT_LIST( 'c', NULL, &conf_file , "FILE" , _("Configuration file (may be given many times)")), + OPT_STRING( 'F', NULL, &fmt_file , "FILE" , _("Formatting file")), OPT_OPTSTRING('t', "ticket", &ticket_no , "ID" , _("Attach FILEs [to bug with this ID]")), OPT_BOOL( 'b', NULL, NULL, _("When creating bug, attach binary files too")), OPT_BOOL( 'f', NULL, NULL, _("Force reporting even if this problem is already reported")), @@ -410,12 +673,18 @@ int main(int argc, char **argv) /* Create new bug */ log(_("Creating a new bug")); - char *aux_msg = crossver_id < 0 ? NULL : - xasprintf("\nPotential duplicate: bug %u\n", crossver_id); + GList *comment_fmt_spec = load_bzrep_conf_file(fmt_file); + + struct strbuf *bzcomment_buf = strbuf_new(); + generate_bz_comment(bzcomment_buf, problem_data, comment_fmt_spec); + if (crossver_id >= 0) + strbuf_append_strf(bzcomment_buf, "\nPotential duplicate: bug %u\n", crossver_id); + char *bzcomment = strbuf_free_nobuf(bzcomment_buf); int new_id = rhbz_new_bug(client, problem_data, rhbz.b_os_release, - aux_msg, - group); - free(aux_msg); + bzcomment, + group + ); + free(bzcomment); log(_("Adding attachments to bug %i"), new_id); char new_id_str[sizeof(int)*3 + 2]; diff -urp libreport.7/src/plugins/rhbz.c libreport.9/src/plugins/rhbz.c --- libreport.7/src/plugins/rhbz.c 2012-11-01 13:30:04.560505828 +0100 +++ libreport.9/src/plugins/rhbz.c 2012-11-07 13:48:46.788219126 +0100 @@ -445,62 +445,6 @@ struct bug_info *rhbz_bug_info(struct ab return bz; } -/* - * Produces an optimal backtrace for bugzilla comment - */ -char *rhbz_get_backtrace_info(problem_data_t *problem_data, size_t max_text_size) -{ - const problem_item *item = problem_data_get_item_or_NULL(problem_data, - FILENAME_BACKTRACE); - - if (!item) - return NULL; - - char *truncated = NULL; - - if (strlen(item->content) >= max_text_size) - { - struct btp_location location; - btp_location_init(&location); - - /* btp_backtrace_parse modifies the input parameter */ - char *content = item->content; - struct btp_backtrace *backtrace = btp_backtrace_parse((const char **)&content, &location); - - if (!backtrace) - { - log(_("Can't parse backtrace")); - return NULL; - } - - /* Get optimized thread stack trace for 10 top most frames */ - struct btp_thread *thread = btp_backtrace_get_optimized_thread(backtrace, 10); - - btp_backtrace_free(backtrace); - - if (!thread) - { - log(_("Can't find crash thread")); - return NULL; - } - - /* Cannot be NULL, it dies on memory error */ - struct btp_strbuf *bt = btp_strbuf_new(); - - btp_thread_append_to_str(thread, bt, true); - - btp_thread_free(thread); - - truncated = btp_strbuf_free_nobuf(bt); - } - - char *bt = make_description_item_multiline(truncated ? "truncated backtrace" : FILENAME_BACKTRACE, - truncated ? truncated : item->content); - free(truncated); - - return bt; -} - /******************************************************************************/ /* New bug logic: * 1. 'Description of problem' is FILENAME_COMMENT @@ -566,7 +510,7 @@ static const char *const g_not_attached_ int rhbz_new_bug(struct abrt_xmlrpc *ax, problem_data_t *problem_data, const char *release, - const char *aux_msg, + const char *bzcomment, GList *group) { func_entry(); @@ -600,8 +544,6 @@ int rhbz_new_bug(struct abrt_xmlrpc *ax, FILENAME_ANALYZER); const char *tainted_short = problem_data_get_content_or_NULL(problem_data, FILENAME_TAINTED_SHORT); - const char *comment = problem_data_get_content_or_NULL(problem_data, - FILENAME_COMMENT); struct strbuf *buf_summary = strbuf_new(); @@ -667,36 +609,6 @@ int rhbz_new_bug(struct abrt_xmlrpc *ax, buf[3] = '\0'; } - /* Generating of a description according to the RH bugzilla default format: - * https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora - */ - struct strbuf *tmp_full_dsc = strbuf_new(); - if (comment) - strbuf_append_strf(tmp_full_dsc, "Description of problem:\n%s\n\n", comment); - - /* the package file always contains 'kernel' string in case of kerneloops */ - /* it doesn't make sense to mess up a description with this useless information */ - if (package && !kerneloops) - strbuf_append_strf(tmp_full_dsc, "Version-Release number of selected component:\n%s\n\n", package); - - char *bz_dsc = make_description(problem_data, (char**)g_excluded_items, - CD_TEXT_ATT_SIZE_BZ, /*flags:*/ 0); - - strbuf_append_strf(tmp_full_dsc, "Additional info:\nlibreport_version: "VERSION"\n%s\n", bz_dsc); - free(bz_dsc); - - char *backtrace = rhbz_get_backtrace_info(problem_data, CD_TEXT_ATT_SIZE_BZ); - if (backtrace) - { - strbuf_append_str(tmp_full_dsc, backtrace); - free(backtrace); - } - - if (aux_msg) - strbuf_append_str(tmp_full_dsc, aux_msg); - - char *full_dsc = strbuf_free_nobuf(tmp_full_dsc); - char *product = NULL; char *version = NULL; parse_release_for_bz(release, &product, &version); @@ -712,7 +624,7 @@ int rhbz_new_bug(struct abrt_xmlrpc *ax, "component", component, "version", version, "summary", summary, - "description", full_dsc, + "description", bzcomment, "status_whiteboard", status_whiteboard, "platform", arch); } @@ -743,7 +655,7 @@ int rhbz_new_bug(struct abrt_xmlrpc *ax, "component", component, "version", version, "summary", summary, - "description", full_dsc, + "description", bzcomment, "status_whiteboard", status_whiteboard, "platform", arch, "groups", xmlrpc_groups); @@ -754,7 +666,6 @@ int rhbz_new_bug(struct abrt_xmlrpc *ax, free(product); free(version); free(summary); - free(full_dsc); if (!result) return -1;