On 15 Sep 2015, at 08:43, Luke Diamand <luke@xxxxxxxxxxx> wrote: > On 14/09/15 17:55, larsxschneider@xxxxxxxxx wrote: >> From: Lars Schneider <larsxschneider@xxxxxxxxx> >> >> A P4 repository can get into a state where it contains a file with >> type UTF-16 that does not contain a valid UTF-16 BOM. If git-p4 > > Sorry - what's a BOM? I'm assuming it's not a Bill of Materials? BOM stands for Byte Order Mark. The UTF-16 BOM is a two byte sequence at the beginning of a UTF-16 file. It is not part of the actual content. It is only used to define the encoding of the remaining file. FEFF stands for UTF-16 big-endian encoding and FFFE for little-endian encoding. More info here: http://www.unicode.org/faq/utf_bom.html#bom1 > 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 >> attempts to retrieve the file then the process crashes with a >> "Translation of file content failed" error. >> >> Fix this by detecting this error and retrieving the file as binary >> instead. The result in Git is the same. >> >> 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? > >> >> Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx> >> --- >> git-p4.py | 27 ++++++++++++++++----------- >> 1 file changed, 16 insertions(+), 11 deletions(-) >> >> diff --git a/git-p4.py b/git-p4.py >> index 073f87b..5ae25a6 100755 >> --- a/git-p4.py >> +++ b/git-p4.py >> @@ -134,13 +134,11 @@ def read_pipe(c, ignore_error=False): >> sys.stderr.write('Reading pipe: %s\n' % str(c)) >> >> expand = isinstance(c,basestring) >> - p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand) >> - pipe = p.stdout >> - val = pipe.read() >> - if p.wait() and not ignore_error: >> - die('Command failed: %s' % str(c)) >> - >> - return val >> + p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand) >> + (out, err) = p.communicate() >> + if p.returncode != 0 and not ignore_error: >> + die('Command failed: %s\nError: %s' % (str(c), err)) >> + return out >> >> def p4_read_pipe(c, ignore_error=False): >> real_cmd = p4_build_cmd(c) >> @@ -2186,10 +2184,17 @@ class P4Sync(Command, P4UserMap): >> # them back too. This is not needed to the cygwin windows version, >> # just the native "NT" type. >> # >> - text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" % (file['depotFile'], file['change']) ]) >> - if p4_version_string().find("/NT") >= 0: >> - text = text.replace("\r\n", "\n") >> - contents = [ text ] >> + 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 > >> + if 'Translation of file content failed' in str(e): >> + type_base = 'binary' >> + else: >> + raise e >> + else: >> + 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 In general, what is the appropriate way to reference code in this email list? Are GitHub links OK? Thanks, Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html