Re: [PATCH] git-p4: handle non-unicode characters in p4 cl

[Andrew Oakley <andrew@adoakley.name>]

On Wed, 03 Feb 2021 16:59:59 +0000
"Feiyang via GitGitGadget" <gitgitgadget@gmail.com> wrote:
> From: Feiynag Xue <fxue@roku.com>
> 
> P4 allows non-unicode characters in changelist description body,
> so git-p4 needs to be character encoding aware when reading p4 cl
> 
> This change adds 2 config options, one specifies encoding,
> the other specifies erro handling upon unrecognized character.
> Those configs  apply when it reads p4 description text, mostly
> from commands "p4 describe" and "p4 changes".

...

>     It seems to make sense to default to replace so that it gets rid
> of non-unicode chars while trying to retain information. However, i am
>     uncertain on if we have use cases where it relies on the
>     stop-on-non-unicode behavior. (Hypothetically say an automation
> that's expected to return error on non-unicode char in order to stop
> them from propagating further?)

I suspect these options will be insufficient for real repositories.

There are two ways a perforce server is configured:
- unicode mode where the metadata is valid UTF-8, and you can request
  conversion to different character sets
- not in unicode mode where the metadata can be pretty much any random
  bytes (but not '\0'), and the encoding is not stored anywhere

There isn't any way to recover the encoding information from perforce,
and it's likely that a server that's not in unicode mode will end up
with both UTF-8 commits, and commits that contain other things (which
we have no way to work out what they are).

Until recently git-p4 was written in python2 and it just moved the
bytes from perforce into git without trying to interpret them in any
way.  This has the advantage that the git repository will accurately
reflect what was in perforce, even if it's complete garbage.

The other useful option I can think of would be to attempt to decode
the data as UTF-8, but fall back to some other encoding if the data
isn't valid (probably Windows-1252, but a config option would make
sense here).