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

From: Lars Schneider <larsxschneider@gmail.com>
Date: Sep 16, 2015, 11:38 UTC
Message-ID: <C8BD1D98-21BB-420B-884E-79D0FC71D119@gmail.com>
In-Reply-To: <55F897D0.9000100@diamand.org>

On 16 Sep 2015, at 00:12, Luke Diamand <luke@diamand.org> wrote:

Show 18 quoted lines

> 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.

Good find! No idea why I did not find this article before… I will copy the text from the KB into the git commit message to explain the problem.

Show 13 quoted lines

> 
>>>> 
>>>> 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.

ok. thanks!

I will add another test case without “—verbose" to document that there is work to do :-)

Show 11 quoted lines

> 
> 
>>>> +            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).

ok

Show 17 quoted lines

> 
> 
>>>> +                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.

Right. However, you could disassemble the URL and use the commit hash, the filename and the line number. They are not ephemeral because they are part of the repo.

Thanks, Lars