Re: Push not writing to standard error

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:46

Jeff King [off-list ref] writes:

On Tue, Oct 12, 2010 at 03:32:04PM -0400, Jeff King wrote:

quoted

It looks like transport_set_verbosity gets called correctly, and then
sets the "progress" flag for the transport. But for the push side, I
don't see any transports actually looking at that flag. I think there
needs to be code in git_transport_push to handle the progress flag, and
it just isn't there.

Here's a quick 5-minute patch. It works on my test case:

  rm -rf parent child
  git init parent &&
  git clone parent child &&
  cd child &&
  echo content >file && git add file && git commit -m one &&
  git push --progress origin master:foo >foo.out 2>&1 &&
  cat foo.out

Does it still work with "git push" without --progress?  I didn't apply nor
test, but just wondering as the manpage description suggests progress is
implicitly set when standard error is terminal even when there is no
command line --progress is given, and also interaction with -q option, but
the patch does not seem to show such subtleties...

quoted hunk

but I didn't even run the test suite. Maybe somebody more clueful in the
area can pick it up?

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 481602d..efd9be6 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c

@@ -48,6 +48,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 		NULL,
 		NULL,
 		NULL,
+		NULL,
 	};
 	struct child_process po;
 	int i;

@@ -59,6 +60,8 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 		argv[i++] = "--delta-base-offset";
 	if (args->quiet)
 		argv[i++] = "-q";
+	if (args->progress)
+		argv[i++] = "--progress";
 	memset(&po, 0, sizeof(po));
 	po.argv = argv;
 	po.in = -1;

diff --git a/send-pack.h b/send-pack.h
index 60b4ba6..fcf4707 100644
--- a/send-pack.h
+++ b/send-pack.h

@@ -4,6 +4,7 @@
 struct send_pack_args {
 	unsigned verbose:1,
 		quiet:1,
+		progress:1,
 		porcelain:1,
 		send_mirror:1,
 		force_update:1,

diff --git a/transport.c b/transport.c
index 4dba6f8..0078660 100644
--- a/transport.c
+++ b/transport.c

@@ -789,6 +789,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.use_thin_pack = data->options.thin;
 	args.verbose = (transport->verbose > 0);
 	args.quiet = (transport->verbose < 0);
+	args.progress = transport->progress;
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
 	args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);

Re: Push not writing to standard error

From: Jeff King <hidden>
Date: 2016-06-15 22:49:46

On Wed, Oct 13, 2010 at 10:33:22AM -0700, Junio C Hamano wrote:

quoted

Here's a quick 5-minute patch. It works on my test case:

  rm -rf parent child
  git init parent &&
  git clone parent child &&
  cd child &&
  echo content >file && git add file && git commit -m one &&
  git push --progress origin master:foo >foo.out 2>&1 &&
  cat foo.out

Does it still work with "git push" without --progress?  I didn't apply nor
test, but just wondering as the manpage description suggests progress is
implicitly set when standard error is terminal even when there is no
command line --progress is given, and also interaction with -q option, but
the patch does not seem to show such subtleties...

Yes, it works in both of those cases. The transport code already does
the right thing to set transport->progress (see the code at the end of
transport_set_verbosity). And we even pass that value on to remote
helpers, which presumably make use of it. But the internal
git_transport_push simply ignored it (probably because it predates the
rest of the transport code, but I didn't check).

What concerns me a bit is that "git push --no-progress" does not do what
I expected (turn off progress, but keep the status table which would
otherwise be suppressed by "-q"). Instead, --no-progress is silently
ignored. We should at least set it to NONEG to generate an error, but
ideally we would handle it properly.

However, that bug exists with or without my patch. The transport code
seems to only ever consider "force progress" or "default progress", but
never "no progress".

-Peff

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help