[Bug spam/25726] iya jipaosy
https://sourceware.org/bugzilla/show_bug.cgi?id=25726 Mark Wielaard changed: What|Removed |Added Assignee|unassigned at sourceware dot org |nobody at sourceware dot org Status|UNCONFIRMED |RESOLVED Product|elfutils|web Resolution|--- |INVALID CC|elfutils-devel at sourceware dot o | |rg | Component|debuginfod |spam --- Comment #1 from Mark Wielaard --- spam -- You are receiving this mail because: You are on the CC list for the bug.
Re: Buildbot failure in Wildebeest Builder on whole buildset
On Wed, 2020-03-25 at 23:48 +, build...@builder.wildebeest.org wrote: > The Buildbot has detected a failed build on builder whole buildset > while building elfutils. > Full details are available at: > https://builder.wildebeest.org/buildbot/#builders/2/builds/495 > > Buildbot URL: https://builder.wildebeest.org/buildbot/ > > Worker for this Build: debian-amd64 Well, this is odd. This is: FAIL: run-backtrace-native-core-biarch.sh = 0xf7f6c000 0xf7f6e000 linux-gate.so.1 0xf7f6e000 0xf7f97948 ld-linux.so.2 0xf7d56000 0xf7f33780 libc.so.6 0xf7f34000 0xf7f542b8 libpthread.so.0 0x565a4000 0x565a8054 backtrace-child-biarch TID 5133: # 0 0xf7f6d069 __kernel_vsyscall # 1 0xf7f469b2 - 1 raise # 2 0x565a548a - 1 sigusr2 # 3 0x565a5564 - 1 stdarg # 4 0x565a557a - 1 backtracegen # 5 0x565a5588 - 1 start # 6 0xf7f3afd2 - 1 start_thread # 7 0xf7e506d6 - 1 __clone TID 5132: # 0 0xf7e506c4 __clone /srv/buildbot/worker/elfutils-debian-amd64/build/tests/backtrace: dwfl_thread_getframes: no matching address range backtrace: backtrace.c:81: callback_verify: Assertion `seen_main' failed. ./test-subr.sh: line 84: 5136 Aborted LD_LIBRARY_PATH="${built_library_path}${LD_LIBR ARY_PATH:+:}$LD_LIBRARY_PATH" $VALGRIND_CMD "$@" backtrace-child-biarch-core.5132: no main rmdir: failed to remove 'test-5124': Directory not empty FAIL run-backtrace-native-core-biarch.sh (exit status: 1) But I cannot replicate it. And it was caused by a change to the README file. Rebuilding on the same buildbot worker also made it PASS... Sigh. Don't know what to do about it, except watch whether it happens again or not. Cheers, Mark
[Bug debuginfod/25728] New: Starting debubinfod with --port=0 is not documented
https://sourceware.org/bugzilla/show_bug.cgi?id=25728 Bug ID: 25728 Summary: Starting debubinfod with --port=0 is not documented Product: elfutils Version: unspecified Status: NEW Severity: normal Priority: P2 Component: debuginfod Assignee: unassigned at sourceware dot org Reporter: kkleine at redhat dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- In order bind the debuginfod server on any port (e.g. for testing) you might want to start it using debuginfod --port=0 When you do that, the logs say: started http server on IPv4 IPv6 port=0 IMHO debuginfod should print the port that was actually assigned by the operating system. -- You are receiving this mail because: You are on the CC list for the bug.
Re: patch PR25548: debuginfod canonicalized source paths
Hi Frank, On Tue, 2020-03-24 at 16:32 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > The following patch generalizes the webapi slightly, which allows > debuggers like lldb to operate against packages/binaries with > source files that include posix pathname noise. > > commit 8ac3883f36c3c3d48bc90891aad6718a798bdd7a (HEAD -> fche/pr25548) > Author: Frank Ch. Eigler > Date: Mon Mar 23 15:33:56 2020 -0400 > > PR25548: support canonicalized source-path names in debuginfod webapi > > Programs are sometimes compiled with source path names containing > noise like /./ or // or /foo/../, and these survive into DWARF. This > code allows either raw or canonicalized pathnames in the webapi, by > letting the client pass things verbatim, and letting the server > store/accept both raw and canonicalized path names for source files. > Tests included & docs updated. This looks good. And seems very useful. Thanks. Just a comment below about the documentation (and some silly whitespace). > Signed-off-by: Frank Ch. Eigler > > diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog > index 2410430c2361..4d687e75bdf8 100644 > --- a/debuginfod/ChangeLog > +++ b/debuginfod/ChangeLog > @@ -1,3 +1,14 @@ > +2020-03-24 Frank Ch. Eigler > + > + * debuginfod-client.c (debuginfod_query_server): Set > + CURLOPT_PATH_AS_IS, to propagate file names verbatim. > + * debuginfod.cxx (canon_pathname): Implement RFC3986 > + style pathname canonicalization. > + (handle_buildid): Canonicalize incoming webapi source > + paths, accept either one. > + (scan_source_file, archive_classify): Store both > + original and canonicalized dwarf-source file names. > + > 2020-03-22 Frank Ch. Eigler > > * debuginfod-client.c (struct debuginfod_client): Add url field. > diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c > index 58a04b9a734b..3d6f645d56f2 100644 > --- a/debuginfod/debuginfod-client.c > +++ b/debuginfod/debuginfod-client.c > @@ -681,6 +681,7 @@ debuginfod_query_server (debuginfod_client *c, >curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); > + curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, ""); >add_extra_headers(data[i].handle); > diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx > index 7c7e85eb6d14..dc62eb088c02 100644 > --- a/debuginfod/debuginfod.cxx > +++ b/debuginfod/debuginfod.cxx > @@ -944,6 +944,82 @@ shell_escape(const string& str) > } > > > +// PR25548: Perform POSIX / RFC3986 style path canonicalization on the input > string. > +// > +// Namely: > +//// -> / > +///foo/../ -> / > +///./-> / > +// > +// This mapping is done on dwarf-side source path names, which may > +// include these constructs, so we can deal with debuginfod clients > +// that accidentally canonicalize the paths. > +// > +// realpath(3) is close but not quite right, because it also resolves > +// symbolic links. Symlinks at the debuginfod server have nothing to > +// do with the build-time symlinks, thus they must not be considered. > +// > +// see also curl Curl_dedotdotify() aka RFC3986, which we mostly follow here > +// see also libc __realpath() > +// see also llvm llvm::sys::path::remove_dots() It would be good to add (part of) this to the documentation. > +static string > +canon_pathname (const string& input) > +{ > + string i = input; // 5.2.4 (1) > + string o; > + > + while (i.size() != 0) > +{ > + // 5.2.4 (2) A > + if (i.substr(0,3) == "../") > +i = i.substr(3); > + else if(i.substr(0,2) == "./") > +i = i.substr(2); > + > + // 5.2.4 (2) B > + else if (i.substr(0,3) == "/./") > +i = i.substr(2); > + else if (i == "/.") > +i = ""; // no need to handle "/." complete-path-segment case; we're > dealing with file names > + > + // 5.2.4 (2) C > + else if (i.substr(0,4) == "/../") { > +i = i.substr(3); > +string::size_type sl = o.rfind("/"); > +if (sl != string::npos) > + o = o.substr(0, sl); > +else > + o = ""; > + } else if (i == "/..") > +i = ""; // no need to handle "/.." complete-path-segment case; we're > dealing with file names > + > + // 5.2.4 (2) D > + // no need to handle these cases; we're dealing with file names > + else if (i == ".") > +i = ""; > + else if (i == "..") > +i = ""; > + > + // POSIX special: map // to / > + else if (i.substr(0,2) == "//") > +i = i.substr(1); > + > + // 5.2.4 (2) E > + else { > +string::s
[Bug debuginfod/25728] Starting debubinfod with --port=0 is not documented
https://sourceware.org/bugzilla/show_bug.cgi?id=25728 Mark Wielaard changed: What|Removed |Added CC||mark at klomp dot org --- Comment #1 from Mark Wielaard --- Is this behavior actually useful or does --port=0 work by accident? I don't see this (http_port == 0) documented for MHD_start_daemon. So I would actually just reject it: diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 7c7e85eb..39736351 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -422,7 +422,8 @@ parse_opt (int key, char *arg, case 'v': verbose ++; break; case 'd': db_path = string(arg); break; case 'p': http_port = (unsigned) atoi(arg); - if (http_port > 65535) argp_failure(state, 1, EINVAL, "port number"); + if (http_port == 0 || http_port > 65535) +argp_failure(state, 1, EINVAL, "port number"); break; case 'F': scan_files = true; break; case 'R': -- You are receiving this mail because: You are on the CC list for the bug.
Re: PR25267: debuginfod status logging improvements
Hi Frank, On Tue, 2020-03-24 at 22:12 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > This makes debuginfod logs more useful to admins. > > Author: Frank Ch. Eigler > Date: Tue Mar 24 21:57:59 2020 -0400 > > PR25367: improve debuginfod webapi logging > > Improve debuginfod logging to show webapi query results including > http status, sizes, and processing times. This looks good and useful. Two small questions below. > +2020-03-24 Frank Ch. Eigler > + > + * debuginfod.cxx (handle_buildid): In case of federated fallback > + queries, handle errors analogously to local ENOENT/404. > + (handle_metrics): Return a size-of-response value. > + (handler_cb): Add code to time entire application-side processing > + stage + response sizes + http codes, so as to emit a complete > + httpd-flavoured log line for each webapi request. OK. > diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx > index 7c7e85eb6d14..490169a40ded 100644 > --- a/debuginfod/debuginfod.cxx > +++ b/debuginfod/debuginfod.cxx > @@ -853,6 +853,7 @@ conninfo (struct MHD_Connection * conn) > > > > + > static void > add_mhd_last_modified (struct MHD_Response *resp, time_t mtime) > { > @@ -1473,8 +1474,16 @@ static struct MHD_Response* handle_buildid (const > string& buildid /* unsafe */, > } >close (fd); > } > - else if (fd != -ENOSYS) // no DEBUGINFOD_URLS configured > -throw libc_exception(-fd, "upstream debuginfod query failed"); > + else > +switch(fd) > + { > + case -ENOSYS: > +break; > + case -ENOENT: > +break; > + default: // some more tricky error > +throw libc_exception(-fd, "upstream debuginfod query failed"); > + } > >throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found"); > } OK > @@ -1564,7 +1573,7 @@ add_metric(const string& metric, > > > static struct MHD_Response* > -handle_metrics () > +handle_metrics (off_t* size) > { >stringstream o; >{ > @@ -1576,6 +1585,7 @@ handle_metrics () >MHD_Response* r = MHD_create_response_from_buffer (os.size(), > (void*) os.c_str(), > MHD_RESPMEM_MUST_COPY); > + *size = os.size(); >MHD_add_response_header (r, "Content-Type", "text/plain"); >return r; > } OK. os is a string and string.size() returns the number of bytes. > @@ -1598,8 +1608,11 @@ handler_cb (void * /*cls*/, >struct MHD_Response *r = NULL; >string url_copy = url; > > - if (verbose) > -obatched(clog) << conninfo(connection) << " " << method << " " << url << > endl; > + int rc = MHD_NO; // mhd > + int http_code = 500; > + off_t http_size = -1; > + struct timeval tv_start, tv_end; > + gettimeofday (&tv_start, NULL); > >try > { > @@ -1632,12 +1645,22 @@ handler_cb (void * /*cls*/, > } > >inc_metric("http_requests_total", "type", artifacttype); > - r = handle_buildid(buildid, artifacttype, suffix, 0); // NB: don't > care about result-fd > + > + // get the resulting fd so we can report its size > + int fd; > + r = handle_buildid(buildid, artifacttype, suffix, &fd); > + if (r) > +{ > + struct stat fs; > + if (fstat(fd, &fs) == 0) > +http_size = fs.st_size; > + // libmicrohttpd will close (fd); > +} If fstat fails for whatever reason http_size isn't set (-1). > } >else if (url1 == "/metrics") > { >inc_metric("http_requests_total", "type", "metrics"); > - r = handle_metrics(); > + r = handle_metrics(& http_size); > } >else > throw reportable_exception("webapi error, unrecognized /operation"); > @@ -1645,16 +1668,27 @@ handler_cb (void * /*cls*/, >if (r == 0) > throw reportable_exception("internal error, missing response"); > > - int rc = MHD_queue_response (connection, MHD_HTTP_OK, r); > + rc = MHD_queue_response (connection, MHD_HTTP_OK, r); > + http_code = MHD_HTTP_OK; >MHD_destroy_response (r); > - return rc; > } >catch (const reportable_exception& e) > { >inc_metric("http_responses_total","result","error"); >e.report(clog); > - return e.mhd_send_response (connection); > + http_code = e.code; > + rc = e.mhd_send_response (connection); > } In the catch case http_size might not be set (or is -1). > + > + gettimeofday (&tv_end, NULL); > + double deltas = (tv_end.tv_sec - tv_start.tv_sec) + (tv_end.tv_usec - > tv_start.tv_usec)*0.01; > + obatched(clog) << conninfo(connection) > + << ' ' << method << ' ' << url > + << ' ' << http_code << ' ' << http_size > + <<
Re: PR25267: debuginfod status logging improvements
Hi - > [...] > > + obatched(clog) << conninfo(connection) > > + << ' ' << method << ' ' << url > > + << ' ' << http_code << ' ' << http_size > > + << ' ' << (int)(deltas*1000) << "ms" > > + << endl; > > + > > + return rc; > > } > > Should the case where http_size == -1 be handled specially? > Would it make sense to only log this if (verbose > 1) ? Web server logs routinely include failures; it should be okay. Thanks for the review! - FChE
Re: patch PR25548: debuginfod canonicalized source paths
Hi - > > The following patch generalizes the webapi slightly, which allows > > debuggers like lldb to operate against packages/binaries with > > source files that include posix pathname noise. > Could you add something that describes the exact form of > canonicalization done? Maybe just: > > "Canonicalization is done according to RFC3986 section 5.2.4 (Remove > Dot Segments), plus reducing any '//' to '/' in the path." OK. > > diff --git > > a/tests/debuginfod-rpms/fedora31/hello3-two-debuginfo-1.0-2.x86_64.rpm > > b/tests/debuginfod-rpms/fedora31/hello3-two-debuginfo-1.0-2.x86_64.rpm > > new file mode 100644 > > index ..0cc24073aa3e > > When using git format-patch instead of git show you'll get something > that people can apply and that shows how big the files are. I assume > they are small? The existing ones are between the 4K and 12K. Yes, they're the same size. Thanks for the review! - FChE
[Bug debuginfod/25548] also support canonicalized source-file name lookups in webapi
https://sourceware.org/bugzilla/show_bug.cgi?id=25548 Frank Ch. Eigler changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #6 from Frank Ch. Eigler --- commit d63a809da467e646480c273b8eb276401679d2bb -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/25367] web request status logging improvements
https://sourceware.org/bugzilla/show_bug.cgi?id=25367 Frank Ch. Eigler changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #2 from Frank Ch. Eigler --- commit 439641e52a3258ddfa1f5de8bbcdb1530fc1 -- You are receiving this mail because: You are on the CC list for the bug.
PR25548 followup
Hi - pushed as obvious: Author: Frank Ch. Eigler Date: Thu Mar 26 11:12:49 2020 -0400 PR25548: CURLOPT_PATH_AS_IS backward compatibility libcurl < 7.42 lacks the CURLOPT_PATH_AS_IS flag, but extraneous client-side canonicalization is mostly harmless. Signed-off-by: Frank Ch. Eigler Reported-by: Mark Wielaard diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 7518e886031c..60d912a37c47 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * debuginfod-client.c (debuginfod_query_server): Don't + set CURLOPT_PATH_AS_IS on old curl. Mostly harmless. + 2020-03-24 Frank Ch. Eigler * debuginfod-client.c (debuginfod_query_server): Set diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 251047caf53f..f1b63160a5f2 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -716,7 +716,12 @@ debuginfod_query_server (debuginfod_client *c, curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); +#if CURL_AT_LEAST_VERSION(7,42,0) curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1); +#else + /* On old curl; no big deal, canonicalization here is almost the + same, except perhaps for ? # type decorations at the tail. */ +#endif curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, ""); add_extra_headers(data[i].handle);
Re: PR25548 followup
Hi - Ah but that would be too easy. Author: Frank Ch. Eigler Date: Thu Mar 26 11:12:49 2020 -0400 PR25548: CURLOPT_PATH_AS_IS backward compatibility old libcurl lacks the CURL_AT_LEAST_VERSION macro, so use LIBCURL_VERSION_NUM testing instead. Signed-off-by: Frank Ch. Eigler diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index f1b63160a5f2..b0d5cb028508 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -716,7 +716,7 @@ debuginfod_query_server (debuginfod_client *c, curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); -#if CURL_AT_LEAST_VERSION(7,42,0) +#if LIBCURL_VERSION_NUM >= 0x072a00 /* 7.42.0 */ curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1); #else /* On old curl; no big deal, canonicalization here is almost the
Buildbot failure in Wildebeest Builder on whole buildset
The Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/1/builds/503 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: centos-x86_64 Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed compile (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/2/builds/498 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-amd64 Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/3/builds/501 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-x86_64 Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/4/builds/498 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-i386 Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/10/builds/469 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-s390x Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/15/builds/294 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-armhf Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a failed build on builder whole buildset while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/16/builds/291 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: centos-aarch64 Build Reason: Blamelist: Frank Ch. Eigler BUILD FAILED: failed compile (failure) Sincerely, -The Buildbot
Re: PR25548 followup
On Thu, 2020-03-26 at 11:24 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > Ah but that would be too easy. :) Also make sure the new test rpms are added to EXTRA_DIST. Pushed the attached. Cheers, Mark From 12305ff528ad9365b4d4e5ebcd307373cdd5bf83 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 26 Mar 2020 18:01:29 +0100 Subject: [PATCH] tests: Add missing debuginfod-rpms/fedora31/hello3-*.rpm to EXTRA_DIST. The new rpms were used in the new test. Make sure they are in the dist tar ball. Signed-off-by: Mark Wielaard --- tests/ChangeLog | 10 ++ tests/Makefile.am | 6 ++ 2 files changed, 16 insertions(+) diff --git a/tests/ChangeLog b/tests/ChangeLog index 933e6726..b7390a57 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,13 @@ +2020-03-26 Mark Wielaard + + * Makefile.am (EXTRA_DIST): Add + debuginfod-rpms/fedora31/hello3-1.0-2.src.rpm, + debuginfod-rpms/fedora31/hello3-1.0-2.x86_64.rpm, + debuginfod-rpms/fedora31/hello3-debuginfo-1.0-2.x86_64.rpm, + debuginfod-rpms/fedora31/hello3-debugsource-1.0-2.x86_64.rpm, + debuginfod-rpms/fedora31/hello3-two-1.0-2.x86_64.rpm, + debuginfod-rpms/fedora31/hello3-two-debuginfo-1.0-2.x86_64.rpm + 2020-03-24 Frank Ch. Eigler * debuginfod-rpms/hello3.spec., /fedora31/*: New files with diff --git a/tests/Makefile.am b/tests/Makefile.am index 934110e0..40b1c001 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -457,6 +457,12 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \ debuginfod-rpms/fedora30/hello2-debugsource-1.0-2.x86_64.rpm \ debuginfod-rpms/fedora30/hello2-two-1.0-2.x86_64.rpm \ debuginfod-rpms/fedora30/hello2-two-debuginfo-1.0-2.x86_64.rpm \ + debuginfod-rpms/fedora31/hello3-1.0-2.src.rpm \ + debuginfod-rpms/fedora31/hello3-1.0-2.x86_64.rpm \ + debuginfod-rpms/fedora31/hello3-debuginfo-1.0-2.x86_64.rpm \ + debuginfod-rpms/fedora31/hello3-debugsource-1.0-2.x86_64.rpm \ + debuginfod-rpms/fedora31/hello3-two-1.0-2.x86_64.rpm \ + debuginfod-rpms/fedora31/hello3-two-debuginfo-1.0-2.x86_64.rpm \ debuginfod-rpms/hello2.spec. \ debuginfod-rpms/rhel6/hello2-1.0-2.i686.rpm \ debuginfod-rpms/rhel6/hello2-1.0-2.src.rpm \ -- 2.18.2
patch: debuginfod rhel7 vs. zstd rpms
Hi - Author: Frank Ch. Eigler Date: Thu Mar 26 13:48:20 2020 -0400 debuginfod: document and workaround fedora31 zstd compression Old enough (RHEL7 era) rpm/libarchive tools cannot grok fedora31 zstd-compressed rpms. Disable those tests if necessary. Document -Z based workaround for debuginfod users. https://fedoraproject.org/wiki/Changes/Switch_RPMs_to_zstd_compression Signed-off-by: Frank Ch. Eigler diff --git a/doc/ChangeLog b/doc/ChangeLog index 564644f41907..cfb03b380ca0 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * debuginfod.8 (-R): Note zstd compression complications + and workaround. + 2020-03-24 Frank Ch. Eigler * debuginfod-find.1, debuginfod_find_debuginfo.3: Document diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index 64795c245f27..d8fbbea09ee9 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -100,7 +100,10 @@ additional patterns. This option may be repeated. .B "\-R" Activate RPM patterns in archive scanning. The default is off. Equivalent to \fB\%\-Z\~.rpm=cat\fP, since libarchive can natively -process RPM archives. +process RPM archives. If your version of libarchive is much older +than 2020, be aware that some distributions have switched to an +incompatible zstd compression for their payload. You may experiment +with \fB\%\-Z\ .rpm='(rpm2cpio|zstdcat)<'\fP instead of \fB\-R\fP. .TP .B "\-U" diff --git a/tests/ChangeLog b/tests/ChangeLog index b7390a575d6a..5bedd0893cf5 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * run-debuginfod-find.sh: Check for bsdtar zstd capability + for running tests against zstd-compressed fedora31 rpms. + 2020-03-26 Mark Wielaard * Makefile.am (EXTRA_DIST): Add diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index a6958fad758c..e53c18f31f44 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -21,6 +21,8 @@ type curl 2>/dev/null || (echo "need curl"; exit 77) type rpm2cpio 2>/dev/null || (echo "need rpm2cpio"; exit 77) type bzcat 2>/dev/null || (echo "need bzcat"; exit 77) +bsdtar --version +bsdtar --version | grep -q zstd && zstd=true || zstd=false # for test case debugging, uncomment: #set -x @@ -300,8 +302,12 @@ archive_test() { # common source file sha1 SHA=f4a1a8062be998ae93b8f1cd744a398c6de6dbb1 # fedora31 -archive_test 420e9e3308971f4b817cc5bf83928b41a6909d88 /usr/src/debug/hello3-1.0-2.x86_64/foobar./../hello.c $SHA -archive_test 87c08d12c78174f1082b7c888b3238219b0eb265 /usr/src/debug/hello3-1.0-2.x86_64///foobar/./..//hello.c $SHA +if [ $zstd = true ]; then +# fedora31 uses zstd compression on rpms, older rpm2cpio/libarchive can't handle it +# and we're not using the fancy -Z '.rpm=(rpm2cpio|zstdcat)<' workaround in this testsuite +archive_test 420e9e3308971f4b817cc5bf83928b41a6909d88 /usr/src/debug/hello3-1.0-2.x86_64/foobar./../hello.c $SHA +archive_test 87c08d12c78174f1082b7c888b3238219b0eb265 /usr/src/debug/hello3-1.0-2.x86_64///foobar/./..//hello.c $SHA +fi # fedora30 archive_test c36708a78618d597dee15d0dc989f093ca5f9120 /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA archive_test 41a236eb667c362a1c4196018cc4581e09722b1b /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
Re: patch: debuginfod rhel7 vs. zstd rpms
Hi - Committing this version after testing on rhel8. Author: Frank Ch. Eigler Date: Thu Mar 26 13:48:20 2020 -0400 debuginfod: document and workaround fedora31 zstd compression Old enough (even RHEL8 era) rpm/libarchive tools cannot grok fedora31 zstd-compressed rpms. Disable those tests if necessary. Document -Z based workaround for these debuginfod users. https://fedoraproject.org/wiki/Changes/Switch_RPMs_to_zstd_compression Signed-off-by: Frank Ch. Eigler diff --git a/doc/ChangeLog b/doc/ChangeLog index 564644f41907..cfb03b380ca0 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * debuginfod.8 (-R): Note zstd compression complications + and workaround. + 2020-03-24 Frank Ch. Eigler * debuginfod-find.1, debuginfod_find_debuginfo.3: Document diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index 64795c245f27..d8fbbea09ee9 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -100,7 +100,10 @@ additional patterns. This option may be repeated. .B "\-R" Activate RPM patterns in archive scanning. The default is off. Equivalent to \fB\%\-Z\~.rpm=cat\fP, since libarchive can natively -process RPM archives. +process RPM archives. If your version of libarchive is much older +than 2020, be aware that some distributions have switched to an +incompatible zstd compression for their payload. You may experiment +with \fB\%\-Z\ .rpm='(rpm2cpio|zstdcat)<'\fP instead of \fB\-R\fP. .TP .B "\-U" diff --git a/tests/ChangeLog b/tests/ChangeLog index b7390a575d6a..5bedd0893cf5 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * run-debuginfod-find.sh: Check for bsdtar zstd capability + for running tests against zstd-compressed fedora31 rpms. + 2020-03-26 Mark Wielaard * Makefile.am (EXTRA_DIST): Add diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index a6958fad758c..4d8d45e8b6d2 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -21,6 +21,8 @@ type curl 2>/dev/null || (echo "need curl"; exit 77) type rpm2cpio 2>/dev/null || (echo "need rpm2cpio"; exit 77) type bzcat 2>/dev/null || (echo "need bzcat"; exit 77) +bsdtar --version | grep -q zstd && zstd=true || zstd=false +echo "zstd=$zstd bsdtar=`bsdtar --version`" # for test case debugging, uncomment: #set -x @@ -231,6 +233,10 @@ filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID cmp $filename ${PWD}/prog2.c cp -rvp ${abs_srcdir}/debuginfod-rpms R +if [ "$zstd" = "false" ]; then # nuke the zstd fedora 31 ones +rm -vrf R/debuginfod-rpms/fedora31 +fi + cp -rvp ${abs_srcdir}/debuginfod-tars Z kill -USR1 $PID1 # All rpms need to be in the index @@ -300,8 +306,12 @@ archive_test() { # common source file sha1 SHA=f4a1a8062be998ae93b8f1cd744a398c6de6dbb1 # fedora31 -archive_test 420e9e3308971f4b817cc5bf83928b41a6909d88 /usr/src/debug/hello3-1.0-2.x86_64/foobar./../hello.c $SHA -archive_test 87c08d12c78174f1082b7c888b3238219b0eb265 /usr/src/debug/hello3-1.0-2.x86_64///foobar/./..//hello.c $SHA +if [ $zstd = true ]; then +# fedora31 uses zstd compression on rpms, older rpm2cpio/libarchive can't handle it +# and we're not using the fancy -Z '.rpm=(rpm2cpio|zstdcat)<' workaround in this testsuite +archive_test 420e9e3308971f4b817cc5bf83928b41a6909d88 /usr/src/debug/hello3-1.0-2.x86_64/foobar./../hello.c $SHA +archive_test 87c08d12c78174f1082b7c888b3238219b0eb265 /usr/src/debug/hello3-1.0-2.x86_64///foobar/./..//hello.c $SHA +fi # fedora30 archive_test c36708a78618d597dee15d0dc989f093ca5f9120 /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA archive_test 41a236eb667c362a1c4196018cc4581e09722b1b /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
[Bug debuginfod/25583] Use libarchive to extract .deb packages?
https://sourceware.org/bugzilla/show_bug.cgi?id=25583 Frank Ch. Eigler changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #9 from Frank Ch. Eigler --- pushed as debuginfod-internal & uncontroversial -- You are receiving this mail because: You are on the CC list for the bug.
patch PR25448: more debuginfod prometheus metrics
Hi - commit 34d851344dbaa5fd00b4368742839f86203202a1 (HEAD -> fche/pr25448) Author: Frank Ch. Eigler Date: Thu Mar 26 16:44:20 2020 -0400 PR25448: debuginfod: add transfer performance metrics We now export metrics related to the time taken and data sent, from which prometheus type tools can compute nice time series with averages. http_responses_duration_milliseconds_count{code="200"} 63 http_responses_duration_milliseconds_count{code="404"} 2 http_responses_duration_milliseconds_count{code="503"} 1 http_responses_duration_milliseconds_sum{code="200"} 66 http_responses_duration_milliseconds_sum{code="404"} 2 http_responses_duration_milliseconds_sum{code="503"} 0 http_responses_transfer_bytes_count{code="200"} 63 http_responses_transfer_bytes_count{code="404"} 2 http_responses_transfer_bytes_count{code="503"} 1 http_responses_transfer_bytes_sum{code="200"} 425177 http_responses_transfer_bytes_sum{code="404"} 18 http_responses_transfer_bytes_sum{code="503"} 37 Signed-off-by: Frank Ch. Eigler diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 5e19db517472..ebb67b0df378 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * debuginfod.cxx (handler_cb): Export two families of metrics for + prometheus traffic analysis: response times and data amounts. + 2020-03-25 Frank Ch. Eigler * debuginfod.cxx (parse_opt): Associate a bsdtar subshell with diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index c03bbf922f59..e17aec862676 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -1759,6 +1759,7 @@ handler_cb (void * /*cls*/, inc_metric("http_responses_total","result","error"); e.report(clog); http_code = e.code; + http_size = e.message.size(); rc = e.mhd_send_response (connection); } @@ -1770,6 +1771,17 @@ handler_cb (void * /*cls*/, << ' ' << (int)(deltas*1000) << "ms" << endl; + // related prometheus metrics + string http_code_str = to_string(http_code); + if (http_size >= 0) +add_metric("http_responses_transfer_bytes_sum","code",http_code_str, + http_size); + inc_metric("http_responses_transfer_bytes_count","code",http_code_str); + + add_metric("http_responses_duration_milliseconds_sum","code",http_code_str, + deltas*1000); // prometheus prefers _seconds and floating point + inc_metric("http_responses_duration_milliseconds_count","code",http_code_str); + return rc; } diff --git a/tests/ChangeLog b/tests/ChangeLog index c06d892440a0..ba88ecf6aeb9 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2020-03-26 Frank Ch. Eigler + + * run-debuginfod-find.sh: Look for debuginfod's new + http_responses_* metrics. + 2020-03-26 Frank Ch. Eigler * run-debuginfod-find.sh: Look for bsdtar instead of dpkg. diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index cc826f9607a4..db36420f75ec 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -416,6 +416,10 @@ curl -s http://127.0.0.1:$PORT2/metrics curl -s http://127.0.0.1:$PORT1/metrics | grep -q 'http_responses_total.*result.*error' curl -s http://127.0.0.1:$PORT1/metrics | grep -q 'http_responses_total.*result.*fdcache' curl -s http://127.0.0.1:$PORT2/metrics | grep -q 'http_responses_total.*result.*upstream' +curl -s http://127.0.0.1:$PORT1/metrics | grep 'http_responses_duration_milliseconds_count' +curl -s http://127.0.0.1:$PORT1/metrics | grep 'http_responses_duration_milliseconds_sum' +curl -s http://127.0.0.1:$PORT1/metrics | grep 'http_responses_transfer_bytes_count' +curl -s http://127.0.0.1:$PORT1/metrics | grep 'http_responses_transfer_bytes_sum' # And generate a few errors into the second debuginfod's logs, for analysis just below curl -s http://127.0.0.1:$PORT2/badapi > /dev/null || true
[Bug debuginfod/25448] Extend debuginfod metrics
https://sourceware.org/bugzilla/show_bug.cgi?id=25448 Frank Ch. Eigler changed: What|Removed |Added CC||fche at redhat dot com --- Comment #1 from Frank Ch. Eigler --- https://sourceware.org/pipermail/elfutils-devel/2020q1/002555.html -- You are receiving this mail because: You are on the CC list for the bug.
Re: PR25369 slice 3/3: debuginfod header relay
Hi Frank, On Tue, 2020-03-24 at 23:49 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > This is the last of three bits for the month-ago PR25369 patchset. > > Author: Frank Ch. Eigler > Date: Tue Mar 24 23:46:30 2020 -0400 > > debuginfod: User-Agent and X-Forwarded-For header relay > > Extend the debuginfod client API with a function to stuff outgoing > headers into libcurl. Use this from debuginfod so federated trees > of debuginfod/httpd can trace back to to the originating client > for administrative purposes. Docs & test included. I see how this is useful for logging what is going on in the debuginfod server. And even how some client program might want to set the User- Agent itself. But it feels a bit like a hack to make this a public API for the client library. Normally a user client doesn't really need to know much about whether or how the file information is fetched. But in this case we have a http transport specific thing (and we just added support for file:// URLs). It also seems very curl specific. To use it correctly you need to know the structure of HTTP request header/value and how CURLOPT_HTTPHEADER uses the given string to add, replace, delete (end with a colon) or create a header with no value (adding a semicolon at the end). I think that we should be very explicit that this API is an optional hint only. That the user must not rely on the header actually being used or forwarded through the transport. And that it is only to be used for optional logging. I am especially worried that people will start using the API to override and/or disable internal (curl) headers, which would mean we are stuck with all kinds of curl specifics. So ideally I would like to programmatically prevent that. Or at document very clearly that if someone does that, that is totally unsupported and we are free to break such usage in any (minor) update/bug fix release. > diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog > index 00e7ec63232e..44c1349bfd80 100644 > --- a/debuginfod/ChangeLog > +++ b/debuginfod/ChangeLog > @@ -1,3 +1,15 @@ > +2020-03-24 Frank Ch. Eigler > + > + * debuginfod.h, libdebuginfod.map: New functions for _add_url_header. > + * debuginfod-client.c (struct debuginfod_client): Add headers fields. > + (debuginfod_add_http_header): New client api to add outgoing headers. > + (add_default_headers): Renamed from add_extra_headers, skip if flag. > + (debuginfod_query_server): Pass accumulated headers to libcurl. > + (debuginfod_end): Clean accumulated headers. > + (debuginfod_find_*): Add default headers at this point. > + * debuginfod.cxx (handle_buildid): Add conn pointer. Use it to relay > + incoming UA and XFF headers to federated upstream debuginfods. > + > 2020-03-24 Frank Ch. Eigler > > * debuginfod-find.c (main): Correct /source full-pathness check for > diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c > index 58a04b9a734b..6517cb72432f 100644 > --- a/debuginfod/debuginfod-client.c > +++ b/debuginfod/debuginfod-client.c > @@ -85,6 +85,10 @@ struct debuginfod_client >/* Stores current/last url, if any. */ >char* url; > > + /* Accumulates outgoing http header names/values. */ > + int user_agent_set_p; /* affects add_default_headers */ > + struct curl_slist *headers; > + >/* Can contain all other context, like cache_path, server_urls, > timeout or other info gotten from environment variables, the > handle data, etc. So those don't have to be reparsed and > @@ -311,8 +315,11 @@ debuginfod_clean_cache(debuginfod_client *c, > > > static void > -add_extra_headers(CURL *handle) > +add_default_headers(debuginfod_client *client) > { > + if (client->user_agent_set_p) > +return; > + >/* Compute a User-Agent: string to send. The more accurately this > describes this host, the likelier that the debuginfod servers > might be able to locate debuginfo for us. */ > @@ -372,7 +379,7 @@ add_extra_headers(CURL *handle) > } > >char *ua = NULL; > - rc = asprintf(& ua, "%s/%s,%s,%s/%s", > + rc = asprintf(& ua, "User-Agent: %s/%s,%s,%s/%s", > PACKAGE_NAME, PACKAGE_VERSION, > utspart ?: "", > id ?: "", > @@ -381,7 +388,7 @@ add_extra_headers(CURL *handle) > ua = NULL; > >if (ua) > -curl_easy_setopt(handle, CURLOPT_USERAGENT, (void*) ua); /* implicit > strdup */ > +(void) debuginfod_add_http_header (client, ua); > >free (ua); >free (id); > @@ -683,7 +690,7 @@ debuginfod_query_server (debuginfod_client *c, >curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1); >curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, ""); > - add_extra_headers(data[i].handle); > + curl_easy_setopt(data[i].handle, CURLOPT_HTTPHEADER, c->headers); > >curl_
Re: PR25369 slice 3/3: debuginfod header relay
Hi - > I see how this is useful for logging what is going on in the debuginfod > server. And even how some client program might want to set the User- > Agent itself. But it feels a bit like a hack to make this a public API > for the client library. Sure. > Normally a user client doesn't really need to know much about whether > or how the file information is fetched. But in this case we have a http > transport specific thing (and we just added support for file:// URLs). > It also seems very curl specific. To use it correctly you need to know > the structure of HTTP request header/value and how CURLOPT_HTTPHEADER > uses the given string to add, replace, delete (end with a colon) or > create a header with no value (adding a semicolon at the end). I don't see why this would be true. The function is named "add", and the documentation says "Header: value". This does not expose CURL replace/delete operations. > I think that we should be very explicit that this API is an optional > hint only. That the user must not rely on the header actually being > used or forwarded through the transport. And that it is only to be used > for optional logging. I guess I don't see a problem here. HTTP request headers are by definition optional things. How it's used --- why would we want to dictate "only ... for" anything? We do what we do. > I am especially worried that people will start using the API to > override and/or disable internal (curl) headers, which would mean we > are stuck with all kinds of curl specifics. So ideally I would like to > programmatically prevent that. Or at document very clearly that if > someone does that, that is totally unsupported and we are free to break > such usage in any (minor) update/bug fix release. People who manage to muck up curl internal headers would already be violating our documentation ("add", "Header: value"). I just don't see any threat here, let alone any sort of special technical obligation on us to stop this or offer scare stories. The function simply does what it says on the tin. > > void > > debuginfod_end (debuginfod_client *client) > > { > > + if (client) /* make it safe to be invoked with NULL */ > > +curl_slist_free_all (client->headers); > >free (client->url); > >free (client); > > } > > It seems a good idea to allow debuginfod_end (NULL). But if client > would be NULL then the free (client->url) would also crash. > So just put everything under the if (client) { ... } Or if (!client) return or whatever. > > { > > + add_default_headers(client); > >return debuginfod_query_server(client, build_id, build_id_len, > > "source", filename, path); > > } > > Why not have add_default_headers at the top of the > debuginfod_query_server () function instead of repeating it 3 times > here? Sure, OK. > So if there is any way to check whether this is overriding and/or > deleting an existing internal curl header here it would be really good > to add that here and simply reject that. I guess I don't see why we would voluntarily undertake the responsibility for detailed classification of already documentation-violating input. Remember, this is just a client. If some user really needs this sort of goofy capability, they could have always just written their code to libcurl directly. We can't stop them. > >struct MHD_Response *r = 0; > >try > > { > > - r = handle_buildid (buildid, "debuginfo", "", &alt_fd); > > + r = handle_buildid (0, buildid, "debuginfo", "", &alt_fd); > > } > > 0 instead of NULL? > I believe we went over this before, 0 is more C++? Yes. > > +/* Add an outgoing HTTP request "Header: Value". Copies string. */ > > +int debuginfod_add_http_header (debuginfod_client *client, const char* > > header); > > So here at least add "optional header" or something. A warning that > this is a hint only and might be ignored depending on backend/transport > used. Adding a http header to a non-http transport URL is obviously meaningless and harmless. I don't see why we should belabour it or give people any concern about it. - FChE
Re: patch PR25583: debuginfod bsdtar for deb
Hi Frank, On Wed, 2020-03-25 at 10:57 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > Author: Frank Ch. Eigler > Date: Wed Mar 25 10:55:53 2020 -0400 > > PR25583: debuginfod: prefer bsdtar to dpkg for .deb handling > > It turns out a bsdtar subshell can do the job of dpkg-deb. > bsdtar comes from/with libarchive so it should be available > everywhere. The patch itself looks correct. And it is a win for non-deb based systems since the bsdtar dependencies are already there. But for .deb based systems it is more likely they already have dpkg installed, while bsdtar might be a separate install/package. And it still needs to spawn an external process. I looked at the buildbot workers and none had bsdtar installed. I have installed it now on the i386, debian/fedora/centos-x86_64, arm32 and arm64 workers and have requested it to be installed on the s390x and ppc64[le] workers. I am not against this patch, but I do wonder if it should not try to fall back on dpkg if bsdtar isn't installed. Cheers, Mark
Re: patch PR25583: debuginfod bsdtar for deb
Hi - > I am not against this patch, but I do wonder if it should not try to > fall back on dpkg if bsdtar isn't installed. Good point, will do that. - FChE