* how to suppress progress percentage in git-push @ 2009-11-22 14:53 bill lam 2009-11-23 15:00 ` Jeff King 0 siblings, 1 reply; 16+ messages in thread From: bill lam @ 2009-11-22 14:53 UTC (permalink / raw) To: git I set crontab to push to another computer for backup. It sent confirmation email after finished. It looked like Counting objects: 1 Counting objects: 9, done. Delta compression using up to 2 threads. Compressing objects: 20% (1/5) Compressing objects: 40% (2/5) Compressing objects: 60% (3/5) Compressing objects: 80% (4/5) Compressing objects: 100% (5/5) Compressing objects: 100% (5/5), done. Writing objects: 20% (1/5) Writing objects: 40% (2/5) Writing objects: 60% (3/5) Writing objects: 80% (4/5) Writing objects: 100% (5/5) Writing objects: 100% (5/5), 549 bytes, done. Total 5 (delta 3), reused 0 (delta 0) Often the list of progress % can be a page long. I want output but not those percentage progress status. Will that be possible? -- regards, ==================================================== GPG key 1024D/4434BAB3 2008-08-24 gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-22 14:53 how to suppress progress percentage in git-push bill lam @ 2009-11-23 15:00 ` Jeff King 2009-11-23 15:50 ` Petr Baudis 2009-11-23 16:56 ` how to suppress progress percentage in git-push Nicolas Pitre 0 siblings, 2 replies; 16+ messages in thread From: Jeff King @ 2009-11-23 15:00 UTC (permalink / raw) To: bill lam; +Cc: Nicolas Pitre, git On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote: > I set crontab to push to another computer for backup. It sent > confirmation email after finished. It looked like > > Counting objects: 1 > Counting objects: 9, done. > Delta compression using up to 2 threads. > Compressing objects: 20% (1/5) > Compressing objects: 40% (2/5) > Compressing objects: 60% (3/5) > Compressing objects: 80% (4/5) > Compressing objects: 100% (5/5) > Compressing objects: 100% (5/5), done. > Writing objects: 20% (1/5) > Writing objects: 40% (2/5) > Writing objects: 60% (3/5) > Writing objects: 80% (4/5) > Writing objects: 100% (5/5) > Writing objects: 100% (5/5), 549 bytes, done. > Total 5 (delta 3), reused 0 (delta 0) > > Often the list of progress % can be a page long. I want output but > not those percentage progress status. Will that be possible? Hmm. There seems to be a bug. pack-objects is supposed to see that stderr is not a tty and suppress the progress messages. But it doesn't, because send-pack gives it the --all-progress flag, which unconditionally tells it to display progress, when the desired impact is actually to just make the progress more verbose. We need to do one of: 1. make --all-progress imply "if we are using progress, then make it more verbose. Otherwise, ignore." 2. fix all callers to check isatty(2) before unconditionally passing the option The patch for (1) would look something like what's below. It's simpler, but it does change the semantics; anyone who was relying on --all-progress to turn on progress unconditionally would need to now also use --progress. However, turning on progress unconditionally is usually an error (the except is if you are piping output in real-time to the user and need to overcome the isatty check). Nicolas, this is your code. Which do you prefer? -Peff --- diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 4c91e94..50dd429 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -75,6 +75,7 @@ static int ignore_packed_keep; static int allow_ofs_delta; static const char *base_name; static int progress = 1; +static int all_progress; static int window = 10; static uint32_t pack_size_limit, pack_size_limit_cfg; static int depth = 50; @@ -2220,7 +2221,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) continue; } if (!strcmp("--all-progress", arg)) { - progress = 2; + all_progress = 1; continue; } if (!strcmp("-q", arg)) { @@ -2295,6 +2296,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) usage(pack_usage); } + if (progress && all_progress) + progress = 2; + /* Traditionally "pack-objects [options] base extra" failed; * we would however want to take refs parameter that would * have been given to upstream rev-list ourselves, which means ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 15:00 ` Jeff King @ 2009-11-23 15:50 ` Petr Baudis 2009-11-23 16:43 ` Jeff King 2009-11-23 16:56 ` how to suppress progress percentage in git-push Nicolas Pitre 1 sibling, 1 reply; 16+ messages in thread From: Petr Baudis @ 2009-11-23 15:50 UTC (permalink / raw) To: Jeff King; +Cc: bill lam, Nicolas Pitre, git On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote: > The patch for (1) would look something like what's below. It's simpler, > but it does change the semantics; anyone who was relying on > --all-progress to turn on progress unconditionally would need to now > also use --progress. However, turning on progress unconditionally is > usually an error (the except is if you are piping output in real-time to > the user and need to overcome the isatty check). I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I would be unhappy if I would have to go through creating ptys or whatever now. Maybe conditioning this by an environment variable? Petr "Pasky" Baudis ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 15:50 ` Petr Baudis @ 2009-11-23 16:43 ` Jeff King 2009-11-23 17:05 ` Petr Baudis 2009-11-23 17:43 ` [PATCH] pack-objects: split implications of --all-progress from progress activation Nicolas Pitre 0 siblings, 2 replies; 16+ messages in thread From: Jeff King @ 2009-11-23 16:43 UTC (permalink / raw) To: Petr Baudis; +Cc: bill lam, Nicolas Pitre, git On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote: > On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote: > > The patch for (1) would look something like what's below. It's simpler, > > but it does change the semantics; anyone who was relying on > > --all-progress to turn on progress unconditionally would need to now > > also use --progress. However, turning on progress unconditionally is > > usually an error (the except is if you are piping output in real-time to > > the user and need to overcome the isatty check). > > I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I > would be unhappy if I would have to go through creating ptys or whatever > now. Maybe conditioning this by an environment variable? You wouldn't need to do anything that drastic. You would just need to pass "--progress --all-progress" instead of only --all-progress. But you have provided the data point that such a change would break at least one user. We could also leave --all-progress as-is and add new option to mean "if you are already doing progress, do all progress". -Peff ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 16:43 ` Jeff King @ 2009-11-23 17:05 ` Petr Baudis 2009-11-23 19:28 ` Jeff King 2009-11-23 17:43 ` [PATCH] pack-objects: split implications of --all-progress from progress activation Nicolas Pitre 1 sibling, 1 reply; 16+ messages in thread From: Petr Baudis @ 2009-11-23 17:05 UTC (permalink / raw) To: Jeff King; +Cc: bill lam, Nicolas Pitre, git On Mon, Nov 23, 2009 at 11:43:19AM -0500, Jeff King wrote: > On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote: > > > On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote: > > > The patch for (1) would look something like what's below. It's simpler, > > > but it does change the semantics; anyone who was relying on > > > --all-progress to turn on progress unconditionally would need to now > > > also use --progress. However, turning on progress unconditionally is > > > usually an error (the except is if you are piping output in real-time to > > > the user and need to overcome the isatty check). > > > > I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I > > would be unhappy if I would have to go through creating ptys or whatever > > now. Maybe conditioning this by an environment variable? > > You wouldn't need to do anything that drastic. You would just need to > pass "--progress --all-progress" instead of only --all-progress. But you > have provided the data point that such a change would break at least one > user. > > We could also leave --all-progress as-is and add new option to mean "if > you are already doing progress, do all progress". Hmm, maybe I'm confused - I just call git remote update and don't pass any progress switches - would your change still affect me? Can I pass --progress to `git remote update`? -- Petr "Pasky" Baudis A lot of people have my books on their bookshelves. That's the problem, they need to read them. -- Don Knuth ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 17:05 ` Petr Baudis @ 2009-11-23 19:28 ` Jeff King 0 siblings, 0 replies; 16+ messages in thread From: Jeff King @ 2009-11-23 19:28 UTC (permalink / raw) To: Petr Baudis; +Cc: bill lam, Nicolas Pitre, git On Mon, Nov 23, 2009 at 06:05:47PM +0100, Petr Baudis wrote: > > You wouldn't need to do anything that drastic. You would just need to > > pass "--progress --all-progress" instead of only --all-progress. But you > > have provided the data point that such a change would break at least one > > user. > > > > We could also leave --all-progress as-is and add new option to mean "if > > you are already doing progress, do all progress". > > Hmm, maybe I'm confused - I just call > > git remote update > > and don't pass any progress switches - would your change still affect > me? Can I pass --progress to `git remote update`? Oh, I misunderstood; I thought you were calling pack-objects directly. So you are actually relying on the "even though isatty(2) is not true, we always print progress messages" behavior? I think that behavior is buggy. It hurts everybody pushing via cron, and it violates the usual rule for when we show progress messages. I don't think you can get a --progress pushed all the way down to the pack-objects in this case; we would need to add code to override the isatty check. That being said, your example of "remote update" means you are dealing with fetch, and we are not touching the fetch code path at all. -Peff ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] pack-objects: split implications of --all-progress from progress activation 2009-11-23 16:43 ` Jeff King 2009-11-23 17:05 ` Petr Baudis @ 2009-11-23 17:43 ` Nicolas Pitre 2009-11-23 18:12 ` Petr Baudis 2009-11-23 19:32 ` Jeff King 1 sibling, 2 replies; 16+ messages in thread From: Nicolas Pitre @ 2009-11-23 17:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Petr Baudis, bill lam, git Currently the --all-progress flag is used to use force progress display during the writing object phase even if output goes to stdout which is primarily the case during a push operation. This has the unfortunate side effect of forcing progress display even if stderr is not a terminal. Let's introduce the --all-progress-implied argument which has the same intent except for actually forcing the activation of any progress display. With this, progress display will be automatically inhibited whenever stderr is not a terminal, or full progress display will be included otherwise. This should let people use 'git push' within a cron job without filling their logs with useless percentage displays. Signed-off-by: Nicolas Pitre <nico@fluxnic.net> --- On Mon, 23 Nov 2009, Jeff King wrote: > We could also leave --all-progress as-is and add new option to mean "if > you are already doing progress, do all progress". Actually, that's what I was working on when I saw the above. diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index 2e49929..f54d433 100644 --- a/Documentation/git-pack-objects.txt +++ b/Documentation/git-pack-objects.txt @@ -9,8 +9,9 @@ git-pack-objects - Create a packed archive of objects SYNOPSIS -------- [verse] -'git pack-objects' [-q] [--no-reuse-delta] [--delta-base-offset] [--non-empty] - [--local] [--incremental] [--window=N] [--depth=N] [--all-progress] +'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied] + [--no-reuse-delta] [--delta-base-offset] [--non-empty] + [--local] [--incremental] [--window=N] [--depth=N] [--revs [--unpacked | --all]*] [--stdout | base-name] [--keep-true-parents] < object-list @@ -137,7 +138,7 @@ base-name:: --all-progress:: When --stdout is specified then progress report is - displayed during the object count and deltification phases + displayed during the object count and compression phases but inhibited during the write-out phase. The reason is that in some cases the output stream is directly linked to another command which may wish to display progress @@ -146,6 +147,11 @@ base-name:: report for the write-out phase as well even if --stdout is used. +--all-progress-implied:: + This is used to imply --all-progress whenever progress display + is activated. Unlike --all-progress this flag doesn't actually + force any progress display by itself. + -q:: This flag makes the command not to report its progress on the standard error stream. diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 4c91e94..4429d53 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -24,6 +24,7 @@ static const char pack_usage[] = "git pack-objects [{ -q | --progress | --all-progress }]\n" + " [--all-progress-implied]\n" " [--max-pack-size=N] [--local] [--incremental]\n" " [--window=N] [--window-memory=N] [--depth=N]\n" " [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\n" @@ -2124,6 +2125,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) { int use_internal_rev_list = 0; int thin = 0; + int all_progress_implied = 0; uint32_t i; const char **rp_av; int rp_ac_alloc = 64; @@ -2223,6 +2225,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) progress = 2; continue; } + if (!strcmp("--all-progress-implied", arg)) { + all_progress_implied = 1; + continue; + } if (!strcmp("-q", arg)) { progress = 0; continue; @@ -2326,6 +2332,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (keep_unreachable && unpack_unreachable) die("--keep-unreachable and --unpack-unreachable are incompatible."); + if (progress && all_progress_implied) + progress = 2; + prepare_packed_git(); if (progress) diff --git a/builtin-send-pack.c b/builtin-send-pack.c index d26997b..8fffdbf 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -40,7 +40,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext */ const char *argv[] = { "pack-objects", - "--all-progress", + "--all-progress-implied", "--revs", "--stdout", NULL, diff --git a/bundle.c b/bundle.c index e04ab49..ff97adc 100644 --- a/bundle.c +++ b/bundle.c @@ -343,7 +343,7 @@ int create_bundle(struct bundle_header *header, const char *path, /* write pack */ argv_pack[0] = "pack-objects"; - argv_pack[1] = "--all-progress"; + argv_pack[1] = "--all-progress-implied"; argv_pack[2] = "--stdout"; argv_pack[3] = "--thin"; argv_pack[4] = NULL; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] pack-objects: split implications of --all-progress from progress activation 2009-11-23 17:43 ` [PATCH] pack-objects: split implications of --all-progress from progress activation Nicolas Pitre @ 2009-11-23 18:12 ` Petr Baudis 2009-11-23 18:27 ` Nicolas Pitre 2009-11-23 19:32 ` Jeff King 1 sibling, 1 reply; 16+ messages in thread From: Petr Baudis @ 2009-11-23 18:12 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Junio C Hamano, Jeff King, bill lam, git On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote: > Currently the --all-progress flag is used to use force progress display > during the writing object phase even if output goes to stdout which is > primarily the case during a push operation. This has the unfortunate > side effect of forcing progress display even if stderr is not a > terminal. > > Let's introduce the --all-progress-implied argument which has the same > intent except for actually forcing the activation of any progress > display. With this, progress display will be automatically inhibited > whenever stderr is not a terminal, or full progress display will be > included otherwise. This should let people use 'git push' within a cron > job without filling their logs with useless percentage displays. > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> Ok, but what is currently the way to force the old behaviour? I believe that should be also part of the commit message. Naive deduction fails: $ git remote update --progress error: unknown option `progress' Thanks, -- Petr "Pasky" Baudis A lot of people have my books on their bookshelves. That's the problem, they need to read them. -- Don Knuth ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] pack-objects: split implications of --all-progress from progress activation 2009-11-23 18:12 ` Petr Baudis @ 2009-11-23 18:27 ` Nicolas Pitre 2009-11-23 19:04 ` Petr Baudis 0 siblings, 1 reply; 16+ messages in thread From: Nicolas Pitre @ 2009-11-23 18:27 UTC (permalink / raw) To: Petr Baudis; +Cc: Junio C Hamano, Jeff King, bill lam, git On Mon, 23 Nov 2009, Petr Baudis wrote: > On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote: > > Currently the --all-progress flag is used to use force progress display > > during the writing object phase even if output goes to stdout which is > > primarily the case during a push operation. This has the unfortunate > > side effect of forcing progress display even if stderr is not a > > terminal. > > > > Let's introduce the --all-progress-implied argument which has the same > > intent except for actually forcing the activation of any progress > > display. With this, progress display will be automatically inhibited > > whenever stderr is not a terminal, or full progress display will be > > included otherwise. This should let people use 'git push' within a cron > > job without filling their logs with useless percentage displays. > > > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> > > Ok, but what is currently the way to force the old behaviour? If any existing out-of-tree users of pack-objects were using --all-progress then nothing has changed for them. > I believe that should be also part of the commit message. > > Naive deduction fails: > > $ git remote update --progress > error: unknown option `progress' Usage of 'git remote" is about fetching and not pushing, right? My patch only affects pushes. So I don't know what old behavior you're after. Nicolas ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] pack-objects: split implications of --all-progress from progress activation 2009-11-23 18:27 ` Nicolas Pitre @ 2009-11-23 19:04 ` Petr Baudis 0 siblings, 0 replies; 16+ messages in thread From: Petr Baudis @ 2009-11-23 19:04 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Junio C Hamano, Jeff King, bill lam, git On Mon, Nov 23, 2009 at 01:27:14PM -0500, Nicolas Pitre wrote: > On Mon, 23 Nov 2009, Petr Baudis wrote: > > Naive deduction fails: > > > > $ git remote update --progress > > error: unknown option `progress' > > Usage of 'git remote" is about fetching and not pushing, right? My patch > only affects pushes. So I don't know what old behavior you're after. Oh, I'm sorry, in that case I was confused from the very beginning; somehow I saw pull instead of push in the initial mail. Petr "Pasky" Baudis ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] pack-objects: split implications of --all-progress from progress activation 2009-11-23 17:43 ` [PATCH] pack-objects: split implications of --all-progress from progress activation Nicolas Pitre 2009-11-23 18:12 ` Petr Baudis @ 2009-11-23 19:32 ` Jeff King 1 sibling, 0 replies; 16+ messages in thread From: Jeff King @ 2009-11-23 19:32 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Junio C Hamano, Petr Baudis, bill lam, git On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote: > Currently the --all-progress flag is used to use force progress display > during the writing object phase even if output goes to stdout which is > primarily the case during a push operation. This has the unfortunate > side effect of forcing progress display even if stderr is not a > terminal. > > Let's introduce the --all-progress-implied argument which has the same > intent except for actually forcing the activation of any progress > display. With this, progress display will be automatically inhibited > whenever stderr is not a terminal, or full progress display will be > included otherwise. This should let people use 'git push' within a cron > job without filling their logs with useless percentage displays. > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> Thanks, Tested-by: Jeff King <peff@peff.net> -Peff ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 15:00 ` Jeff King 2009-11-23 15:50 ` Petr Baudis @ 2009-11-23 16:56 ` Nicolas Pitre 2009-11-23 19:25 ` Jeff King 2009-11-24 1:13 ` bill lam 1 sibling, 2 replies; 16+ messages in thread From: Nicolas Pitre @ 2009-11-23 16:56 UTC (permalink / raw) To: Jeff King; +Cc: bill lam, git On Mon, 23 Nov 2009, Jeff King wrote: > On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote: > > > I set crontab to push to another computer for backup. It sent > > confirmation email after finished. It looked like > > > > Counting objects: 1 > > Counting objects: 9, done. > > Delta compression using up to 2 threads. > > Compressing objects: 20% (1/5) > > Compressing objects: 40% (2/5) > > Compressing objects: 60% (3/5) > > Compressing objects: 80% (4/5) > > Compressing objects: 100% (5/5) > > Compressing objects: 100% (5/5), done. > > Writing objects: 20% (1/5) > > Writing objects: 40% (2/5) > > Writing objects: 60% (3/5) > > Writing objects: 80% (4/5) > > Writing objects: 100% (5/5) > > Writing objects: 100% (5/5), 549 bytes, done. > > Total 5 (delta 3), reused 0 (delta 0) > > > > Often the list of progress % can be a page long. I want output but > > not those percentage progress status. Will that be possible? > > Hmm. There seems to be a bug. pack-objects is supposed to see that > stderr is not a tty and suppress the progress messages. But it doesn't, > because send-pack gives it the --all-progress flag, which > unconditionally tells it to display progress, when the desired impact is > actually to just make the progress more verbose. Not exactly. First, the progress variable is initialized with the result of isatty(2) by default. The --progress argument is there to override that since pack-objects is often executed on the sending end of a fetch operation where stderr is not a terminal. Then, during the pack-objects process, there are 3 phases: counting objects, compressing objects, and writing objects. However in the fetch case we prefer to let the receiving end (index-pack or unpack-objects) take care of the progress display during the third phase. This is why by default pack-objects doesn't display any "writing objects" progress when the generated pack is sent to stdout. The --all-progress argument is there to override that, namely for a push. The fact that --all-progress implies --progress is a bad side effect which wouldn't need to exist, but I don't think this is the cause of the issue here. > We need to do one of: > > 1. make --all-progress imply "if we are using progress, then make it > more verbose. Otherwise, ignore." > > 2. fix all callers to check isatty(2) before unconditionally passing > the option None of the above would fix the issue as this only affects progress display for phase #3. You'd still get progress display for the counting phase and the compressing phase. That doesn't mean it is OK for send-pack to unconditionally use --all-progress though, although it does provide the -q argument to pack-objects when push -q is used which inhibits any progress display already. Nicolas ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 16:56 ` how to suppress progress percentage in git-push Nicolas Pitre @ 2009-11-23 19:25 ` Jeff King 2009-11-23 19:40 ` Nicolas Pitre 2009-11-24 1:13 ` bill lam 1 sibling, 1 reply; 16+ messages in thread From: Jeff King @ 2009-11-23 19:25 UTC (permalink / raw) To: Nicolas Pitre; +Cc: bill lam, git On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote: > > We need to do one of: > > > > 1. make --all-progress imply "if we are using progress, then make it > > more verbose. Otherwise, ignore." > > > > 2. fix all callers to check isatty(2) before unconditionally passing > > the option > > None of the above would fix the issue as this only affects progress > display for phase #3. You'd still get progress display for the counting > phase and the compressing phase. I think it does fix the issue, as it is exactly the side effect that we are concerned about (and I tested my patch for 1, which squelched it). But your --all-progress-implied is a better fix, so I think we should go with that. > That doesn't mean it is OK for send-pack to unconditionally use > --all-progress though, although it does provide the -q argument to > pack-objects when push -q is used which inhibits any progress display > already. It does, and I almost suggested that (although -q is new as of 1.6.5, so it may not help the original poster). But "-q" actually does more than that; it also silences any output for a successful push, which may not be desired. -Peff ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 19:25 ` Jeff King @ 2009-11-23 19:40 ` Nicolas Pitre 0 siblings, 0 replies; 16+ messages in thread From: Nicolas Pitre @ 2009-11-23 19:40 UTC (permalink / raw) To: Jeff King; +Cc: bill lam, git On Mon, 23 Nov 2009, Jeff King wrote: > On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote: > > > > We need to do one of: > > > > > > 1. make --all-progress imply "if we are using progress, then make it > > > more verbose. Otherwise, ignore." > > > > > > 2. fix all callers to check isatty(2) before unconditionally passing > > > the option > > > > None of the above would fix the issue as this only affects progress > > display for phase #3. You'd still get progress display for the counting > > phase and the compressing phase. > > I think it does fix the issue, as it is exactly the side effect that we > are concerned about (and I tested my patch for 1, which squelched it). Sorry, you were right. I somehow understood something else initially. > But your --all-progress-implied is a better fix, so I think we should go > with that. OK. Nicolas ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-23 16:56 ` how to suppress progress percentage in git-push Nicolas Pitre 2009-11-23 19:25 ` Jeff King @ 2009-11-24 1:13 ` bill lam 2009-11-24 3:07 ` Jeff King 1 sibling, 1 reply; 16+ messages in thread From: bill lam @ 2009-11-24 1:13 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Jeff King, git On Mon, 23 Nov 2009, Nicolas Pitre wrote: > Then, during the pack-objects process, there are 3 phases: counting > objects, compressing objects, and writing objects. However in the fetch during git-gc it shows yet another progress Removing duplicate objects: 100% (256/256), done. -- regards, ==================================================== GPG key 1024D/4434BAB3 2008-08-24 gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: how to suppress progress percentage in git-push 2009-11-24 1:13 ` bill lam @ 2009-11-24 3:07 ` Jeff King 0 siblings, 0 replies; 16+ messages in thread From: Jeff King @ 2009-11-24 3:07 UTC (permalink / raw) To: bill lam; +Cc: Junio C Hamano, Nicolas Pitre, git On Tue, Nov 24, 2009 at 09:13:39AM +0800, bill lam wrote: > On Mon, 23 Nov 2009, Nicolas Pitre wrote: > > Then, during the pack-objects process, there are 3 phases: counting > > objects, compressing objects, and writing objects. However in the fetch > > during git-gc it shows yet another progress > > Removing duplicate objects: 100% (256/256), done. Thanks, this doesn't seem to have been guarded at all (but since it is on a 2-second delay, you have to have quite a lot of loose objects or a slow disk to trigger it). We should apply the patch below to keep things consistent. I also checked every other call to start_progress; everything else seems to be guarded. Most of them were easy to trace to an isatty check, though the one in unpack-trees is influenced by o->verbose_update. That in turn usually corresponds to a quiet option, though merge does seem to use it unconditionally. Maybe that should be tweaked, too? -- >8 -- Subject: [PATCH] prune-packed: only show progress when stderr is a tty This matches the behavior of other git programs, and helps keep cruft out of things like cron job output. Signed-off-by: Jeff King <peff@peff.net> --- builtin-prune-packed.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index be99eb0..f9463de 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -71,7 +71,7 @@ void prune_packed_objects(int opts) int cmd_prune_packed(int argc, const char **argv, const char *prefix) { - int opts = VERBOSE; + int opts = isatty(2) ? VERBOSE : 0; const struct option prune_packed_options[] = { OPT_BIT('n', "dry-run", &opts, "dry run", DRY_RUN), OPT_NEGBIT('q', "quiet", &opts, "be quiet", VERBOSE), -- 1.6.6.rc0.249.g9b4cf.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2009-11-24 3:07 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-11-22 14:53 how to suppress progress percentage in git-push bill lam 2009-11-23 15:00 ` Jeff King 2009-11-23 15:50 ` Petr Baudis 2009-11-23 16:43 ` Jeff King 2009-11-23 17:05 ` Petr Baudis 2009-11-23 19:28 ` Jeff King 2009-11-23 17:43 ` [PATCH] pack-objects: split implications of --all-progress from progress activation Nicolas Pitre 2009-11-23 18:12 ` Petr Baudis 2009-11-23 18:27 ` Nicolas Pitre 2009-11-23 19:04 ` Petr Baudis 2009-11-23 19:32 ` Jeff King 2009-11-23 16:56 ` how to suppress progress percentage in git-push Nicolas Pitre 2009-11-23 19:25 ` Jeff King 2009-11-23 19:40 ` Nicolas Pitre 2009-11-24 1:13 ` bill lam 2009-11-24 3:07 ` Jeff King
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.