From: Nikolay Shustov <nikolay.shustov@gmail.com>
Date: Tue, 08 Apr 2025 12:13:41 GMT
Subject: Re: [PATCH] git p4 fix for failure to decode p4 errors
Message-ID: <e91c0859-da89-47a2-b0c7-ce1943318529@gmail.com>
In-Reply-To: <501e308d-61b3-429a-bc4a-6f0c81455279@gmail.com>

Hi Fahad,
I hope you are doing well.
If there are troubles with trying this on your Perforce system, maybe we 
could think of other way verifying the patch?

Thank you,
- Nikolay

On 4/5/25 14:46, Nikolay Shustov wrote:
> Hello Fahad,
> Did you have a chance to try it with your Perforce system yet?
>
> Thanks,
> - Nikolay
>
> On 3/31/25 19:37, Nikolay Shustov wrote:
>> Hi Fahad,
>> Thank you for taking a look.
>> Yes, you can run this test locally, that should be enough.
>> The test is conditionalized on a) having p4 server installation with 
>> no Unicode support and b) running with Python 3 only.
>> If either of these do not match, it will skip the execution.
>>
>> I tested it on my computer with the freshly installed p4 helix server 
>> and client; but it was on Kubuntu only, did not try it on Windows.
>> Perforce company allows limited use of the server and client for 
>> non-commercial purposes, so it was all legit.
>>
>> Please let me know how it runs for you.
>>
>> Cheers,
>> - Nikolay
>>
>> On 3/31/25 05:01, Fahad Al-Rashed wrote:
>>> Hi Nikolay,
>>>
>>> The patch looks reasonable.
>>>
>>> What I can help with is test it on our Perforce installation when I 
>>> go back to work next week.
>>>
>>> For the purpose of testing, is running 
>>> t/t9837-git-p4-error-encoding.sh locally on my computer enough to 
>>> test your patch?
>>>
>>> Best,
>>> Fahad
>>>
>>>> On 31 Mar 2025, at 4:21 AM, Nikolay Shustov 
>>>> <nikolay.shustov@gmail.com> wrote:
>>>>
>>>> <adding Fahad Alrashed, James Touton and Patrick Steinhardt, whom 
>>>> Git points to as the contributors to the latest p4-git logic changes>
>>>>
>>>> Hello,
>>>> May I ask you to review the below change to p4-git?
>>>>
>>>> Thank you in advance,
>>>> - Nikolay
>>>>
>>>> On 3/30/25 16:06, Nikolay Shustov wrote:
>>>>> Hi Phillip,
>>>>> Thank you for your time and your feedback.
>>>>> It is especially valuable to me as it is the very first PR of mine.
>>>>> I will try to contact the recent contributors of git-p4 changes 
>>>>> for review.
>>>>>
>>>>> To clarify on the fix:
>>>>>
>>>>> The error I hit was while using "git p4 clone":
>>>>> It was throwing decoding exception at line 901 of git-p4, 
>>>>> preventing import from Perforce depot to complete successfully.
>>>>> The root cause is the logic for "git p4 clone" anticipates some p4 
>>>>> operations may return errors, it is a normal part of import process.
>>>>> But that logic uses just .decode() on the byte array of the 
>>>>> returned error message, which does not work well when it contains 
>>>>> the characters with high bit set (which may be the case when 
>>>>> Perforce configured without unicode support). git-p4 
>>>>> implementation has a decoding fallback logic for such cases in 
>>>>> other places, but this specific place did not use any.
>>>>>
>>>>> Using the bullet list in description was not intended to enumerate 
>>>>> the separate changes, but rather to highlight the details of the 
>>>>> change.
>>>>> I will make sure I won't use it in the future to avoid the confusion.
>>>>>
>>>>> That small refactoring I did was not a sidecar but a way I chose 
>>>>> to implement the changes:
>>>>> There was an existing function that was doing the job of decoding 
>>>>> the received p4 metadata, using the existing git-p4 configuration 
>>>>> settings.
>>>>> There also were a few existing variables that kept the state 
>>>>> between the calls of that function (e.g. indicator not to show 
>>>>> decoding fallback warning twice, configuration settings).
>>>>> However, with the way the function was implemented, it could not 
>>>>> be reused as-is for the new case.
>>>>> I would had to add a new function that would have to use the same 
>>>>> core transcoding implementation but behave differently.
>>>>> Adding behavior variances into the existing function felt 
>>>>> suboptimal: it would complicate it quite a bit and I felt sorry 
>>>>> about next one who will have to reverse engineer its behavior 
>>>>> again. Duplicating the part of logic of the existing function also 
>>>>> looked suboptimal: any further changes would have to be done in 
>>>>> two places.
>>>>> So, seeing the need in keeping state between calls and separating 
>>>>> a part of existing logic into separate functions, I went for 
>>>>> moving the implementation into a new class and organizing things 
>>>>> there with the class instance. In my opinion, the new code looks 
>>>>> pretty self-descritpitve.
>>>>>
>>>>> Thank you,
>>>>> - Nikolay
>>>>>
>>>>> On 3/26/25 11:09, Phillip Wood wrote:
>>>>>> Hi Nikolay
>>>>>>
>>>>>> On 25/03/2025 23:09, Nikolay Shustov wrote:
>>>>>>> I think this fix is important.
>>>>>>> git-p4 is used in the companies where there is an intent to 
>>>>>>> migrate from Perforce to Git and having the issue that this 
>>>>>>> change fixes is a real roadblock.
>>>>>>> The better we can make git-p4, the more adoption Git would get 
>>>>>>> in the commercial world.
>>>>>> Unfortunately I don't think any of the regular git contributors 
>>>>>> use git-p4 so to find someone to review this patch I would look 
>>>>>> at who has contributed to git-p4 recently and cc them. Before you 
>>>>>> do that I have a couple of suggestions below
>>>>>>
>>>>>>> On 3/22/25 07:48, Nikolay Shustov wrote:
>>>>>>>> ping, pretty please? :-)
>>>>>>>>
>>>>>>>> On 3/19/25 23:20, Nikolay Shustov via GitGitGadget wrote:
>>>>>>>>> From: Nikolay Shustov <Nikolay.Shustov@gmail.com>
>>>>>>>>>
>>>>>>>>> Fixes the git p4 failure happening when Perforce command 
>>>>>>>>> returns error
>>>>>>>>> containing byte stream of characters with high bit set. In 
>>>>>>>>> such situations
>>>>>>>>> git p4 implementatino fails to decode this byte stream into 
>>>>>>>>> utf-8 string.
>>>>>>>>>
>>>>>>>>> Design:
>>>>>>>>> Make use of existing decoding fallback strategy, described by
>>>>>>>>> git-p4.metadataDecodingStrategy and 
>>>>>>>>> git-p4.metadataFallbackEncoding
>>>>>>>>> settings in the logic that decodes the Perforce command error 
>>>>>>>>> bytes.
>>>>>> Our usual style for commit messages is to explain what the 
>>>>>> problem is and how it is fixed by the changes in the patch. 
>>>>>> Rather than saying "fixes the git p4 failure" I would start by 
>>>>>> explaining what that failure is and how it is caused. It would 
>>>>>> also be helpful to explain what the settings that you refer to do 
>>>>>> so that someone who is familiar with python but not with git-p4 
>>>>>> can understand and potentially review the changes.
>>>>>>
>>>>>>>>> Details:
>>>>>>>>> - Moved p4 metadata transcoding logic from
>>>>>>>>>     metadata_stream_to_writable_bytes(..) into a new 
>>>>>>>>> MetadataTranscoder class.
>>>>>>>>> - Enhcanced the implementation to use 
>>>>>>>>> git-p4.metadataDecodingStrategy and
>>>>>>>>>     git-p4.metadataFallbackEncoding settings for p4 errors 
>>>>>>>>> decoding.
>>>>>>>>> - Added test.
>>>>>> Thanks for taking the time to add a new test, it is much 
>>>>>> appreciated. When there is a bullet list in a commit message it 
>>>>>> is often a sign that the commit is doing more than one thing at 
>>>>>> once. In this case it appears there is a bug fix mixed in with 
>>>>>> some refactoring. I would split the refactoring out into a 
>>>>>> preparatory patch so that reviews can clearly see which changes 
>>>>>> are due to creating the MetadataTranscoder class and which are 
>>>>>> the changes that fix the bug. The new test should be added in the 
>>>>>> commit that fixes the bug.
>>>>>>
>>>>>> Best Wishes
>>>>>>
>>>>>> Phillip
>>>>>>
>>>>>>>>> Signed-off-by: Nikolay Shustov <Nikolay.Shustov@gmail.com>
>>>>>>>>> ---
>>>>>>>>>       git p4 fix for failure to decode p4 errors
>>>>>>>>>
>>>>>>>>> Published-As: 
>>>>>>>>> https://github.com/gitgitgadget/git/releases/tag/pr- 
>>>>>>>>> git-1926%2Fnshustov%2Fgit-p4-error-decoding-v1
>>>>>>>>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
>>>>>>>>> pr- git-1926/nshustov/git-p4-error-decoding-v1
>>>>>>>>> Pull-Request: https://github.com/git/git/pull/1926
>>>>>>>>>
>>>>>>>>>    git-p4.py                        | 135 
>>>>>>>>> ++++++++++++++++++-------------
>>>>>>>>>    t/meson.build                    |   1 +
>>>>>>>>>    t/t9837-git-p4-error-encoding.sh |  53 ++++++++++++
>>>>>>>>>    t/t9837/git-p4-error-python3.py  |  15 ++++
>>>>>>>>>    4 files changed, 149 insertions(+), 55 deletions(-)
>>>>>>>>>    create mode 100755 t/t9837-git-p4-error-encoding.sh
>>>>>>>>>    create mode 100644 t/t9837/git-p4-error-python3.py
>>>>>>>>>
>>>>>>>>> diff --git a/git-p4.py b/git-p4.py
>>>>>>>>> index c0ca7becaf4..72a4c55f99e 100755
>>>>>>>>> --- a/git-p4.py
>>>>>>>>> +++ b/git-p4.py
>>>>>>>>> @@ -234,67 +234,91 @@ else:
>>>>>>>>>        class MetadataDecodingException(Exception):
>>>>>>>>> -    def __init__(self, input_string):
>>>>>>>>> +    def __init__(self, input_string, error=None):
>>>>>>>>>            self.input_string = input_string
>>>>>>>>> +        self.error = error
>>>>>>>>>          def __str__(self):
>>>>>>>>> -        return """Decoding perforce metadata failed!
>>>>>>>>> +        message = """Decoding perforce metadata failed!
>>>>>>>>>    The failing string was:
>>>>>>>>>    ---
>>>>>>>>>    {}
>>>>>>>>>    ---
>>>>>>>>>    Consider setting the git-p4.metadataDecodingStrategy config 
>>>>>>>>> option to
>>>>>>>>>    'fallback', to allow metadata to be decoded using a 
>>>>>>>>> fallback encoding,
>>>>>>>>> -defaulting to cp1252.""".format(self.input_string)
>>>>>>>>> +defaulting to cp1252."""
>>>>>>>>> +        if verbose and self.error is not None:
>>>>>>>>> +            message += """
>>>>>>>>> +---
>>>>>>>>> +Error:
>>>>>>>>> +---
>>>>>>>>> +{}"""
>>>>>>>>> +        return message.format(self.input_string, self.error)
>>>>>>>>>      -encoding_fallback_warning_issued = False
>>>>>>>>> -encoding_escape_warning_issued = False
>>>>>>>>> -def metadata_stream_to_writable_bytes(s):
>>>>>>>>> -    encodingStrategy = 
>>>>>>>>> gitConfig('git-p4.metadataDecodingStrategy') or 
>>>>>>>>> defaultMetadataDecodingStrategy
>>>>>>>>> -    fallbackEncoding = 
>>>>>>>>> gitConfig('git-p4.metadataFallbackEncoding') or 
>>>>>>>>> defaultFallbackMetadataEncoding
>>>>>>>>> -    if not isinstance(s, bytes):
>>>>>>>>> -        return s.encode('utf_8')
>>>>>>>>> -    if encodingStrategy == 'passthrough':
>>>>>>>>> -        return s
>>>>>>>>> -    try:
>>>>>>>>> -        s.decode('utf_8')
>>>>>>>>> -        return s
>>>>>>>>> -    except UnicodeDecodeError:
>>>>>>>>> -        if encodingStrategy == 'fallback' and fallbackEncoding:
>>>>>>>>> -            global encoding_fallback_warning_issued
>>>>>>>>> -            global encoding_escape_warning_issued
>>>>>>>>> -            try:
>>>>>>>>> -                if not encoding_fallback_warning_issued:
>>>>>>>>> -                    print("\nCould not decode value as utf-8; 
>>>>>>>>> using configured fallback encoding %s: %s" % 
>>>>>>>>> (fallbackEncoding, s))
>>>>>>>>> -                    print("\n(this warning is only displayed 
>>>>>>>>> once during an import)")
>>>>>>>>> - encoding_fallback_warning_issued = True
>>>>>>>>> -                return 
>>>>>>>>> s.decode(fallbackEncoding).encode('utf_8')
>>>>>>>>> -            except Exception as exc:
>>>>>>>>> -                if not encoding_escape_warning_issued:
>>>>>>>>> -                    print("\nCould not decode value with 
>>>>>>>>> configured fallback encoding %s; escaping bytes over 127: %s" 
>>>>>>>>> % (fallbackEncoding, s))
>>>>>>>>> -                    print("\n(this warning is only displayed 
>>>>>>>>> once during an import)")
>>>>>>>>> - encoding_escape_warning_issued = True
>>>>>>>>> -                escaped_bytes = b''
>>>>>>>>> -                # bytes and strings work very differently in 
>>>>>>>>> python2 vs python3...
>>>>>>>>> -                if str is bytes:
>>>>>>>>> -                    for byte in s:
>>>>>>>>> -                        byte_number = struct.unpack('>B', 
>>>>>>>>> byte)[0]
>>>>>>>>> -                        if byte_number > 127:
>>>>>>>>> -                            escaped_bytes += b'%'
>>>>>>>>> -                            escaped_bytes += hex(byte_number) 
>>>>>>>>> [2:].upper()
>>>>>>>>> -                        else:
>>>>>>>>> -                            escaped_bytes += byte
>>>>>>>>> -                else:
>>>>>>>>> -                    for byte_number in s:
>>>>>>>>> -                        if byte_number > 127:
>>>>>>>>> -                            escaped_bytes += b'%'
>>>>>>>>> -                            escaped_bytes += 
>>>>>>>>> hex(byte_number).upper().encode()[2:]
>>>>>>>>> -                        else:
>>>>>>>>> -                            escaped_bytes += 
>>>>>>>>> bytes([byte_number])
>>>>>>>>> -                return escaped_bytes
>>>>>>>>> +class MetadataTranscoder:
>>>>>>>>> +    def __init__(self, default_metadata_decoding_strategy, 
>>>>>>>>> default_fallback_metadata_encoding):
>>>>>>>>> +        self.decoding_fallback_warning_issued = False
>>>>>>>>> +        self.decoding_escape_warning_issued = False
>>>>>>>>> +        self.decodingStrategy = gitConfig('git- 
>>>>>>>>> p4.metadataDecodingStrategy') or 
>>>>>>>>> default_metadata_decoding_strategy
>>>>>>>>> +        self.fallbackEncoding = gitConfig('git- 
>>>>>>>>> p4.metadataFallbackEncoding') or 
>>>>>>>>> default_fallback_metadata_encoding
>>>>>>>>> +
>>>>>>>>> +    def decode_metadata(self, s, error_from_fallback=True):
>>>>>>>>> +        try:
>>>>>>>>> +            return [s.decode('utf_8'), 'utf_8']
>>>>>>>>> +        except UnicodeDecodeError as decode_exception:
>>>>>>>>> +            error = decode_exception
>>>>>>>>> +            if self.decodingStrategy == 'fallback' and 
>>>>>>>>> self.fallbackEncoding:
>>>>>>>>> +                try:
>>>>>>>>> +                    if not 
>>>>>>>>> self.decoding_fallback_warning_issued:
>>>>>>>>> +                        print("\nCould not decode value as 
>>>>>>>>> utf-8; using configured fallback encoding %s: %s" % 
>>>>>>>>> (self.fallbackEncoding, s))
>>>>>>>>> +                        print("\n(this warning is only 
>>>>>>>>> displayed once during an import)")
>>>>>>>>> + self.decoding_fallback_warning_issued = True
>>>>>>>>> +                    return [s.decode(self.fallbackEncoding), 
>>>>>>>>> self.fallbackEncoding]
>>>>>>>>> +                except Exception as decode_exception:
>>>>>>>>> +                    if not error_from_fallback:
>>>>>>>>> +                        return [s, None]
>>>>>>>>> +                    error = decode_exception
>>>>>>>>> +            raise MetadataDecodingException(s, error)
>>>>>>>>> +
>>>>>>>>> +    def metadata_stream_to_writable_bytes(self, s):
>>>>>>>>> +        if not isinstance(s, bytes):
>>>>>>>>> +            return s.encode('utf_8')
>>>>>>>>> +        if self.decodingStrategy == 'passthrough':
>>>>>>>>> +            return s
>>>>>>>>> +
>>>>>>>>> +        [text, encoding] = self.decode_metadata(s, False)
>>>>>>>>> +        if encoding == 'utf_8':
>>>>>>>>> +            # s is of utf-8 already
>>>>>>>>> +            return s
>>>>>>>>> +
>>>>>>>>> +        if encoding is None:
>>>>>>>>> +            # could not decode s, even with fallback encoding
>>>>>>>>> +            if not self.decoding_escape_warning_issued:
>>>>>>>>> +                print("\nCould not decode value with 
>>>>>>>>> configured fallback encoding %s; escaping bytes over 127: %s" 
>>>>>>>>> % (self.fallbackEncoding, s))
>>>>>>>>> +                print("\n(this warning is only displayed once 
>>>>>>>>> during an import)")
>>>>>>>>> + self.decoding_escape_warning_issued = True
>>>>>>>>> +            escaped_bytes = b''
>>>>>>>>> +            # bytes and strings work very differently in 
>>>>>>>>> python2 vs python3...
>>>>>>>>> +            if str is bytes:
>>>>>>>>> +                for byte in s:
>>>>>>>>> +                    byte_number = struct.unpack('>B', byte)[0]
>>>>>>>>> +                    if byte_number > 127:
>>>>>>>>> +                        escaped_bytes += b'%'
>>>>>>>>> +                        escaped_bytes += 
>>>>>>>>> hex(byte_number)[2:].upper()
>>>>>>>>> +                    else:
>>>>>>>>> +                        escaped_bytes += byte
>>>>>>>>> +            else:
>>>>>>>>> +                for byte_number in s:
>>>>>>>>> +                    if byte_number > 127:
>>>>>>>>> +                        escaped_bytes += b'%'
>>>>>>>>> +                        escaped_bytes += 
>>>>>>>>> hex(byte_number).upper().encode()[2:]
>>>>>>>>> +                    else:
>>>>>>>>> +                        escaped_bytes += bytes([byte_number])
>>>>>>>>> +            return escaped_bytes
>>>>>>>>>    -        raise MetadataDecodingException(s)
>>>>>>>>> +        # were able to decode but not to utf-8
>>>>>>>>> +        return text.encode('utf_8')
>>>>>>>>>        def decode_path(path):
>>>>>>>>> @@ -898,14 +922,14 @@ def p4CmdList(cmd, stdin=None, 
>>>>>>>>> stdin_mode='w+b', cb=None, skip_info=False,
>>>>>>>>>                        decoded_entry[key] = value
>>>>>>>>>                    # Parse out data if it's an error response
>>>>>>>>>                    if decoded_entry.get('code') == 'error' and 
>>>>>>>>> 'data' in decoded_entry:
>>>>>>>>> -                    decoded_entry['data'] = 
>>>>>>>>> decoded_entry['data'].decode()
>>>>>>>>> +                    decoded_entry['data'] = 
>>>>>>>>> metadataTranscoder.decode_metadata(decoded_entry['data'])
>>>>>>>>>                    entry = decoded_entry
>>>>>>>>>                if skip_info:
>>>>>>>>>                    if 'code' in entry and entry['code'] == 
>>>>>>>>> 'info':
>>>>>>>>>                        continue
>>>>>>>>>                for key in p4KeysContainingNonUtf8Chars():
>>>>>>>>>                    if key in entry:
>>>>>>>>> -                    entry[key] = 
>>>>>>>>> metadata_stream_to_writable_bytes(entry[key])
>>>>>>>>> +                    entry[key] = 
>>>>>>>>> metadataTranscoder.metadata_stream_to_writable_bytes(entry[key])
>>>>>>>>>                if cb is not None:
>>>>>>>>>                    cb(entry)
>>>>>>>>>                else:
>>>>>>>>> @@ -1718,7 +1742,7 @@ class P4UserMap:
>>>>>>>>>                # python2 or python3. To support
>>>>>>>>>                # git-p4.metadataDecodingStrategy=fallback, 
>>>>>>>>> self.users dict values
>>>>>>>>>                # are always bytes, ready to be written to git.
>>>>>>>>> -            emailbytes = 
>>>>>>>>> metadata_stream_to_writable_bytes(output["Email"])
>>>>>>>>> +            emailbytes = 
>>>>>>>>> metadataTranscoder.metadata_stream_to_writable_bytes(output["Email"]) 
>>>>>>>>>
>>>>>>>>>                self.users[output["User"]] = output["FullName"] 
>>>>>>>>> + b" <" + emailbytes + b">"
>>>>>>>>>                self.emails[output["Email"]] = output["User"]
>>>>>>>>>    @@ -1730,12 +1754,12 @@ class P4UserMap:
>>>>>>>>>                    fullname = mapUser[0][1]
>>>>>>>>>                    email = mapUser[0][2]
>>>>>>>>>                    fulluser = fullname + " <" + email + ">"
>>>>>>>>> -                self.users[user] = 
>>>>>>>>> metadata_stream_to_writable_bytes(fulluser)
>>>>>>>>> +                self.users[user] = 
>>>>>>>>> metadataTranscoder.metadata_stream_to_writable_bytes(fulluser)
>>>>>>>>>                    self.emails[email] = user
>>>>>>>>>              s = b''
>>>>>>>>>            for (key, val) in self.users.items():
>>>>>>>>> -            keybytes = metadata_stream_to_writable_bytes(key)
>>>>>>>>> +            keybytes = 
>>>>>>>>> metadataTranscoder.metadata_stream_to_writable_bytes(key)
>>>>>>>>>                s += b"%s\t%s\n" % (keybytes.expandtabs(1), 
>>>>>>>>> val.expandtabs(1))
>>>>>>>>>              open(self.getUserCacheFilename(), 'wb').write(s)
>>>>>>>>> @@ -3349,7 +3373,7 @@ class P4Sync(Command, P4UserMap):
>>>>>>>>>            if userid in self.users:
>>>>>>>>>                return self.users[userid]
>>>>>>>>>            else:
>>>>>>>>> -            userid_bytes = 
>>>>>>>>> metadata_stream_to_writable_bytes(userid)
>>>>>>>>> +            userid_bytes = 
>>>>>>>>> metadataTranscoder.metadata_stream_to_writable_bytes(userid)
>>>>>>>>>                return b"%s <a@b>" % userid_bytes
>>>>>>>>>          def streamTag(self, gitStream, labelName, 
>>>>>>>>> labelDetails, commit, epoch):
>>>>>>>>> @@ -4561,6 +4585,7 @@ commands = {
>>>>>>>>>        "unshelve": P4Unshelve,
>>>>>>>>>    }
>>>>>>>>>    +metadataTranscoder = 
>>>>>>>>> MetadataTranscoder(defaultMetadataDecodingStrategy, 
>>>>>>>>> defaultFallbackMetadataEncoding)
>>>>>>>>>      def main():
>>>>>>>>>        if len(sys.argv[1:]) == 0:
>>>>>>>>> diff --git a/t/meson.build b/t/meson.build
>>>>>>>>> index a59da26be3f..656424fdff3 100644
>>>>>>>>> --- a/t/meson.build
>>>>>>>>> +++ b/t/meson.build
>>>>>>>>> @@ -1090,6 +1090,7 @@ integration_tests = [
>>>>>>>>>      't9834-git-p4-file-dir-bug.sh',
>>>>>>>>>      't9835-git-p4-metadata-encoding-python2.sh',
>>>>>>>>>      't9836-git-p4-metadata-encoding-python3.sh',
>>>>>>>>> +  't9837-git-p4-error-encoding.sh',
>>>>>>>>>      't9850-shell.sh',
>>>>>>>>>      't9901-git-web--browse.sh',
>>>>>>>>>      't9902-completion.sh',
>>>>>>>>> diff --git a/t/t9837-git-p4-error-encoding.sh 
>>>>>>>>> b/t/t9837-git-p4-error- encoding.sh
>>>>>>>>> new file mode 100755
>>>>>>>>> index 00000000000..1ea774afb1b
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/t/t9837-git-p4-error-encoding.sh
>>>>>>>>> @@ -0,0 +1,53 @@
>>>>>>>>> +#!/bin/sh
>>>>>>>>> +
>>>>>>>>> +test_description='git p4 error encoding
>>>>>>>>> +
>>>>>>>>> +This test checks that the import process handles inconsistent 
>>>>>>>>> text
>>>>>>>>> +encoding in p4 error messages without failing'
>>>>>>>>> +
>>>>>>>>> +. ./lib-git-p4.sh
>>>>>>>>> +
>>>>>>>>> +###############################
>>>>>>>>> +## SECTION REPEATED IN t9835 ##
>>>>>>>>> +###############################
>>>>>>>>> +
>>>>>>>>> +# These tests require Perforce with non-unicode setup.
>>>>>>>>> +out=$(2>&1 P4CHARSET=utf8 p4 client -o)
>>>>>>>>> +if test $? -eq 0
>>>>>>>>> +then
>>>>>>>>> +    skip_all="skipping git p4 error encoding tests; Perforce 
>>>>>>>>> is setup with unicode"
>>>>>>>>> +    test_done
>>>>>>>>> +fi
>>>>>>>>> +
>>>>>>>>> +# These tests are specific to Python 3. Write a custom script 
>>>>>>>>> that executes
>>>>>>>>> +# git-p4 directly with the Python 3 interpreter to ensure 
>>>>>>>>> that we use that
>>>>>>>>> +# version even if Git was compiled with Python 2.
>>>>>>>>> +python_target_binary=$(which python3)
>>>>>>>>> +if test -n "$python_target_binary"
>>>>>>>>> +then
>>>>>>>>> +    mkdir temp_python
>>>>>>>>> +    PATH="$(pwd)/temp_python:$PATH"
>>>>>>>>> +    export PATH
>>>>>>>>> +
>>>>>>>>> +    write_script temp_python/git-p4-python3 <<-EOF
>>>>>>>>> +    exec "$python_target_binary" "$(git --exec-path)/git-p4" 
>>>>>>>>> "\$@"
>>>>>>>>> +    EOF
>>>>>>>>> +fi
>>>>>>>>> +
>>>>>>>>> +git p4-python3 >err
>>>>>>>>> +if ! grep 'valid commands' err
>>>>>>>>> +then
>>>>>>>>> +    skip_all="skipping python3 git p4 tests; python3 not 
>>>>>>>>> available"
>>>>>>>>> +    test_done
>>>>>>>>> +fi
>>>>>>>>> +
>>>>>>>>> +test_expect_success 'start p4d' '
>>>>>>>>> +    start_p4d
>>>>>>>>> +'
>>>>>>>>> +
>>>>>>>>> +test_expect_success 'see if Perforce error with characters 
>>>>>>>>> not convertable to utf-8 will be processed correctly' '
>>>>>>>>> +    test_when_finished cleanup_git &&
>>>>>>>>> +    $python_target_binary 
>>>>>>>>> "$TEST_DIRECTORY"/t9837/git-p4-error- python3.py 
>>>>>>>>> "$TEST_DIRECTORY"
>>>>>>>>> +'
>>>>>>>>> +
>>>>>>>>> +test_done
>>>>>>>>> diff --git a/t/t9837/git-p4-error-python3.py 
>>>>>>>>> b/t/t9837/git-p4-error- python3.py
>>>>>>>>> new file mode 100644
>>>>>>>>> index 00000000000..fb65aee386e
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/t/t9837/git-p4-error-python3.py
>>>>>>>>> @@ -0,0 +1,15 @@
>>>>>>>>> +import os
>>>>>>>>> +import sys
>>>>>>>>> +from  importlib.machinery import SourceFileLoader
>>>>>>>>> +
>>>>>>>>> +def main():
>>>>>>>>> +    if len(sys.argv[1:]) != 1:
>>>>>>>>> +        print("Expected test directory name")
>>>>>>>>> +
>>>>>>>>> +    gitp4_path = sys.argv[1] + "/../git-p4.py"
>>>>>>>>> +    gitp4 = SourceFileLoader("gitp4", gitp4_path).load_module()
>>>>>>>>> +    gitp4.p4CmdList(["edit", b'\xFEfile'])
>>>>>>>>> +
>>>>>>>>> +if __name__ == '__main__':
>>>>>>>>> +    main()
>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e