git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Clemens Buchacher <drizzd@aon.at>
Cc: dfowler <davidfowl@gmail.com>,
	git@vger.kernel.org, msysgit@googlegroups.com,
	Paul Betts <paul@github.com>, David Ebbo <david.ebbo@gmail.com>
Subject: Re: 1.7.10 doesn't show file pushstatus
Date: Tue, 1 May 2012 03:33:26 -0400	[thread overview]
Message-ID: <20120501073326.GA21087@sigill.intra.peff.net> (raw)
In-Reply-To: <20120501065832.GA17777@sigill.intra.peff.net>

On Tue, May 01, 2012 at 02:58:32AM -0400, Jeff King wrote:

> > I just updated to msysGit 1.7.10 and I noticed I don't see any details
> > while pushing (like file upload speed and % completion). Was this
> > intentionally removed? If so why?
> 
> No, it's a regression. I can reproduce it easily, and it bisects to
> Clemens' 01fdc21 (push/fetch/clone --no-progress suppresses progress
> output) which went into v1.7.9.2 (and v1.7.10).

Hmm. OK, I think I see what is going on. For most protocols, send_pack
happens without a separate process these days. So the stack is like:

  - transport_push
    - git_transport_push
      - send_pack

where the "progress" flag to send_pack is a boolean; we have already
figured out at the transport layer whether we want progress, and are
telling it what to do. Thus this hunk from 01fdc21 makes sense:

> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index 71f258e..9df341c 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -58,7 +58,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
>                 argv[i++] = "--thin";
>         if (args->use_ofs_delta)
>                 argv[i++] = "--delta-base-offset";
> -       if (args->quiet)
> +       if (args->quiet || !args->progress)
>                 argv[i++] = "-q";
>         if (args->progress)
>                 argv[i++] = "--progress";

If we have been asked not to have progress, then we tell pack-objects
"-q" (which is, for historical reasons, the way to spell "--no-progress"
for pack-objects).

But send-pack is also a command in its own right, and gets invoked
separately for the --stateless-rpc case (i.e., smart http). In that case
you end up with:

  - transport_push
    - transport-helper.c:push_refs
      - [pipes to git-remote-https]
        - remote-curl.c:push_git
          - [forks send-pack]
            - cmd_send_pack
              - send_pack

In this case, send pack gets its arguments from the command-line, not
from the options set at the transport layer. Remote-curl will pass along
"--quiet" if we get that from the transport layer, but it does not
otherwise pass along the "progress" flag. So there are two problems:

  1. send-pack defaults its progress boolean to 0. Before 01fdc21, that
     was OK, because it meant "don't explicitly ask for progress". But
     after 01fdc21 that now means "explicitly ask for no progress", and
     the direct-transport code paths were updated without updating
     cmd_send_pack.

  2. There's no way to tell send-pack explicitly "yes, I would like
     progress, no matter what isatty(2) says". I doubt anybody cares
     much, but it probably makes sense to handle that for the sake of
     completeness.

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

  reply	other threads:[~2012-05-01  7:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120501010609.GA14715@jupiter.local>
2012-05-01  6:58 ` 1.7.10 doesn't show file pushstatus Jeff King
2012-05-01  7:33   ` Jeff King [this message]
2012-05-01  8:40     ` Jeff King
2012-05-01  8:41       ` [PATCH 1/3] send-pack: show progress when isatty(2) Jeff King
2012-05-01  8:42       ` [PATCH 2/3] teach send-pack about --[no-]progress Jeff King
2012-05-01  8:43       ` [PATCH 3/3] t5541: test more combinations of --progress Jeff King
2012-05-01  9:35         ` Clemens Buchacher
2012-05-01  9:37           ` Jeff King
2012-05-01 17:53             ` David Ebbo
2012-05-01 20:45         ` Zbigniew Jędrzejewski-Szmek
2012-05-01 20:53           ` Junio C Hamano
2012-05-01  9:32       ` 1.7.10 doesn't show file pushstatus Clemens Buchacher
2012-05-01 17:33         ` Junio C Hamano
2012-05-02  1:04       ` 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=20120501073326.GA21087@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=david.ebbo@gmail.com \
    --cc=davidfowl@gmail.com \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=msysgit@googlegroups.com \
    --cc=paul@github.com \
    /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).