From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id zPA3J01XImLgCwAAWB0awg (envelope-from ) for ; Fri, 04 Mar 2022 13:15:41 -0500 Received: by simark.ca (Postfix, from userid 112) id 8FCCF1F3CA; Fri, 4 Mar 2022 13:15:41 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id CB7741EA69 for ; Fri, 4 Mar 2022 13:15:40 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EB0A93857C6D for ; Fri, 4 Mar 2022 18:15:39 +0000 (GMT) Received: from alt-proxy28.mail.unifiedlayer.com (alt-proxy28.mail.unifiedlayer.com [74.220.216.123]) by sourceware.org (Postfix) with ESMTPS id E820C3858D39 for ; Fri, 4 Mar 2022 18:15:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E820C3858D39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from cmgw14.mail.unifiedlayer.com (unknown [10.0.90.129]) by progateway1.mail.pro1.eigbox.com (Postfix) with ESMTP id 57C2710040193 for ; Fri, 4 Mar 2022 18:15:28 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id QCSdnKxtP2s5dQCSen27wY; Fri, 04 Mar 2022 18:15:28 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=BOh2EHcG c=1 sm=1 tr=0 ts=62225740 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=o8Y5sQTvuykA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=CCpqsmhAAAAA:8 a=NEAV23lmAAAA:8 a=K1-z-kNxt7b86zuFu64A:9 a=ul9cdbp4aOFLsgKbc677:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=gecNpWR5/s+8Y+1Zd9HnvYYuYZAenJtnYSxh7wWK26c=; b=lEJpePRjCvAr5nPE/PbG6DMwJW npEL2h36ZrWv8soc7Vfk0FamW8Rcob/NpotzbmKpTKyahgFWq7ZqUQhlhyWuOdGy427fD+VAYwsEw c1f2+UQtwfSz1NqwV0asqNRwt; Received: from 75-166-141-253.hlrn.qwest.net ([75.166.141.253]:37928 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nQCSd-003a5m-JX; Fri, 04 Mar 2022 11:15:27 -0700 From: Tom Tromey To: Aaron Merey via Gdb-patches Subject: Re: [PATCH v3] gdb: Improve debuginfod progress updates References: <20220126005817.56356-1-amerey@redhat.com> <20220209022548.343785-1-amerey@redhat.com> X-Attribution: Tom Date: Fri, 04 Mar 2022 11:15:26 -0700 In-Reply-To: <20220209022548.343785-1-amerey@redhat.com> (Aaron Merey via Gdb-patches's message of "Tue, 8 Feb 2022 21:25:48 -0500") Message-ID: <875yotshxd.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 75.166.141.253 X-Source-L: No X-Exim-ID: 1nQCSd-003a5m-JX X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 75-166-141-253.hlrn.qwest.net (murgatroyd) [75.166.141.253]:37928 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: tom@tromey.com Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" >>>>> "Aaron" == Aaron Merey via Gdb-patches writes: Aaron> The new function build_message will try to fit as much of the Aaron> untruncated message as possible. For example if the original Aaron> message is: Aaron> Downloading XX MB separate debug info for /aa/bb/cc/dd/ee Instead of this, which seems like it can hide useful info from the user, what do you think about a "cylon"-style display: Downloading mumble.so: [ # ] ... where the '#' moves back and forth inside the []. See the "indeterminate" example here: https://github.com/p-ranav/indicators (That page made me LOL with the emoji rocket, maybe gdb should be more emojified.) Anyway it seems like this sort of change would avoid some hair in the implementation. Aaron> if (!stream->isatty ()) Aaron> { Aaron> - fprintf_unfiltered (stream, "%s...", meter.name.c_str ()); Aaron> + fprintf_unfiltered (stream, "%s\n", info.name.c_str ()); I suspect the flush can be removed here, it's normally only needed when the previous print doesn't end in a newline. Aaron> + info.state = should_print ? progress_update::START Aaron> + : progress_update::NO_PRINT; gdb style is to parenthesize the RHS of the assignment in a case like this; something like: info.state = (should_print ? progress_update::START : progress_update::NO_PRINT); Aaron> +/* Get the current state of the most recent progress update. */ Aaron> + Aaron> +cli_ui_out::progress_update::state Aaron> +cli_ui_out::get_progress_state () Aaron> +{ I didn't really get this part of the design. It seems to me that the debuginfod support code just wants to notify the user of some progress. It might know how much data to expect, or it might not -- so when first starting the notification, it seems like it can just explain this. After that, it could just say "got xyz", and the progress meter itself can be responsible for all state changes, UI display, etc. That is, I don't understand why this needs to be exported from the cli-out. Aaron> + if (data->progress.has_value ()) Aaron> + data->progress.reset (); In retrospect I also don't understand why the progress meter is even an optional. It seems like one could be made unconditionally and then the display (or not) left to the ui-out implementation. Aaron> + else if (debuginfod_verbose > 0 && fd < 0 && fd != -ENOENT) Aaron> + printf_filtered (_("Download failed: %s. " \ Aaron> + "Continuing without %s %s.\n"), Aaron> + safe_strerror (-fd), data.desc, Aaron> + styled_fname.c_str ()); It seems like a download failure should be reported even in non-verbose mode. Aaron> +void Aaron> +mi_ui_out::do_progress_start (const std::string &name, bool should_print) I tend to doubt that MI should be changed. Technically MI does have a progress notification approach, see mi_load_progress. I don't know if anything MI consumer actually uses this, though, and so I'm not sure if it makes sense to try to wire this up to debuginfo downloads. Tom