Repository navigation
Progress: issue with bytes/str handling (py3) #564
Description
Activity
here's a stack trace where it happens, with gitpython-2.1.1 (I updated since last report):
> git remote add all https://bitbucket.org/atlassian/python-bitbucket > Popen(['git', 'remote', 'add', 'all', 'https://bitbucket.org/atlassian/python-bitbucket'], cwd=/tmp/tmp_f32zp1_, universal_ newlines=False, shell=None) > git remote add bitbucket https://bitbucket.org/atlassian/python-bitbucket > Popen(['git', 'remote', 'add', 'bitbucket', 'https://bitbucket.org/atlassian/python-bitbucket'], cwd=/tmp/tmp_f32zp1_, univ ersal_newlines=False, shell=None) > git version > Popen(['git', 'version'], cwd=/tmp/tmp_f32zp1_, universal_newlines=False, shell=None) > git pull --progress -v bitbucket master > Popen(['git', 'pull', '--progress', '-v', 'bitbucket', 'master'], cwd=/tmp/tmp_f32zp1_, universal_newlines=True, shell=None ) > Pumping 'stderr' of cmd(['git', 'pull', '--progress', '-v', 'bitbucket', 'master']) failed due to: TypeError("a bytes-like object is required, not 'str'",) Exception in thread Thread-2: Traceback (most recent call last): File "…/git/cmd.py", line 87, in pump_stream handler(line) File "…/git/util.py", line 483, in handler return self._parse_progress_line(line.rstrip()) File "…/git/util.py", line 388, in _parse_progres s_line if len(self.error_lines) > 0 or self._cur_line.startswith(('error:', 'fatal:')): TypeError: a bytes-like object is required, not 'str' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/lib/python3.5/threading.py", line 914, in _bootstrap_inner self.run() File "/usr/lib/python3.5/threading.py", line 862, in run self._target(*self._args, **self._kwargs) File "…/git/cmd.py", line 90, in pump_stream raise CommandError(['<%s-pump>' % name] + cmdline, ex) git.exc.CommandError: Cmd('<stderr-pump>') failed due to: TypeError('a bytes-like object is required, not 'str'') cmdline: <stderr-pump> git pull --progress -v bitbucket masterI think I'm narrowing the issue to the fact that input is supposed to be bytes (former
str()), but all string litterals are unicode over there.But because the
handle_process_output()function is called from_get_fetch_info_from_stderr()withdecode_stream=False, the pump function is not decoding thebytestostr, and later in theprogress_handlerit fails.I switched all
decode_streamstoTrueingit.remoteand it's not crashing anymore.In either cases, with that solution it's yelling:
cmd.py 585 DEBUG Popen(['git', 'pull', '--progress', '-v', 'bitbucket', 'master'], cwd=/tmp/tmp4z86lqw9, universal_newlines=True, shell=None) remote.py 660 DEBUG Fetch head lines do not match lines provided via progress information length of progress lines 2 should be equal to lines in FETCH_HEAD file 1 Will ignore extra progress lines or fetch head lines.but I believe it's unrelated to the issue at hand.
So we've got two solutions, etiher make the progress handler bytes and str agnostic (as I first suggested), or change the remote.fetch calls to
handle_process_output().@guyzmo Thanks for the elaborate description of the problem and the effort you put in already! As for a PR, I believe to remember that
decode_streamsbeing False was required to make that (or other) things work, so I would be careful going down that path. Therefore it seems making the parser agnostic of the input type is a more localised fix that seems preferable.
It would be interesting to understand while the type is changing in the first place too, maybe you discover that while looking into the issue.uuuurgh… I tried to reproduce the issue today, by removing my patch… and now it's working fine…
What. The. Bloody. Hell. O_O
I guess I'm closing the issue for now, and next time it happens I'll try to make a minimal replicable snippet of it. I mean it happened twice, it's likely to hit me a third time, because as we say in french: jamais deux sans trois.
Reacted by Sebastian Thieldamn… it just hit me again. Same circumstances, same situation.
self._cur_line = line = line.decode('utf-8') if isinstance(line, bytes) else line
fixed it.
Though right now I'm focused on doing a new release of
git-repo, and I want to stay focused on it. I'll see if I got time to make a minimal snippet next week.The root of the problem is because I'm mocking up the git calling subprocess calls:
I'd be happy to, are you happy with the fix I'm suggesting above?
It looks good to me, all things considered :). I mean it's probably a lost cause to try to get bytes/string handling right in this project, so applying fixes where needed seems to be a suitable way to make life for users a little bit better.
👌 I'm adding that in my todo list for the next few days ☺
Reacted by Sebastian Thiel and Daryl TuckerI wonder if this issue might have been solved by now?
@yarikoptic I hope with the linked commit, this issue will be resolved. It's a certainly a quick stab at it.
Closed due to inactivity - please feel free to comment if there is an interest to pick it up.
I still am unable to produce a proper minimal reproduceable example — I've been busy with feature implementation to dig this further, but depending on some context to be determined, in the method
_parse_progress_line()ofRemoteProgresssometimes the line argument is abytesstring, other times it's anstrstring.I've hit the issue with my implementation in guyzmo/git-repo and to fix it I've made the following fix at line 387 of git.util:
This issue has been noticed with GitPython 2.1.0, I'm upgrading it locally, to see if it hasn't been fixed with 2.1.1, so please pardon me if it's been fixed ☺