* [PATCH] Remove download size from debuginfod progress messages @ 2022-03-23 0:34 Aaron Merey 2022-03-23 16:02 ` Keith Seitz 0 siblings, 1 reply; 6+ messages in thread From: Aaron Merey @ 2022-03-23 0:34 UTC (permalink / raw) To: gdb-patches Currently debuginfod progress update messages include the size of each download: Downloading 7.5 MB separate debug info /lib/libxyz.so.0 This value originates from the Content-Length HTTP header of the transfer. However this header is not guaranteed to be present for each download. This can happen when debuginfod servers compress files on-the-fly at the time of transfer. In this case gdb wrongly prints "-0.00 MB" as the size. This patch removes download sizes from progress messages for now until a more thorough reworking of progress updating is implemented [1]. [1] https://sourceware.org/pipermail/gdb-patches/2022-February/185798.html --- gdb/debuginfod-support.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index 3614ff23882..ab40d117c9e 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -86,12 +86,12 @@ debuginfod_exec_query (const unsigned char *build_id, struct user_data { user_data (const char *desc, const char *fname) - : desc (desc), fname (fname) + : desc (desc), fname (fname), has_printed (false) { } const char * const desc; const char * const fname; - gdb::optional<ui_out::progress_meter> meter; + bool has_printed; }; /* Deleter for a debuginfod_client. */ @@ -121,25 +121,13 @@ progressfn (debuginfod_client *c, long cur, long total) return 1; } - if (total == 0) - return 0; - - if (!data->meter.has_value ()) + if (!data->has_printed) { - float size_in_mb = 1.0f * total / (1024 * 1024); - string_file styled_filename (current_uiout->can_emit_style_escape ()); - fprintf_styled (&styled_filename, - file_name_style.style (), - "%s", - data->fname); - std::string message - = string_printf ("Downloading %.2f MB %s %s", size_in_mb, data->desc, - styled_filename.c_str()); - data->meter.emplace (current_uiout, message, 1); + data->has_printed = true; + printf_filtered ("Downloading %s %ps...\n", data->desc, + styled_string (file_name_style.style (), data->fname)); } - data->meter->progress ((double)cur / (double)total); - return 0; } -- 2.35.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove download size from debuginfod progress messages 2022-03-23 0:34 [PATCH] Remove download size from debuginfod progress messages Aaron Merey @ 2022-03-23 16:02 ` Keith Seitz 2022-03-23 18:06 ` Aaron Merey 0 siblings, 1 reply; 6+ messages in thread From: Keith Seitz @ 2022-03-23 16:02 UTC (permalink / raw) To: Aaron Merey, gdb-patches On 3/22/22 17:34, Aaron Merey wrote: > Currently debuginfod progress update messages include the size of > each download: > > Downloading 7.5 MB separate debug info /lib/libxyz.so.0 > > This value originates from the Content-Length HTTP header of the > transfer. However this header is not guaranteed to be present for > each download. This can happen when debuginfod servers compress files > on-the-fly at the time of transfer. In this case gdb wrongly prints > "-0.00 MB" as the size. I realize this is an interim patch to address an issue currently under development, and I'm okay with that. There are a lot of people that use devel versions of gdb to do work. [At least I hope I'm not alone!] However, I would really rather not simply remove all download sizes. If we can detect the condition that would print the unhelpful "-0.00 MB", then *just* removing the download size for that case (or printing "unknown" or something) would be my preferred path. YMMV, and I am not a maintainer, etc. Keith ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove download size from debuginfod progress messages 2022-03-23 16:02 ` Keith Seitz @ 2022-03-23 18:06 ` Aaron Merey 2022-03-23 18:56 ` Keith Seitz 2022-03-24 15:17 ` Simon Marchi 0 siblings, 2 replies; 6+ messages in thread From: Aaron Merey @ 2022-03-23 18:06 UTC (permalink / raw) To: keiths; +Cc: kevinb, gdb-patches, Aaron Merey Hi Keith, On Wed, Mar 23, 2022 at 12:02 PM Keith Seitz <keiths@redhat.com> wrote: > On 3/22/22 17:34, Aaron Merey wrote: > > Currently debuginfod progress update messages include the size of > > each download: > > > > Downloading 7.5 MB separate debug info /lib/libxyz.so.0 > > > > This value originates from the Content-Length HTTP header of the > > transfer. However this header is not guaranteed to be present for > > each download. This can happen when debuginfod servers compress files > > on-the-fly at the time of transfer. In this case gdb wrongly prints > > "-0.00 MB" as the size. > > I realize this is an interim patch to address an issue currently > under development, and I'm okay with that. There are a lot of > people that use devel versions of gdb to do work. [At least I > hope I'm not alone!] > > However, I would really rather not simply remove all download > sizes. If we can detect the condition that would print the unhelpful > "-0.00 MB", then *just* removing the download size for that case (or > printing "unknown" or something) would be my preferred path. I wanted to keep this patch as simple as possible in order to avoid getting bogged down by details that might be more appropriate for the patch in-progress that reworks these messages. But I agree that we should continue to print sizes if they are available. I've added this to the patch below. Aaron --- Remove download size from debuginfod progress messages if unavailable Currently debuginfod progress update messages include the size of each download: Downloading 7.5 MB separate debug info /lib/libxyz.so.0 This value originates from the Content-Length HTTP header of the transfer. However this header is not guaranteed to be present for each download. This can happen when debuginfod servers compress files on-the-fly at the time of transfer. In this case gdb wrongly prints "-0.00 MB" as the size. This patch removes download sizes from progress messages when they are not available. It also removes usage of the progress bar until a more thorough reworking of progress updating is implemented. [1] [1] https://sourceware.org/pipermail/gdb-patches/2022-February/185798.html --- gdb/debuginfod-support.c | 44 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index 3614ff23882..d5d023ff7bb 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -86,12 +86,12 @@ debuginfod_exec_query (const unsigned char *build_id, struct user_data { user_data (const char *desc, const char *fname) - : desc (desc), fname (fname) + : desc (desc), fname (fname), has_printed (false) { } const char * const desc; const char * const fname; - gdb::optional<ui_out::progress_meter> meter; + bool has_printed; }; /* Deleter for a debuginfod_client. */ @@ -121,24 +121,32 @@ progressfn (debuginfod_client *c, long cur, long total) return 1; } - if (total == 0) - return 0; - - if (!data->meter.has_value ()) + if (!data->has_printed) { - float size_in_mb = 1.0f * total / (1024 * 1024); - string_file styled_filename (current_uiout->can_emit_style_escape ()); - fprintf_styled (&styled_filename, - file_name_style.style (), - "%s", - data->fname); - std::string message - = string_printf ("Downloading %.2f MB %s %s", size_in_mb, data->desc, - styled_filename.c_str()); - data->meter.emplace (current_uiout, message, 1); - } + /* Include the transfer size, if available. */ + if (total > 0) + { + float size = 1.0f * total / 1024; + const char *unit = "KB"; + + /* If size is greater than 0.01 MB, set unit to MB. */ + if (size > 10.24) + { + size /= 1024; + unit = "MB"; + } + + printf_filtered ("Downloading %.2f %s %s %ps...\n", + size, unit, data->desc, + styled_string (file_name_style.style (), + data->fname)); + } + else + printf_filtered ("Downloading %s %ps...\n", data->desc, + styled_string (file_name_style.style (), data->fname)); - data->meter->progress ((double)cur / (double)total); + data->has_printed = true; + } return 0; } -- 2.35.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove download size from debuginfod progress messages 2022-03-23 18:06 ` Aaron Merey @ 2022-03-23 18:56 ` Keith Seitz 2022-03-24 15:17 ` Simon Marchi 1 sibling, 0 replies; 6+ messages in thread From: Keith Seitz @ 2022-03-23 18:56 UTC (permalink / raw) To: Aaron Merey; +Cc: kevinb, gdb-patches On 3/23/22 11:06, Aaron Merey wrote: > On Wed, Mar 23, 2022 at 12:02 PM Keith Seitz <keiths@redhat.com> wrote: >> On 3/22/22 17:34, Aaron Merey wrote: >> >> However, I would really rather not simply remove all download >> sizes. If we can detect the condition that would print the unhelpful >> "-0.00 MB", then *just* removing the download size for that case (or >> printing "unknown" or something) would be my preferred path. > > I wanted to keep this patch as simple as possible in order to avoid > getting bogged down by details that might be more appropriate for > the patch in-progress that reworks these messages. But I agree that > we should continue to print sizes if they are available. I've added > this to the patch below. I appreciate your patience. I think this is fine, but you will need to wait for a global maintainer to approve. Thanks! Keith ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove download size from debuginfod progress messages 2022-03-23 18:06 ` Aaron Merey 2022-03-23 18:56 ` Keith Seitz @ 2022-03-24 15:17 ` Simon Marchi 2022-03-24 19:04 ` Aaron Merey 1 sibling, 1 reply; 6+ messages in thread From: Simon Marchi @ 2022-03-24 15:17 UTC (permalink / raw) To: Aaron Merey, keiths; +Cc: gdb-patches On 2022-03-23 14:06, Aaron Merey via Gdb-patches wrote: > Hi Keith, > > On Wed, Mar 23, 2022 at 12:02 PM Keith Seitz <keiths@redhat.com> wrote: >> On 3/22/22 17:34, Aaron Merey wrote: >>> Currently debuginfod progress update messages include the size of >>> each download: >>> >>> Downloading 7.5 MB separate debug info /lib/libxyz.so.0 >>> >>> This value originates from the Content-Length HTTP header of the >>> transfer. However this header is not guaranteed to be present for >>> each download. This can happen when debuginfod servers compress files >>> on-the-fly at the time of transfer. In this case gdb wrongly prints >>> "-0.00 MB" as the size. >> >> I realize this is an interim patch to address an issue currently >> under development, and I'm okay with that. There are a lot of >> people that use devel versions of gdb to do work. [At least I >> hope I'm not alone!] >> >> However, I would really rather not simply remove all download >> sizes. If we can detect the condition that would print the unhelpful >> "-0.00 MB", then *just* removing the download size for that case (or >> printing "unknown" or something) would be my preferred path. > > I wanted to keep this patch as simple as possible in order to avoid > getting bogged down by details that might be more appropriate for > the patch in-progress that reworks these messages. But I agree that > we should continue to print sizes if they are available. I've added > this to the patch below. > > Aaron > > --- > Remove download size from debuginfod progress messages if unavailable > > Currently debuginfod progress update messages include the size of > each download: > > Downloading 7.5 MB separate debug info /lib/libxyz.so.0 > > This value originates from the Content-Length HTTP header of the > transfer. However this header is not guaranteed to be present for > each download. This can happen when debuginfod servers compress files > on-the-fly at the time of transfer. In this case gdb wrongly prints > "-0.00 MB" as the size. > > This patch removes download sizes from progress messages when they are > not available. It also removes usage of the progress bar until > a more thorough reworking of progress updating is implemented. [1] > > [1] https://sourceware.org/pipermail/gdb-patches/2022-February/185798.html I'm fine with an interim patch, but I'd really like to see the progress bar back, I enjoy it :)! I haven't been involved in those dicussions so I don't know what the plan is. But typically progress bars have an "undefined total" mode where you can still tell that the operation is progressing (bytes are being received), you just don't what percentage of the total. But really, is it a problem for GDB master to print "~0.00 MB", until the "real" fix is merged? It's not like it's causing a crash or anything like that? Or is it something you want to push to the GDB 12 branch? The patch LGTM in any case, if that's the route you want to take. Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Remove download size from debuginfod progress messages 2022-03-24 15:17 ` Simon Marchi @ 2022-03-24 19:04 ` Aaron Merey 0 siblings, 0 replies; 6+ messages in thread From: Aaron Merey @ 2022-03-24 19:04 UTC (permalink / raw) To: Simon Marchi; +Cc: Keith Seitz, gdb-patches Hi Simon, On Thu, Mar 24, 2022 at 11:26 AM Simon Marchi <simark@simark.ca> wrote: > I'm fine with an interim patch, but I'd really like to see the progress > bar back, I enjoy it :)! > > I haven't been involved in those dicussions so I don't know what the > plan is. But typically progress bars have an "undefined total" mode > where you can still tell that the operation is progressing (bytes are > being received), you just don't what percentage of the total. That is probably what we'll end up doing. The progress bar will return! > But really, is it a problem for GDB master to print "~0.00 MB", until > the "real" fix is merged? It's not like it's causing a crash or > anything like that? Or is it something you want to push to the GDB 12 > branch? We received a request to fix this issue and thought it would be best to patch it here first before addressing it downstream. > The patch LGTM in any case, if that's the route you want to take. Thanks. Pushed as commit b0e0d830d9. Aaron ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-24 19:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-23 0:34 [PATCH] Remove download size from debuginfod progress messages Aaron Merey 2022-03-23 16:02 ` Keith Seitz 2022-03-23 18:06 ` Aaron Merey 2022-03-23 18:56 ` Keith Seitz 2022-03-24 15:17 ` Simon Marchi 2022-03-24 19:04 ` Aaron Merey
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 read-only IMAP folder(s) and NNTP newsgroup(s).