Re: [PATCH v2 2/2] git-p4: handle "Translation of file content failed"

[Luke Diamand <luke@diamand.org>]

On 15/09/15 16:38, Lars Schneider wrote:
>
> On 15 Sep 2015, at 08:43, Luke Diamand <luke@diamand.org> wrote:
>


>> Do we know the mechanism by which we end up in this state?
> Unfortunately no. I tried hard to reproduce the error with “conventional” methods. As you can see I ended up manipulating the P4 database…
>
> However, it looks like this error happens in the wild, too:
> https://stackoverflow.com/questions/5156909/translation-of-file-content-failed-error-in-perforce
> https://stackoverflow.com/questions/887006/perforce-translation-of-file-content-failed-error

It's described in the Perforce FAQ here:

http://answers.perforce.com/articles/KB/3117

i.e. it looks to be caused by mixing old and new P4 clients.

>>>
>>> Known issue: This works only if git-p4 is executed in verbose mode.
>>> In normal mode no exceptions are thrown and git-p4 just exits.
>>
>> Does that mean that the error will only be detected in verbose mode? That doesn't seem right!
> Correct. I don’t like this either but I also don’t want to make huge changes to git-p4.
> You can see the root problem here:
> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L110-L114
>
> Any idea how to approach that best?

I guess what we have is not ideal but probably good enough.


>>> +            try:
>>> +                text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])
>>> +            except Exception as e:
>>
>> Would it be better to specify which kind of Exception you are catching? Looks like you could get OSError, ValueError and CalledProcessError; it's the last of these you want (I think).
> I think it is just a plain exception. See here:
> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L111

OK, you're right (probably less than ideal behaviour from read_pipe() 
and die() but let's not try to fix that).


>>> +                if p4_version_string().find('/NT') >= 0:
>>> +                    text = text.replace('\r\n', '\n')
>>> +                contents = [ text ]
>>
>> The indentation on this bit doesn't look right to me.
> I believe it is exactly how it was:
> https://github.com/git/git/blob/97d7ad75b6fe74960d2a12e4a9151a55a5a87d6d/git-p4.py#L2397-L2399

OK.

>
>
> In general, what is the appropriate way to reference code in this email list? Are GitHub links OK?

I'm not an expert, but it feels possibly a bit ephemeral - if someone is 
digging through email archives in a future where that github project has 
been moved elsewhere, the links will all be dead.

Luke