> On 12 Jun 2017, at 19:11, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Sergey Yurzin <jurzin.s@xxxxxxxxx> writes:
>
>> Subject: Re: [PATCH] Fix KeyError "fileSize" in verbose mode
>
> ...
>
>> git-p4.py | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index 8d151da91b969..b3666eddf12e3 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2523,8 +2523,11 @@ def streamOneP4File(self, file, contents):
>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
>> relPath = self.encodeWithUTF8(relPath)
>> if verbose:
>> - size = int(self.stream_file['fileSize'])
>> - sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], relPath, size/1024/1024))
>> + if 'fileSize' in self.stream_file:
>> + size = int(self.stream_file['fileSize'])
>> + sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], relPath, size/1024/1024))
>> + else:
>> + sys.stdout.write('\r%s --> %s\n' % (file['depotFile'], relPath))
>> sys.stdout.flush()
>
Are you working with P4 Streams? I think I ran into the same problem
if Streams are involved and I solved it like this:
if verbose:
- size = int(self.stream_file['fileSize'])
+ if 'fileSize' in self.stream_file:
+ size = int(self.stream_file['fileSize'])
+ elif 'streamContentSize' in self.stream_file:
+ size = int(self.stream_file['streamContentSize'])
+ else:
+ size = 0
sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], relPath, size/1024/1024))
sys.stdout.flush()
However "streamContentSize" is something we define ourselves elsewhere
in git-p4 and not something we get from P4. Therefore it might be
garbage. I haven't looked into it further, yet.
Your 'fileSize' check is definitively correct!
Cheers,
Lars
> I can see from your patch that self.stream_file[] sometimes may not
> have `fileSize` and when that happens the current code will barf. I
> also can see that with your patch, the code will NOT barf but output
> would lack the size information (obviously, because it is not
> available).
>
> However, it is not at all obvious if this is fixing the problem or
> sweeping the problem under the rug. The proposed log message you
> write before the patch is the ideal place to say something like
>
> In such and such circumstances, it is perfectly normal that
> P4Sync.stream_file does not know its file and lacks `fileSize`.
> streamOneP4File() method, however, assumes that this key is
> always available and tries to write it under the verbose mode.
>
> Check the existence of the `fileSize` key and show it only when
> available.
>
> Note that the above _assumes_ that a stream_file that lacks
> `fileSize` is perfectly normal; if that assumption is incorrect,
> then perhaps a real fix may be to set `fileSize` in the codepath
> that forgets to set it (I am not a git-p4 expert, and I am asking
> Luke to review this patch by CC'ing him).
>
> Also note that in a real log message that is helpful for future
> readers, "In such and such circumstances" in the above illustration
> needs to become a more concrete description.
>
> Thanks, and welcome to Git development community.
>
>> (type_base, type_mods) = split_p4_type(file["type"])
>>
>> --
>> https://github.com/git/git/pull/373