git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luke Mewburn <luke@mewburn.net>
To: git@vger.kernel.org
Cc: Nicolas Pitre <nico@fluxnic.net>, Luke Mewburn <luke@mewburn.net>
Subject: [PATCH] reduce progress updates in background
Date: Mon, 13 Apr 2015 23:48:50 +1000	[thread overview]
Message-ID: <20150413134850.GC23475@mewburn.net> (raw)


[-- Attachment #1.1: Type: text/plain, Size: 1300 bytes --]

Hi,

I've noticed that when a long-running git operation that generates
progress output is suspended and converted to a background process,
the terminal still gets spammed with progress updates (to stderr).

Many years ago I fixed a similar issue in the NetBSD ftp progress
bar code (which I wrote).

I've experimented around with a couple of different solutions, including:
1. suppress all progress output whilst in the background
2. suppress "in progress" updates whilst in the background,
   but display the "done" message even if in the background.

In both cases, warnings were still output to the terminal.

I've attached a patch that implements (2) above.

If the consensus is that all progress messages should be suppressed,
I can provide the (simpler) patch for that.

I've explicitly separated the in_progress_fd() function
so that it's easier to (a) reuse elsewhere where appropriate,
and (b) make any portability changes to the test if necessary.
I also used getpgid(0) versus getpgrp() to avoid portability
issues with the signature in the latter with pre-POSIX.

A minor optimisation could be to pass in struct progress *
and to cache getpgid(0) in a member of struct progress
in start_progress_delay(), since this value shouldn't change
during the life of the process.

regards,
Luke.

[-- Attachment #1.2: 0001-progress-no-progress-in-background.patch --]
[-- Type: text/plain, Size: 1941 bytes --]

From 843a367bac87674666dafbaf7fdb7d6b0e1660f7 Mon Sep 17 00:00:00 2001
From: Luke Mewburn <luke@mewburn.net>
Date: Mon, 13 Apr 2015 23:30:51 +1000
Subject: [PATCH] progress: no progress in background

Disable the display of the progress if stderr is not the
current foreground process.
Still display the final result when done.

Signed-off-by: Luke Mewburn <luke@mewburn.net>
---
 progress.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/progress.c b/progress.c
index 412e6b1..8094404 100644
--- a/progress.c
+++ b/progress.c
@@ -72,9 +72,15 @@ static void clear_progress_signal(void)
 	progress_update = 0;
 }
 
+static int is_foreground_fd(int fd)
+{
+	return getpgid(0) == tcgetpgrp(fd);
+}
+
 static int display(struct progress *progress, unsigned n, const char *done)
 {
 	const char *eol, *tp;
+	const int is_foreground = is_foreground_fd(fileno(stderr));
 
 	if (progress->delay) {
 		if (!progress_update || --progress->delay)
@@ -98,16 +104,21 @@ static int display(struct progress *progress, unsigned n, const char *done)
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
 			progress->last_percent = percent;
-			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
-				progress->title, percent, n,
-				progress->total, tp, eol);
-			fflush(stderr);
+			if (is_foreground || done) {
+				fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
+					progress->title, percent, n,
+					progress->total, tp, eol);
+				fflush(stderr);
+			}
 			progress_update = 0;
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
-		fflush(stderr);
+		if (is_foreground || done) {
+			fprintf(stderr, "%s: %u%s%s",
+				progress->title, n, tp, eol);
+			fflush(stderr);
+		}
 		progress_update = 0;
 		return 1;
 	}
-- 
2.3.5.dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

             reply	other threads:[~2015-04-13 13:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-13 13:48 Luke Mewburn [this message]
2015-04-13 14:11 ` [PATCH] reduce progress updates in background Nicolas Pitre
2015-04-13 14:40   ` Luke Mewburn
2015-04-13 15:01     ` Nicolas Pitre
2015-04-14 11:03       ` [PATCH v2] " Luke Mewburn
2015-04-14 15:16         ` Nicolas Pitre
2015-04-15 18:29         ` [PATCH] compat/mingw: stubs for getpgid() and tcgetpgrp() Johannes Sixt
2015-04-15 18:48           ` Junio C Hamano
2015-04-15 20:34             ` Johannes Sixt
2015-04-16 12:48             ` Johannes Schindelin
2015-04-16 15:00               ` Junio C Hamano
2015-04-15 19:43           ` Erik Faye-Lund
2015-04-16 12:44             ` [msysGit] " Johannes Schindelin
2015-04-23 19:25               ` rupert thurner
2015-04-24  3:03                 ` rupert thurner
2015-04-24  6:25                 ` [msysGit] " Johannes Schindelin
2015-04-17  1:25           ` Luke Mewburn
2015-04-14 11:08       ` [PATCH] reduce progress updates in background Luke Mewburn
2015-04-14  3:12 ` brian m. carlson
2015-04-14  8:47   ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150413134850.GC23475@mewburn.net \
    --to=luke@mewburn.net \
    --cc=git@vger.kernel.org \
    --cc=nico@fluxnic.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).