{"thread":{"id":"21702","subject":"how to suppress progress percentage in git-push","startedAt":"2009-11-22T14:53:53Z","lastAt":"2009-11-24T03:07:42Z","messageCount":16,"participants":["bill lam","Jeff King","Petr Baudis","Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"128103","messageId":"20091122145352.GA3941@debian.b2j","threadId":"21702","inReplyTo":null,"subject":"how to suppress progress percentage in git-push","fromName":"bill lam","fromEmail":"cbill.lam@gmail.com","sentAt":"2009-11-22T14:53:53Z","receivedAt":"2009-11-22T14:53:53Z","isPatch":false,"sender":{"key":"cbill.lam@gmail.com","avatar":null},"body":"I set crontab to push to another computer for backup. It sent\nconfirmation email after finished.  It looked like\n\nCounting objects: 1\nCounting objects: 9, done.\nDelta compression using up to 2 threads.\nCompressing objects:  20% (1/5)\nCompressing objects:  40% (2/5)\nCompressing objects:  60% (3/5)\nCompressing objects:  80% (4/5)\nCompressing objects: 100% (5/5)\nCompressing objects: 100% (5/5), done.\nWriting objects:  20% (1/5)\nWriting objects:  40% (2/5)\nWriting objects:  60% (3/5)\nWriting objects:  80% (4/5)\nWriting objects: 100% (5/5)\nWriting objects: 100% (5/5), 549 bytes, done.\nTotal 5 (delta 3), reused 0 (delta 0)\n\nOften the list of progress % can be a page long.  I want output but\nnot those percentage progress status.  Will that be possible?\n\n-- \nregards,\n====================================================\nGPG key 1024D/4434BAB3 2008-08-24\ngpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3\n"},{"id":"128173","messageId":"20091123145959.GA13138@sigill.intra.peff.net","threadId":"21702","inReplyTo":"20091122145352.GA3941@debian.b2j","subject":"Re: how to suppress progress percentage in git-push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-23T15:00:00Z","receivedAt":"2009-11-23T15:00:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote:\n\n> I set crontab to push to another computer for backup. It sent\n> confirmation email after finished.  It looked like\n> \n> Counting objects: 1\n> Counting objects: 9, done.\n> Delta compression using up to 2 threads.\n> Compressing objects:  20% (1/5)\n> Compressing objects:  40% (2/5)\n> Compressing objects:  60% (3/5)\n> Compressing objects:  80% (4/5)\n> Compressing objects: 100% (5/5)\n> Compressing objects: 100% (5/5), done.\n> Writing objects:  20% (1/5)\n> Writing objects:  40% (2/5)\n> Writing objects:  60% (3/5)\n> Writing objects:  80% (4/5)\n> Writing objects: 100% (5/5)\n> Writing objects: 100% (5/5), 549 bytes, done.\n> Total 5 (delta 3), reused 0 (delta 0)\n> \n> Often the list of progress % can be a page long.  I want output but\n> not those percentage progress status.  Will that be possible?\n\nHmm. There seems to be a bug. pack-objects is supposed to see that\nstderr is not a tty and suppress the progress messages. But it doesn't,\nbecause send-pack gives it the --all-progress flag, which\nunconditionally tells it to display progress, when the desired impact is\nactually to just make the progress more verbose.\n\nWe need to do one of:\n\n  1. make --all-progress imply \"if we are using progress, then make it\n     more verbose. Otherwise, ignore.\"\n\n  2. fix all callers to check isatty(2) before unconditionally passing\n     the option\n\nThe patch for (1) would look something like what's below.  It's simpler,\nbut it does change the semantics; anyone who was relying on\n--all-progress to turn on progress unconditionally would need to now\nalso use --progress. However, turning on progress unconditionally is\nusually an error (the except is if you are piping output in real-time to\nthe user and need to overcome the isatty check).\n\nNicolas, this is your code. Which do you prefer?\n\n-Peff\n\n---\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 4c91e94..50dd429 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -75,6 +75,7 @@ static int ignore_packed_keep;\n static int allow_ofs_delta;\n static const char *base_name;\n static int progress = 1;\n+static int all_progress;\n static int window = 10;\n static uint32_t pack_size_limit, pack_size_limit_cfg;\n static int depth = 50;\n@@ -2220,7 +2221,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--all-progress\", arg)) {\n-\t\t\tprogress = 2;\n+\t\t\tall_progress = 1;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"-q\", arg)) {\n@@ -2295,6 +2296,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tusage(pack_usage);\n \t}\n \n+\tif (progress && all_progress)\n+\t\tprogress = 2;\n+\n \t/* Traditionally \"pack-objects [options] base extra\" failed;\n \t * we would however want to take refs parameter that would\n \t * have been given to upstream rev-list ourselves, which means\n"},{"id":"128174","messageId":"20091123155043.GA28963@machine.or.cz","threadId":"21702","inReplyTo":"20091123145959.GA13138@sigill.intra.peff.net","subject":"Re: how to suppress progress percentage in git-push","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-23T15:50:43Z","receivedAt":"2009-11-23T15:50:43Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:\n> The patch for (1) would look something like what's below.  It's simpler,\n> but it does change the semantics; anyone who was relying on\n> --all-progress to turn on progress unconditionally would need to now\n> also use --progress. However, turning on progress unconditionally is\n> usually an error (the except is if you are piping output in real-time to\n> the user and need to overcome the isatty check).\n\nI'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I\nwould be unhappy if I would have to go through creating ptys or whatever\nnow. Maybe conditioning this by an environment variable?\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"128177","messageId":"20091123164319.GA23011@sigill.intra.peff.net","threadId":"21702","inReplyTo":"20091123155043.GA28963@machine.or.cz","subject":"Re: how to suppress progress percentage in git-push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-23T16:43:19Z","receivedAt":"2009-11-23T16:43:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote:\n\n> On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:\n> > The patch for (1) would look something like what's below.  It's simpler,\n> > but it does change the semantics; anyone who was relying on\n> > --all-progress to turn on progress unconditionally would need to now\n> > also use --progress. However, turning on progress unconditionally is\n> > usually an error (the except is if you are piping output in real-time to\n> > the user and need to overcome the isatty check).\n> \n> I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I\n> would be unhappy if I would have to go through creating ptys or whatever\n> now. Maybe conditioning this by an environment variable?\n\nYou wouldn't need to do anything that drastic. You would just need to\npass \"--progress --all-progress\" instead of only --all-progress. But you\nhave provided the data point that such a change would break at least one\nuser.\n\nWe could also leave --all-progress as-is and add new option to mean \"if\nyou are already doing progress, do all progress\".\n\n-Peff\n"},{"id":"128178","messageId":"alpine.LFD.2.00.0911231043310.2059@xanadu.home","threadId":"21702","inReplyTo":"20091123145959.GA13138@sigill.intra.peff.net","subject":"Re: how to suppress progress percentage in git-push","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-23T16:56:43Z","receivedAt":"2009-11-23T16:56:43Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 23 Nov 2009, Jeff King wrote:\n\n> On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote:\n> \n> > I set crontab to push to another computer for backup. It sent\n> > confirmation email after finished.  It looked like\n> > \n> > Counting objects: 1\n> > Counting objects: 9, done.\n> > Delta compression using up to 2 threads.\n> > Compressing objects:  20% (1/5)\n> > Compressing objects:  40% (2/5)\n> > Compressing objects:  60% (3/5)\n> > Compressing objects:  80% (4/5)\n> > Compressing objects: 100% (5/5)\n> > Compressing objects: 100% (5/5), done.\n> > Writing objects:  20% (1/5)\n> > Writing objects:  40% (2/5)\n> > Writing objects:  60% (3/5)\n> > Writing objects:  80% (4/5)\n> > Writing objects: 100% (5/5)\n> > Writing objects: 100% (5/5), 549 bytes, done.\n> > Total 5 (delta 3), reused 0 (delta 0)\n> > \n> > Often the list of progress % can be a page long.  I want output but\n> > not those percentage progress status.  Will that be possible?\n> \n> Hmm. There seems to be a bug. pack-objects is supposed to see that\n> stderr is not a tty and suppress the progress messages. But it doesn't,\n> because send-pack gives it the --all-progress flag, which\n> unconditionally tells it to display progress, when the desired impact is\n> actually to just make the progress more verbose.\n\nNot exactly.\n\nFirst, the progress variable is initialized with the result of isatty(2) \nby default.  The --progress argument is there to override that since \npack-objects is often executed on the sending end of a fetch operation \nwhere stderr is not a terminal.\n\nThen, during the pack-objects process, there are 3 phases: counting \nobjects, compressing objects, and writing objects.  However in the fetch \ncase we prefer to let the receiving end (index-pack or unpack-objects) \ntake care of the progress display during the third phase.  This is why \nby default pack-objects doesn't display any \"writing objects\" progress \nwhen the generated pack is sent to stdout.  The --all-progress argument \nis there to override that, namely for a push.  The fact that \n--all-progress implies --progress is a bad side effect which wouldn't \nneed to exist, but I don't think this is the cause of the issue here.\n\n> We need to do one of:\n> \n>   1. make --all-progress imply \"if we are using progress, then make it\n>      more verbose. Otherwise, ignore.\"\n> \n>   2. fix all callers to check isatty(2) before unconditionally passing\n>      the option\n\nNone of the above would fix the issue as this only affects progress \ndisplay for phase #3.  You'd still get progress display for the counting \nphase and the compressing phase.\n\nThat doesn't mean it is OK for send-pack to unconditionally use \n--all-progress though, although it does provide the -q argument to \npack-objects when push -q is used which inhibits any progress display \nalready.\n\n\nNicolas\n"},{"id":"128179","messageId":"20091123170547.GC26996@machine.or.cz","threadId":"21702","inReplyTo":"20091123164319.GA23011@sigill.intra.peff.net","subject":"Re: how to suppress progress percentage in git-push","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-23T17:05:47Z","receivedAt":"2009-11-23T17:05:47Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Nov 23, 2009 at 11:43:19AM -0500, Jeff King wrote:\n> On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote:\n> \n> > On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:\n> > > The patch for (1) would look something like what's below.  It's simpler,\n> > > but it does change the semantics; anyone who was relying on\n> > > --all-progress to turn on progress unconditionally would need to now\n> > > also use --progress. However, turning on progress unconditionally is\n> > > usually an error (the except is if you are piping output in real-time to\n> > > the user and need to overcome the isatty check).\n> > \n> > I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I\n> > would be unhappy if I would have to go through creating ptys or whatever\n> > now. Maybe conditioning this by an environment variable?\n> \n> You wouldn't need to do anything that drastic. You would just need to\n> pass \"--progress --all-progress\" instead of only --all-progress. But you\n> have provided the data point that such a change would break at least one\n> user.\n> \n> We could also leave --all-progress as-is and add new option to mean \"if\n> you are already doing progress, do all progress\".\n\nHmm, maybe I'm confused - I just call\n\n\tgit remote update\n\nand don't pass any progress switches - would your change still affect\nme? Can I pass --progress to `git remote update`?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nA lot of people have my books on their bookshelves.\nThat's the problem, they need to read them. -- Don Knuth\n"},{"id":"128183","messageId":"alpine.LFD.2.00.0911231221320.2059@xanadu.home","threadId":"21702","inReplyTo":"20091123164319.GA23011@sigill.intra.peff.net","subject":"[PATCH] pack-objects: split implications of --all-progress from progress activation","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-23T17:43:50Z","receivedAt":"2009-11-23T17:43:50Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Currently the --all-progress flag is used to use force progress display \nduring the writing object phase even if output goes to stdout which is \nprimarily the case during a push operation.  This has the unfortunate \nside effect of forcing progress display even if stderr is not a \nterminal.\n\nLet's introduce the --all-progress-implied argument which has the same \nintent except for actually forcing the activation of any progress \ndisplay.  With this, progress display will be automatically inhibited \nwhenever stderr is not a terminal, or full progress display will be \nincluded otherwise.  This should let people use 'git push' within a cron \njob without filling their logs with useless percentage displays.\n\nSigned-off-by: Nicolas Pitre <nico@fluxnic.net>\n---\n\nOn Mon, 23 Nov 2009, Jeff King wrote:\n\n> We could also leave --all-progress as-is and add new option to mean \"if\n> you are already doing progress, do all progress\".\n\nActually, that's what I was working on when I saw the above.\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 2e49929..f54d433 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -9,8 +9,9 @@ git-pack-objects - Create a packed archive of objects\n SYNOPSIS\n --------\n [verse]\n-'git pack-objects' [-q] [--no-reuse-delta] [--delta-base-offset] [--non-empty]\n-\t[--local] [--incremental] [--window=N] [--depth=N] [--all-progress]\n+'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied]\n+\t[--no-reuse-delta] [--delta-base-offset] [--non-empty]\n+\t[--local] [--incremental] [--window=N] [--depth=N]\n \t[--revs [--unpacked | --all]*] [--stdout | base-name]\n \t[--keep-true-parents] < object-list\n \n@@ -137,7 +138,7 @@ base-name::\n \n --all-progress::\n \tWhen --stdout is specified then progress report is\n-\tdisplayed during the object count and deltification phases\n+\tdisplayed during the object count and compression phases\n \tbut inhibited during the write-out phase. The reason is\n \tthat in some cases the output stream is directly linked\n \tto another command which may wish to display progress\n@@ -146,6 +147,11 @@ base-name::\n \treport for the write-out phase as well even if --stdout is\n \tused.\n \n+--all-progress-implied::\n+\tThis is used to imply --all-progress whenever progress display\n+\tis activated.  Unlike --all-progress this flag doesn't actually\n+\tforce any progress display by itself.\n+\n -q::\n \tThis flag makes the command not to report its progress\n \ton the standard error stream.\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 4c91e94..4429d53 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -24,6 +24,7 @@\n \n static const char pack_usage[] =\n   \"git pack-objects [{ -q | --progress | --all-progress }]\\n\"\n+  \"        [--all-progress-implied]\\n\"\n   \"        [--max-pack-size=N] [--local] [--incremental]\\n\"\n   \"        [--window=N] [--window-memory=N] [--depth=N]\\n\"\n   \"        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\\n\"\n@@ -2124,6 +2125,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n {\n \tint use_internal_rev_list = 0;\n \tint thin = 0;\n+\tint all_progress_implied = 0;\n \tuint32_t i;\n \tconst char **rp_av;\n \tint rp_ac_alloc = 64;\n@@ -2223,6 +2225,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tprogress = 2;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--all-progress-implied\", arg)) {\n+\t\t\tall_progress_implied = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(\"-q\", arg)) {\n \t\t\tprogress = 0;\n \t\t\tcontinue;\n@@ -2326,6 +2332,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (keep_unreachable && unpack_unreachable)\n \t\tdie(\"--keep-unreachable and --unpack-unreachable are incompatible.\");\n \n+\tif (progress && all_progress_implied)\n+\t\tprogress = 2;\n+\n \tprepare_packed_git();\n \n \tif (progress)\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex d26997b..8fffdbf 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -40,7 +40,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext\n \t */\n \tconst char *argv[] = {\n \t\t\"pack-objects\",\n-\t\t\"--all-progress\",\n+\t\t\"--all-progress-implied\",\n \t\t\"--revs\",\n \t\t\"--stdout\",\n \t\tNULL,\ndiff --git a/bundle.c b/bundle.c\nindex e04ab49..ff97adc 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -343,7 +343,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \t/* write pack */\n \targv_pack[0] = \"pack-objects\";\n-\targv_pack[1] = \"--all-progress\";\n+\targv_pack[1] = \"--all-progress-implied\";\n \targv_pack[2] = \"--stdout\";\n \targv_pack[3] = \"--thin\";\n \targv_pack[4] = NULL;\n"},{"id":"128187","messageId":"20091123181206.GD26996@machine.or.cz","threadId":"21702","inReplyTo":"alpine.LFD.2.00.0911231221320.2059@xanadu.home","subject":"Re: [PATCH] pack-objects: split implications of --all-progress from progress activation","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-23T18:12:06Z","receivedAt":"2009-11-23T18:12:06Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote:\n> Currently the --all-progress flag is used to use force progress display \n> during the writing object phase even if output goes to stdout which is \n> primarily the case during a push operation.  This has the unfortunate \n> side effect of forcing progress display even if stderr is not a \n> terminal.\n> \n> Let's introduce the --all-progress-implied argument which has the same \n> intent except for actually forcing the activation of any progress \n> display.  With this, progress display will be automatically inhibited \n> whenever stderr is not a terminal, or full progress display will be \n> included otherwise.  This should let people use 'git push' within a cron \n> job without filling their logs with useless percentage displays.\n> \n> Signed-off-by: Nicolas Pitre <nico@fluxnic.net>\n\nOk, but what is currently the way to force the old behaviour? I believe\nthat should be also part of the commit message.\n\nNaive deduction fails:\n\n\t$ git remote update --progress\n\terror: unknown option `progress'\n\nThanks,\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nA lot of people have my books on their bookshelves.\nThat's the problem, they need to read them. -- Don Knuth\n"},{"id":"128192","messageId":"alpine.LFD.2.00.0911231323010.2059@xanadu.home","threadId":"21702","inReplyTo":"20091123181206.GD26996@machine.or.cz","subject":"Re: [PATCH] pack-objects: split implications of --all-progress from progress activation","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-23T18:27:14Z","receivedAt":"2009-11-23T18:27:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 23 Nov 2009, Petr Baudis wrote:\n\n> On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote:\n> > Currently the --all-progress flag is used to use force progress display \n> > during the writing object phase even if output goes to stdout which is \n> > primarily the case during a push operation.  This has the unfortunate \n> > side effect of forcing progress display even if stderr is not a \n> > terminal.\n> > \n> > Let's introduce the --all-progress-implied argument which has the same \n> > intent except for actually forcing the activation of any progress \n> > display.  With this, progress display will be automatically inhibited \n> > whenever stderr is not a terminal, or full progress display will be \n> > included otherwise.  This should let people use 'git push' within a cron \n> > job without filling their logs with useless percentage displays.\n> > \n> > Signed-off-by: Nicolas Pitre <nico@fluxnic.net>\n> \n> Ok, but what is currently the way to force the old behaviour?\n\nIf any existing out-of-tree users of pack-objects were using \n--all-progress then nothing has changed for them.\n\n> I believe that should be also part of the commit message.\n> \n> Naive deduction fails:\n> \n> \t$ git remote update --progress\n> \terror: unknown option `progress'\n\nUsage of 'git remote\" is about fetching and not pushing, right? My patch \nonly affects pushes.  So I don't know what old behavior you're after.\n\n\nNicolas\n"},{"id":"128194","messageId":"20091123190402.GE26996@machine.or.cz","threadId":"21702","inReplyTo":"alpine.LFD.2.00.0911231323010.2059@xanadu.home","subject":"Re: [PATCH] pack-objects: split implications of --all-progress from progress activation","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-23T19:04:02Z","receivedAt":"2009-11-23T19:04:02Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Nov 23, 2009 at 01:27:14PM -0500, Nicolas Pitre wrote:\n> On Mon, 23 Nov 2009, Petr Baudis wrote:\n> > Naive deduction fails:\n> > \n> > \t$ git remote update --progress\n> > \terror: unknown option `progress'\n> \n> Usage of 'git remote\" is about fetching and not pushing, right? My patch \n> only affects pushes.  So I don't know what old behavior you're after.\n\nOh, I'm sorry, in that case I was confused from the very beginning;\nsomehow I saw pull instead of push in the initial mail.\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"128195","messageId":"20091123192518.GA1607@coredump.intra.peff.net","threadId":"21702","inReplyTo":"alpine.LFD.2.00.0911231043310.2059@xanadu.home","subject":"Re: how to suppress progress percentage in git-push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-23T19:25:18Z","receivedAt":"2009-11-23T19:25:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote:\n\n> > We need to do one of:\n> > \n> >   1. make --all-progress imply \"if we are using progress, then make it\n> >      more verbose. Otherwise, ignore.\"\n> > \n> >   2. fix all callers to check isatty(2) before unconditionally passing\n> >      the option\n> \n> None of the above would fix the issue as this only affects progress \n> display for phase #3.  You'd still get progress display for the counting \n> phase and the compressing phase.\n\nI think it does fix the issue, as it is exactly the side effect that we\nare concerned about (and I tested my patch for 1, which squelched it).\nBut your --all-progress-implied is a better fix, so I think we should go\nwith that.\n\n> That doesn't mean it is OK for send-pack to unconditionally use \n> --all-progress though, although it does provide the -q argument to \n> pack-objects when push -q is used which inhibits any progress display \n> already.\n\nIt does, and I almost suggested that (although -q is new as of 1.6.5, so\nit may not help the original poster). But \"-q\" actually does more than\nthat; it also silences any output for a successful push, which may not\nbe desired.\n\n-Peff\n"},{"id":"128196","messageId":"20091123192848.GB1607@coredump.intra.peff.net","threadId":"21702","inReplyTo":"20091123170547.GC26996@machine.or.cz","subject":"Re: how to suppress progress percentage in git-push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-23T19:28:48Z","receivedAt":"2009-11-23T19:28:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 23, 2009 at 06:05:47PM +0100, Petr Baudis wrote:\n\n> > You wouldn't need to do anything that drastic. You would just need to\n> > pass \"--progress --all-progress\" instead of only --all-progress. But you\n> > have provided the data point that such a change would break at least one\n> > user.\n> > \n> > We could also leave --all-progress as-is and add new option to mean \"if\n> > you are already doing progress, do all progress\".\n> \n> Hmm, maybe I'm confused - I just call\n> \n> \tgit remote update\n> \n> and don't pass any progress switches - would your change still affect\n> me? Can I pass --progress to `git remote update`?\n\nOh, I misunderstood; I thought you were calling pack-objects directly.\nSo you are actually relying on the \"even though isatty(2) is not true,\nwe always print progress messages\" behavior? I think that behavior is\nbuggy. It hurts everybody pushing via cron, and it violates the usual\nrule for when we show progress messages.\n\nI don't think you can get a --progress pushed all the way down to the\npack-objects in this case; we would need to add code to override the\nisatty check.\n\nThat being said, your example of \"remote update\" means you are dealing\nwith fetch, and we are not touching the fetch code path at all.\n\n-Peff\n"},{"id":"128197","messageId":"20091123193242.GA3140@coredump.intra.peff.net","threadId":"21702","inReplyTo":"alpine.LFD.2.00.0911231221320.2059@xanadu.home","subject":"Re: [PATCH] pack-objects: split implications of --all-progress from progress activation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-23T19:32:42Z","receivedAt":"2009-11-23T19:32:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote:\n\n> Currently the --all-progress flag is used to use force progress display \n> during the writing object phase even if output goes to stdout which is \n> primarily the case during a push operation.  This has the unfortunate \n> side effect of forcing progress display even if stderr is not a \n> terminal.\n> \n> Let's introduce the --all-progress-implied argument which has the same \n> intent except for actually forcing the activation of any progress \n> display.  With this, progress display will be automatically inhibited \n> whenever stderr is not a terminal, or full progress display will be \n> included otherwise.  This should let people use 'git push' within a cron \n> job without filling their logs with useless percentage displays.\n> \n> Signed-off-by: Nicolas Pitre <nico@fluxnic.net>\n\nThanks,\n\nTested-by: Jeff King <peff@peff.net>\n\n-Peff\n"},{"id":"128198","messageId":"alpine.LFD.2.00.0911231437460.2059@xanadu.home","threadId":"21702","inReplyTo":"20091123192518.GA1607@coredump.intra.peff.net","subject":"Re: how to suppress progress percentage in git-push","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-23T19:40:31Z","receivedAt":"2009-11-23T19:40:31Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 23 Nov 2009, Jeff King wrote:\n\n> On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote:\n> \n> > > We need to do one of:\n> > > \n> > >   1. make --all-progress imply \"if we are using progress, then make it\n> > >      more verbose. Otherwise, ignore.\"\n> > > \n> > >   2. fix all callers to check isatty(2) before unconditionally passing\n> > >      the option\n> > \n> > None of the above would fix the issue as this only affects progress \n> > display for phase #3.  You'd still get progress display for the counting \n> > phase and the compressing phase.\n> \n> I think it does fix the issue, as it is exactly the side effect that we\n> are concerned about (and I tested my patch for 1, which squelched it).\n\nSorry, you were right.  I somehow understood something else initially.\n\n> But your --all-progress-implied is a better fix, so I think we should go\n> with that.\n\nOK.\n\n\nNicolas\n"},{"id":"128220","messageId":"20091124011339.GA18003@debian.b2j","threadId":"21702","inReplyTo":"alpine.LFD.2.00.0911231043310.2059@xanadu.home","subject":"Re: how to suppress progress percentage in git-push","fromName":"bill lam","fromEmail":"cbill.lam@gmail.com","sentAt":"2009-11-24T01:13:39Z","receivedAt":"2009-11-24T01:13:39Z","isPatch":false,"sender":{"key":"cbill.lam@gmail.com","avatar":null},"body":"On Mon, 23 Nov 2009, Nicolas Pitre wrote:\n> Then, during the pack-objects process, there are 3 phases: counting \n> objects, compressing objects, and writing objects.  However in the fetch \n\nduring git-gc it shows yet another progress \n\nRemoving duplicate objects: 100% (256/256), done.\n\n-- \nregards,\n====================================================\nGPG key 1024D/4434BAB3 2008-08-24\ngpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3\n"},{"id":"128226","messageId":"20091124030742.GA32029@coredump.intra.peff.net","threadId":"21702","inReplyTo":"20091124011339.GA18003@debian.b2j","subject":"Re: how to suppress progress percentage in git-push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-24T03:07:42Z","receivedAt":"2009-11-24T03:07:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 24, 2009 at 09:13:39AM +0800, bill lam wrote:\n\n> On Mon, 23 Nov 2009, Nicolas Pitre wrote:\n> > Then, during the pack-objects process, there are 3 phases: counting \n> > objects, compressing objects, and writing objects.  However in the fetch \n> \n> during git-gc it shows yet another progress \n> \n> Removing duplicate objects: 100% (256/256), done.\n\nThanks, this doesn't seem to have been guarded at all (but since it is\non a 2-second delay, you have to have quite a lot of loose objects or a\nslow disk to trigger it).\n\nWe should apply the patch below to keep things consistent.\n\nI also checked every other call to start_progress; everything else seems\nto be guarded. Most of them were easy to trace to an isatty check,\nthough the one in unpack-trees is influenced by o->verbose_update. That\nin turn usually corresponds to a quiet option, though merge does seem to\nuse it unconditionally. Maybe that should be tweaked, too?\n\n-- >8 --\nSubject: [PATCH] prune-packed: only show progress when stderr is a tty\n\nThis matches the behavior of other git programs, and helps\nkeep cruft out of things like cron job output.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin-prune-packed.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-prune-packed.c b/builtin-prune-packed.c\nindex be99eb0..f9463de 100644\n--- a/builtin-prune-packed.c\n+++ b/builtin-prune-packed.c\n@@ -71,7 +71,7 @@ void prune_packed_objects(int opts)\n \n int cmd_prune_packed(int argc, const char **argv, const char *prefix)\n {\n-\tint opts = VERBOSE;\n+\tint opts = isatty(2) ? VERBOSE : 0;\n \tconst struct option prune_packed_options[] = {\n \t\tOPT_BIT('n', \"dry-run\", &opts, \"dry run\", DRY_RUN),\n \t\tOPT_NEGBIT('q', \"quiet\", &opts, \"be quiet\", VERBOSE),\n-- \n1.6.6.rc0.249.g9b4cf.dirty\n"}]}