{"thread":{"id":"21674","subject":"[PATCH] git-update-index: report(...) now flushes stdout after printing the report line","startedAt":"2009-11-19T21:17:41Z","lastAt":"2010-01-06T01:51:06Z","messageCount":9,"participants":["Sebastian Thiel","Nanako Shiraishi","Junio C Hamano","Tay Ray Chuan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127928","messageId":"loom.20091119T221732-624@post.gmane.org","threadId":"21674","inReplyTo":null,"subject":"[PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Sebastian Thiel","fromEmail":"byronimo@gmail.com","sentAt":"2009-11-19T21:17:41Z","receivedAt":"2009-11-19T21:17:41Z","isPatch":true,"sender":{"key":"byronimo@gmail.com","avatar":"https://gravatar.com/avatar/84cc94f1c3a848e9e0e11bb1545f5db3245280ffd354687ea8116546509c8429?d=mp&s=160"},"body":"This makes it equivalent to the behavior of git-hash-object and allows tools to\nwrite one path\nto stdin, flush and assure the work is done once it reads the corresponding\nreport line.\nPreviously attempting to do that would result in the program blocking as\ngit-update-index\ndid not flush its report line (yet). External programs use the git-hash-object\nlike behavior\nto precisely control when which work is done while providing just-in-time\nfeedback to the end-user.\n---\n builtin-update-index.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 92beaaf..08bf933 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -37,6 +37,7 @@ static void report(const char *fmt, ...)\n \tva_start(vp, fmt);\n \tvprintf(fmt, vp);\n \tputchar('\\n');\n+\tmaybe_flush_or_die(stdout, \"line to stdout\");\n \tva_end(vp);\n }\n \n-- \n1.6.5.3.172.g9e796\n"},{"id":"130551","messageId":"20091230224122.6117@nanako3.lavabit.com","threadId":"21674","inReplyTo":"loom.20091119T221732-624@post.gmane.org","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-30T13:41:22Z","receivedAt":"2009-12-30T13:41:22Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Junio, could you tell us what happened to this thread?\n"},{"id":"130556","messageId":"loom.20091230T145525-818@post.gmane.org","threadId":"21674","inReplyTo":"loom.20091119T221732-624@post.gmane.org","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Sebastian Thiel","fromEmail":"byronimo@gmail.com","sentAt":"2009-12-30T13:56:32Z","receivedAt":"2009-12-30T13:56:32Z","isPatch":true,"sender":{"key":"byronimo@gmail.com","avatar":"https://gravatar.com/avatar/84cc94f1c3a848e9e0e11bb1545f5db3245280ffd354687ea8116546509c8429?d=mp&s=160"},"body":"I'd like to add that since version 1.6.5, non-tty's do not receive any progress\ninformation anymore. The patch causing this says it wants to, in short words,\nunify the push and fetch handling regarding the way progress messages are sent.\n\nNow third-party wrappers, such as git-python, are unable to provide any progress\ninformation anymore for possibly lengthy operations.\n\nThis is why I clearly recommend to add some kind of a \"progress-force\" flag that\nturns progress messages on again for send-pack and receive-pack.\n"},{"id":"130575","messageId":"7v1vic1bzh.fsf@alter.siamese.dyndns.org","threadId":"21674","inReplyTo":"20091230224122.6117@nanako3.lavabit.com","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-30T19:46:26Z","receivedAt":"2009-12-30T19:46:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Junio, could you tell us what happened to this thread?\n\nI didn't feel I had enough energy to read the commit log message after\nseeing it was badly linewrapped and didn't have a sign-off, so I didn't\nread it.\n\nI've read it now; it is unclear from the proposed commit log message how\nthis fits in the larger picture. Presumably this change is meant to be\nuseful when driving update-index through --stdin? To see if I got the\nintention right, let me try paraphrasing it...\n\n update-index: flush standard output after each action is reported\n\n A scripted Porcelain that runs \"git update-index --stdin\" might want\n to use a bidirectional pipe, while feeding one path at a time and\n reading the output from report() every time after feeding a path.\n\n Such a Porcelain would deadlock if the standard output is not flushed\n after report().\n\nI don't know if the above is what Sebastian meant, though..\n\nAn obvious question, when phrased this way, is \"what impact does this\nchange have for scripted Porcelains that don't use bi-di pipe?\" I think\nthe answer would be \"The I/O overhead for flushing would increase\", but I\ndon't know if it would be \"... would increase but it is still negligible\"\nor \"... would increase too much to make it noticeably or unusably slow\nespecially if it feeds hundreds of paths\". If it is the latter, this may\nneed to be controlled by another command line option.\n\nSebastian, care to redo the justification, make it a bit more readable,\nand add your sign-off?\n\nThanks.\n"},{"id":"130728","messageId":"loom.20100103T114055-16@post.gmane.org","threadId":"21674","inReplyTo":"loom.20091119T221732-624@post.gmane.org","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Sebastian Thiel","fromEmail":"byronimo@gmail.com","sentAt":"2010-01-03T10:41:31Z","receivedAt":"2010-01-03T10:41:31Z","isPatch":true,"sender":{"key":"byronimo@gmail.com","avatar":"https://gravatar.com/avatar/84cc94f1c3a848e9e0e11bb1545f5db3245280ffd354687ea8116546509c8429?d=mp&s=160"},"body":"Sorry for the badly formatted message, and thanks a lot for the correction which\nis what my post should have been in the first place.\n\nRedoing the commit is not what will be needed for git-python to work properly\nwhich is why I will tell the whole story before submitting any(more) patches.\n\nWith git v1.6.5, git-push was adjusted not to provide progress messages anymore\nif the device attached to stderr is not a tty. Previously, this was only the\ncase with git-fetch. For git-python, and other callers of the commandline such\nas tortoise-git, there currently is no way to provide progress information to\nthe user unless they (somehow) simulate a tty which appears unfeasible. When\nusing these tools, time consuming operations tend to appear as if they are\nhanging. One might argue that most code is fetched and pushed in a matter of\nseconds, but if git is used to store large binary data, processing and\ntransferring it will take time.\n\nThe issue mentioned with git-update-index and it's missing flush that would\ncause a deadlock in some porcelain can be fixed trivially, but seen in the\ncontext of the git-push and git-pull a more thorough solution might be more\nappealing. As mentioned by Junio, a default flush after each report might slow\ndown some existing porcelain, and a commandline option would be part of the\nproper solution. I would argue though that a separate option would add quite\nsome complexity to the command as it is a very specialized one. Instead I would\nrecommend checking whether --stdin is given on the commandline, and flush stdout\nif that is true. This would natively make the command behave like\ngit-hash-object and git-cat-file. If --stdin is not provided, report is not\nrequired to flush after every call as the commandline options are processed\nwithout additional user interaction.\n\nAdding a commandline option to git-push and git-pull that enforces progress\nmessages to be printed to stderr would be a feasible and simple fix that would\nclearly improve the usability of tortoise-git and git-python to name only two.\n\nThat said, I hope I managed to make myself clear enough this time to help the\npeople in charge to figure out how to solve the issue. Once the desired solution\nhas been sketched out and the desired new commandline options have been named,\nit could even be me to implement it if necessary, as I'd consider it a gentle\nstart into the world of the git codebase.\n\nThanks for picking this up again,\nSebastian\n"},{"id":"130737","messageId":"be6fef0d1001031503h11cb65c6ha34eee345b9b7055@mail.gmail.com","threadId":"21674","inReplyTo":"loom.20100103T114055-16@post.gmane.org","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-03T23:03:19Z","receivedAt":"2010-01-03T23:03:19Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\n(you dropped the Cc list; fixed that for you.)\n\nOn Sun, Jan 3, 2010 at 6:41 PM, Sebastian Thiel wrote:\n> Adding a commandline option to git-push and git-pull that enforces progress\n> messages to be printed to stderr would be a feasible and simple fix that would\n> clearly improve the usability of tortoise-git and git-python to name only two.\n>\n> That said, I hope I managed to make myself clear enough this time to help the\n> people in charge to figure out how to solve the issue. Once the desired solution\n> has been sketched out and the desired new commandline options have been named,\n> it could even be me to implement it if necessary, as I'd consider it a gentle\n> start into the world of the git codebase.\n\nfrom your above message solely and setting aside your original patch,\nI presume that you want to introduce the ability to force progress\nreporting even if stderr isn't a terminal.\n\nI am working a feature (display progress for http operations) that\nhappens to add this ability to git-push and git-fetch, by specifying\nthe --progress option.\n\nRegarding git-pull - I guess it's only git-fetch (being\ntransport-related) that reports progress?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"130759","messageId":"ba5802b21001040230ud27c06bw3531724862cb12c4@mail.gmail.com","threadId":"21674","inReplyTo":"be6fef0d1001031503h11cb65c6ha34eee345b9b7055@mail.gmail.com","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Sebastian Thiel","fromEmail":"byronimo@googlemail.com","sentAt":"2010-01-04T10:30:16Z","receivedAt":"2010-01-04T10:30:16Z","isPatch":true,"sender":{"key":"byronimo@googlemail.com","avatar":null},"body":"Thanks Ray Chuan, for the clarification, the progress is supposed to\nbe sent in git-push and git-fetch ( not git-pull as I mentioned ).\nA --progress flag would already do it for me, is there a way to fetch\nyour code from somewhere for a test run ?\n\nWhen do you think will your changes be available for a mainline merge,\nor would it even be possible to separate the --progress adjustment\nfrom your feature to merge it into mainline individually ?\n\nThanks,\nSebastian\n"},{"id":"130878","messageId":"7vwrzwoxh4.fsf@alter.siamese.dyndns.org","threadId":"21674","inReplyTo":"be6fef0d1001031503h11cb65c6ha34eee345b9b7055@mail.gmail.com","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-06T01:04:07Z","receivedAt":"2010-01-06T01:04:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> from your above message solely and setting aside your original patch,\n> I presume that you want to introduce the ability to force progress\n> reporting even if stderr isn't a terminal.\n>\n> I am working a feature (display progress for http operations) that\n> happens to add this ability to git-push and git-fetch, by specifying\n> the --progress option.\n>\n> Regarding git-pull - I guess it's only git-fetch (being\n> transport-related) that reports progress?\n\nAre you talking about this topic?\n\n * tc/clone-v-progress (2009-12-26) 4 commits\n - clone: use --progress to force progress reporting\n - clone: set transport->verbose when -v/--verbose is used\n - git-clone.txt: reword description of progress behaviour\n - check stderr with isatty() instead of stdout when deciding to show progress\n\nWhat do people think about it? I vaguely recall that somebody asked to\nadd a warning to release notes on the behaviour change to this series, and\nI think it may be a worthwhile thing to do (e.g. \"Earlier we did X but now\nwe do Y; change things in this way if you want us to keep doing X\"), but\notherwise I think it is a sensible change.\n"},{"id":"130888","messageId":"be6fef0d1001051751p33c2f4a6r5b6f3fbbbeac9fa9@mail.gmail.com","threadId":"21674","inReplyTo":"7vwrzwoxh4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-update-index: report(...) now flushes stdout after printing the report line","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-06T01:51:06Z","receivedAt":"2010-01-06T01:51:06Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Jan 6, 2010 at 9:04 AM, Junio C Hamano wrote:\n> Tay Ray Chuan writes:\n>\n>> from your above message solely and setting aside your original patch,\n>> I presume that you want to introduce the ability to force progress\n>> reporting even if stderr isn't a terminal.\n>>\n>> I am working a feature (display progress for http operations) that\n>> happens to add this ability to git-push and git-fetch, by specifying\n>> the --progress option.\n>>\n>> Regarding git-pull - I guess it's only git-fetch (being\n>> transport-related) that reports progress?\n>\n> Are you talking about this topic?\n>\n>  * tc/clone-v-progress (2009-12-26) 4 commits\n>  - clone: use --progress to force progress reporting\n>  - clone: set transport->verbose when -v/--verbose is used\n>  - git-clone.txt: reword description of progress behaviour\n>  - check stderr with isatty() instead of stdout when deciding to show progress\n\nno, I'm not referring to that - the topic I mentioned is still off-list.\n\n> What do people think about it?  I vaguely recall that somebody asked to\n> add a warning to release notes on the behaviour change to this series, and\n> I think it may be a worthwhile thing to do (e.g. \"Earlier we did X but now\n> we do Y; change things in this way if you want us to keep doing X\"), but\n> otherwise I think it is a sensible change.\n\nYes, that request was from Dscho, and Miklos said something to that\neffect as well. Could you advise how one could go about adding such a\nwarning, as I'm not sure about the release schedule details.\n\n-- \nCheers,\nRay Chuan\n"}]}