Repository navigation
Progress not showing up before the last second #444
Description
I've implemented the Progress class to implement a nice progress bar:
I'm then using it, for example here:
And then, when I clone a very large repository (like mozilla's gecko):
% git hub clone mozilla/gecko-dev
Pulling from github |████████████████████████████████|
Successfully cloned `https://github.com/guyzmo/git-repo` into `./git-repo`!
the progress bar is "progressing" (i.e. incrementing from 0 to 100%), but it shows up only at the very end of the fetching, after hanging for a long while. I started to debug it, and it looks like it's related to subprocess stuff.
Though, I might be implementing it wrong, which is why I'm asking here while I'm still debugging that issue.
Activity
Byron commentedon May 26, 2016
As far as I know, the progress line parsing is immediate, and thus will produce results in real time, similar to what you see on the command-line.
To understand what's the problem in your case, I recommend having a look at the parsing code to understand how exactly it works.
As an alternative step that you seem to have taken already, I would believe that just printing the remote progress arguments will indicate they are indeed provided in real-time.
If that's the case indeed, the problem would lie with the way the progress bar is used.
However, I am interested in your findings, maybe there is something unexpected here.
guyzmo commentedon May 26, 2016
There's definitely something fishy going on, I have replaced my "progress bar" instance with the following code:
and then I ran the command. I made an asciicinema output of the command to better see the pausing, followed by the output:
https://asciinema.org/a/cwoo3r1gnziyxgycbybd1p485
which is not outputting at the same rate as a raw git clone command:
http://asciinema.org/a/0u002zielzbpprrj6e5h20hpa
I already looked at the parsing code, and I agree it's looking totally fine, and it should work as intended. So maybe, a good idea would be to try to replicate the issue through a minimal snippet, to see whether my code could be interfering (which I believe should not, but who knows?).
I'll also try to add printouts in the parser to see where it's blocking.
guyzmo commentedon May 26, 2016
Ok, I've added a bunch of printouts, along with a timer to find where the pausing is happening (cf below).
So it's pretty obvious that the pausing is happening in
_get_fetch_info_from_stderr()between marks 2 and 3, which is:and now, it's pretty clear what's happening, on a file object
readlines()is reading all output up untilEOFand returns it as a list, it does not return an iterator. So as a fix, I'll try next with:which is the right way to iterate line by line over a file object.
full log:
Byron commentedon May 26, 2016
Great debugging and findings ! I just pushed the fix !
And thanks a lot !
guyzmo commentedon May 26, 2016
Well, it's actually still not very smooth:
https://asciinema.org/a/55m8ugnq1j43k69keiasnjzoo
and, as I am using master's HEAD, there's an issue with it showing:
I guess it's not relevant to the issue at hand, though.
Then, I ran it again with the fix and timers in place, and here's the output:
What I'm thinking is that there's another issue here, and it's that the python file object might be iterating over
\n, but progress is mostly shown using\rby git. Thus, it's only updating the output a couple of times with big pauses because it's not returning each line ending with\r(that might actually explain the error shown). We'd need to check whether how thestderrfile object is being configured bysubprocessand whether it explicitly disables universal line ending mode.Byron commentedon May 26, 2016
Maybe ioctl can be used to configure the handle after it was created. Or there is a way to do that where the process is spawned.
43 remaining items
barry-scott commentedon Oct 11, 2016
@Byron I thought this was fixed. I'm not seeing the problem any more. Can this issue me closed?
guyzmo commentedon Oct 11, 2016
worksforme ☺
ankostis commentedon Oct 11, 2016
We can close it then, fixed under the milestone it is currently under!
Thanks.
(do you want to push any AUTHOR changes?)
guyzmo commentedon Oct 11, 2016
sure thanks, I'll do that in a currently pending PR I'm pushing!
mrozekma commentedon Aug 6, 2017
Not sure if I should file a new bug or resurrect this one, but an earlier comment says:
Was cloning just overlooked?
git.Repo.clone_fromseems to have the exact same problem. I fixed it locally by changinggit.remote.add_progressto also setkwargs['universal_newlines'] = True, and changinggit.base.Repo._cloneto passdecode_streams = Falsetohandle_process_outputByron commentedon Sep 28, 2017
@mrozekma I believe creating a new issue would be the way to go, while assuring the issue can be reproduced with the latest version.
fix(remote): real-time reading of lines from stderr