Re: [PATCH] add git-p4.fallbackEncoding config variable, to prevent git-p4 from crashing on non UTF-8 changeset descriptions

From: Torsten Bögershausen <tboegi@web.de>
Date: Apr 24, 2021, 08:14 UTC
Message-ID: <20210424081447.uxabqbxc54k6yxrg@tb-raspi4>
In-Reply-To: <CAKu1iLXPi4zc-5-RtZo3UBwTQ1GqshXjLEZKT=WvtvB0aiuUJA@mail.gmail.com>

(Adding some of the p4 and Windows experts in cc)

On Fri, Apr 23, 2021 at 12:08:17PM -0700, Tzadik Vanderhoof wrote:

> To clarify....

Good. This is good information, and the important stuff could go into the commit message. And because the commit as such should be self-contained (as much as possible). Giving an overview about the problem.

>
> The new config variable I am introducing addresses an issue that only
> occurs on Windows.  This is because the behavior of the "p4" command
> differs on Windows vs Linux around Unicode in changeset descriptions.

What does Windows mean in this context ? Is p4 a "console application" ? In this case it may be possible to use CHCP to change to a different code page ?

Show 9 quoted lines

>
> I don't have the source code for "p4", but I'm guessing it's written
> in C, and that this difference in behavior is simply a result of the
> fact that there is no defined standard of how "char *argv[]" in "main"
> should deal with non-ASCII characters being passed in from the command
> line.
>
> As a result, "git p4 clone" on Linux is not affected by this "p4"
> behavior.

Is it ? What happens if yoy have a p4 depot that was feed from Windows in CP-1252 and is now accessed from a Linux machine ? Doesthe Linux box face the same problems ?

> Since my tests assume the Windows behavior, they fail when
> run on Linux.  For this reason, I added code to my tests to skip them
> on Linux.

That makes sense, but what is the "Windows behavior" more in detail ? My understanding is that when you press e.g. the key 'Ä' on the keybaord, it will give a different byte sequence once that 'Ä' is transferred across the wire (to the p4 server). This is dependent on what Linux calls a locale, and all major Linux installations use UTF-8 these days by default. But that was not always the case, since in old days they used ISO-8851-1 or something else, usable for your contry/region.

So most Windows "console applications" are not run under UTF-8, but it may be possible that "CHCP 65000" (or so) works - more testing needed.

Show 5 quoted lines

>
> On a related note, I don't think there are any CI environments on
> github for git that are (a) on Windows, and (b) have Python and (c)
> have Perforce, so I don't think my tests are actually running on
> github CI.  I'm not sure how that can be addressed.

That are 3 different questions - (a) Yes, git is compiled under Windows, both gcc and MSVC (correct me if that is wrong) (b) Yes, we have python on the different CI. Github actions has python, I use it every day. (c) There are tests run for p4, but it seems if they are only run under Linux.

It would be nice if your test can pass under Linux, why are they failing ?

If I dig here: <https://github.com/git/git/runs/2420889332?check_suite_focus=true>

We can see that the t98 test are run, and are passing. Just to pick one: [15:28:22] t9804-git-p4-label.sh .............................. ok 3969

Thanks for working on this. It would be good to have a v5 version of the patch with some more informations, like above, and may be :how is the p4 server configured ? (Unicode or not ?)