On Wed, Apr 15, 2015 at 07:34:18PM +1000, Luke Mewburn wrote:
> 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@xxxxxxxxxxx>
> Acked-by: Nicolas Pitre <nico@xxxxxxxxxxx>
> ---
> [...]
> +static int is_foreground_fd(int fd)
> +{
> + return getpgid(0) == tcgetpgrp(fd);
> +}
I've noticed that this patch causes a regression when we are
transmitting progress over the sideband channel of the git protocol. You
can see it pretty easily by cloning something large (like the kernel):
git clone --no-local /path/to/linux.git
The "counting objects" phase is generated by the server side and sent
over the sideband, where we show it locally. So you'll get:
remote: Counting objects: 499771
and so on, progressively, until we get to the final value. But with your
patch, we get silence for tens of seconds, and then the final value.
is_foreground_fd never returns true, and we print only once the "done"
flag is set.
The problem is that tcgetpgrp() returns -1 on the server side, because
of course there is no terminal. I suspect this may also break other
esoteric cases where "--progress" has been explicitly specified, but we
don't actually have a terminal (e.g., even something as simple as "ssh
host 'cd repo && git fsck --progress" exhibits the same behavior).
One reasonable fix (I think) would be to treat an error return from
tcgetpgrp() as "yes, we are the foreground", like:
diff --git a/progress.c b/progress.c
index 43d9228..2e31bec 100644
--- a/progress.c
+++ b/progress.c
@@ -74,7 +74,8 @@ static void clear_progress_signal(void)
static int is_foreground_fd(int fd)
{
- return getpgid(0) == tcgetpgrp(fd);
+ int tpgrp = tcgetpgrp(fd);
+ return tpgrp < 0 || tpgrp == getpgid(0);
}
static int display(struct progress *progress, unsigned n, const char *done)
But I don't know if that messes up any other cases you were trying to
hit. We could also check that errno == ENOTTY, but I'm not sure it's
worth it. Whatever the reason, it probably makes sense to err on the
side of printing the progress.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html