patchgit-update-index: report(...) now flushes stdout after printing the report line

This makes it equivalent to the behavior of git-hash-object and allows tools to
write one path
to stdin, flush and assure the work is done once it reads the corresponding
report line.
Previously attempting to do that would result in the program blocking as
git-update-index
did not flush its report line (yet). External programs use the git-hash-object
like behavior
to precisely control when which work is done while providing just-in-time
feedback to the end-user.
---
 builtin-update-index.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Junio, could you tell us what happened to this thread?

Nanako Shiraishi <nanako3@lavabit.com> writes:

I didn't feel I had enough energy to read the commit log message after seeing it was badly linewrapped and didn't have a sign-off, so I didn't read it.

I've read it now; it is unclear from the proposed commit log message how this fits in the larger picture. Presumably this change is meant to be useful when driving update-index through --stdin? To see if I got the intention right, let me try paraphrasing it...

    update-index: flush standard output after each action is reported

    A scripted Porcelain that runs "git update-index --stdin" might want
    to use a bidirectional pipe, while feeding one path at a time and
    reading the output from report() every time after feeding a path.

    Such a Porcelain would deadlock if the standard output is not flushed
    after report().

I don't know if the above is what Sebastian meant, though..

An obvious question, when phrased this way, is "what impact does this change have for scripted Porcelains that don't use bi-di pipe?" I think the answer would be "The I/O overhead for flushing would increase", but I don't know if it would be "... would increase but it is still negligible" or "... would increase too much to make it noticeably or unusably slow especially if it feeds hundreds of paths". If it is the latter, this may need to be controlled by another command line option.

Sebastian, care to redo the justification, make it a bit more readable, and add your sign-off?

Thanks.

I'd like to add that since version 1.6.5, non-tty's do not receive any progress information anymore. The patch causing this says it wants to, in short words, unify the push and fetch handling regarding the way progress messages are sent.

Now third-party wrappers, such as git-python, are unable to provide any progress information anymore for possibly lengthy operations.

This is why I clearly recommend to add some kind of a "progress-force" flag that turns progress messages on again for send-pack and receive-pack.

Sorry for the badly formatted message, and thanks a lot for the correction which is what my post should have been in the first place.

Redoing the commit is not what will be needed for git-python to work properly which is why I will tell the whole story before submitting any(more) patches.

With git v1.6.5, git-push was adjusted not to provide progress messages anymore if the device attached to stderr is not a tty. Previously, this was only the case with git-fetch. For git-python, and other callers of the commandline such as tortoise-git, there currently is no way to provide progress information to the user unless they (somehow) simulate a tty which appears unfeasible. When using these tools, time consuming operations tend to appear as if they are hanging. One might argue that most code is fetched and pushed in a matter of seconds, but if git is used to store large binary data, processing and transferring it will take time.

The issue mentioned with git-update-index and it's missing flush that would cause a deadlock in some porcelain can be fixed trivially, but seen in the context of the git-push and git-pull a more thorough solution might be more appealing. As mentioned by Junio, a default flush after each report might slow down some existing porcelain, and a commandline option would be part of the proper solution. I would argue though that a separate option would add quite some complexity to the command as it is a very specialized one. Instead I would recommend checking whether --stdin is given on the commandline, and flush stdout if that is true. This would natively make the command behave like git-hash-object and git-cat-file. If --stdin is not provided, report is not required to flush after every call as the commandline options are processed without additional user interaction.

Adding a commandline option to git-push and git-pull that enforces progress messages to be printed to stderr would be a feasible and simple fix that would clearly improve the usability of tortoise-git and git-python to name only two.

That said, I hope I managed to make myself clear enough this time to help the people in charge to figure out how to solve the issue. Once the desired solution has been sketched out and the desired new commandline options have been named, it could even be me to implement it if necessary, as I'd consider it a gentle start into the world of the git codebase.

Thanks for picking this up again, Sebastian

Hi,

(you dropped the Cc list; fixed that for you.)

On Sun, Jan 3, 2010 at 6:41 PM, Sebastian Thiel <byronimo@gmail.com> wrote:

from your above message solely and setting aside your original patch, I presume that you want to introduce the ability to force progress reporting even if stderr isn't a terminal.

I am working a feature (display progress for http operations) that happens to add this ability to git-push and git-fetch, by specifying the --progress option.

Regarding git-pull - I guess it's only git-fetch (being transport-related) that reports progress?

Thanks Ray Chuan, for the clarification, the progress is supposed to be sent in git-push and git-fetch ( not git-pull as I mentioned ). A --progress flag would already do it for me, is there a way to fetch your code from somewhere for a test run ?

When do you think will your changes be available for a mainline merge, or would it even be possible to separate the --progress adjustment from your feature to merge it into mainline individually ?

Thanks, Sebastian

Tay Ray Chuan <rctay89@gmail.com> writes:

Are you talking about this topic?

 * tc/clone-v-progress (2009-12-26) 4 commits
  - clone: use --progress to force progress reporting
  - clone: set transport->verbose when -v/--verbose is used
  - git-clone.txt: reword description of progress behaviour
  - check stderr with isatty() instead of stdout when deciding to show progress

What do people think about it? I vaguely recall that somebody asked to add a warning to release notes on the behaviour change to this series, and I think it may be a worthwhile thing to do (e.g. "Earlier we did X but now we do Y; change things in this way if you want us to keep doing X"), but otherwise I think it is a sensible change.

Hi,

On Wed, Jan 6, 2010 at 9:04 AM, Junio C Hamano <gitster@pobox.com> wrote:

no, I'm not referring to that - the topic I mentioned is still off-list.

Yes, that request was from Dscho, and Miklos said something to that effect as well. Could you advise how one could go about adding such a warning, as I'm not sure about the release schedule details.