{"thread":{"id":"58946","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","startedAt":"2022-12-14T06:11:14Z","lastAt":"2022-12-22T08:26:34Z","messageCount":6,"participants":["Tzadik Vanderhoof","Junio C Hamano","Tao Klerks"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469003","messageId":"CAKu1iLWvd+PVOK405Q+SNDyyYnhbi=LtovZvWw55y6eQqoRpaA@mail.gmail.com","threadId":"58946","inReplyTo":null,"subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Tzadik Vanderhoof","fromEmail":"tzadik.vanderhoof@gmail.com","sentAt":"2022-12-14T06:10:56Z","receivedAt":"2022-12-14T06:11:14Z","isPatch":true,"sender":{"key":"tzadik.vanderhoof@gmail.com","avatar":null},"body":"From: Tao Klerks <tao@klerks.biz>\n> Perforce has a file type \"utf8\" which represents a text file with\n> explicit BOM. utf8-encoded files *without* BOM are stored as\n> regular file type \"text\". The \"utf8\" file type behaves like text\n> in all but one important way: it is stored, internally, without\n> the leading 3 BOM bytes.\n>\n> git-p4 has historically imported utf8-with-BOM files (files stored,\n> in Perforce, as type \"utf8\") the same way as regular text files -\n> losing the BOM in the process.\n>\n> Under most circumstances this issue has little functional impact,\n> as most systems consider the BOM to be optional and redundant, but\n> this *is* a correctness failure, and can have lead to practical\n> issues for example when BOMs are explicitly included in test files,\n> for example in a file encoding test suite.\n>\n> Fix the handling of utf8-with-BOM files when importing changes from\n> p4 to git, and introduce a test that checks it is working correctly\n\nI don't really understand the problem that this patch addresses, but I\nfeel it was incorrect to modify the behavior of git-p4 in this way.\nThis change has made git-p4 behave differently than the native\nPerforce tools.\n\nPerforce already has a \"variable\" (setting) to control this behavior.\nIf P4CHARSET is set to \"utf8-bom\", the BOM will be added to files in\nthe local workspace of type \"utf8\".  If P4CHARSET is set to \"utf8\",\nthe BOM will NOT be stored in the local workspace file, even if its\ntype is \"utf8\"!\n\nWhatever the problem was that motivated this change, it should have\nbeen solved using the Perforce variable P4CHARSET, not by modifying\nthe behavior of git-p4.  This new behavior has made it impossible for\nme to submit changes to files of type \"utf8\"!  Any attempt fails with\n\"patch does not apply\" and the erroneously added BOM is the cause.\n\nI propose rolling back the patch that introduced this behavior, and if\nthat is not possible, I would like to submit a patch that adds a new\nsetting in the \"git-p4\" section that will enable users to opt out of\nthis new behavior (for example a boolean setting \"git-p4.noUtf8Bom\").\n\n-- \nTzadik\n"},{"id":"469004","messageId":"xmqqv8mewfqc.fsf@gitster.g","threadId":"58946","inReplyTo":"CAKu1iLWvd+PVOK405Q+SNDyyYnhbi=LtovZvWw55y6eQqoRpaA@mail.gmail.com","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T08:29:15Z","receivedAt":"2022-12-14T08:29:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tzadik Vanderhoof <tzadik.vanderhoof@gmail.com> writes:\n\n> I propose rolling back the patch that introduced this behavior, and if\n> that is not possible, I would like to submit a patch that adds a new\n> setting in the \"git-p4\" section that will enable users to opt out of\n> this new behavior (for example a boolean setting \"git-p4.noUtf8Bom\").\n\nIf a change in the most recent broke P4 users who have happily been\nusing \"git p4\", then we definitely should do both.  Immediately\nrevert in preparation for Git 2.39.1 or Git 2.39.2, while working to\nreintroduce it as a separate option to allow the feature to add BOM\nautomatically to please users like Tao.\n\nBut for such a resolution, this report should have come while\nfbe5f6b8 (git-p4: preserve utf8 BOM when importing from p4 to git,\n2022-04-04) was still in flight, before it was merged for e4a4b315\n(Git 2.37, 2022-06-27).  We had three months to do so, and now it is\nway too late.\n\n\n\n"},{"id":"469025","messageId":"CAPMMpoi=6x5VbSh=Lkbi7WJKudGpQS2U_GnJk8GJi+ArJNp2EA@mail.gmail.com","threadId":"58946","inReplyTo":"CAKu1iLWvd+PVOK405Q+SNDyyYnhbi=LtovZvWw55y6eQqoRpaA@mail.gmail.com","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-12-14T18:24:04Z","receivedAt":"2022-12-14T18:25:39Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Wed, Dec 14, 2022 at 7:11 AM Tzadik Vanderhoof\n<tzadik.vanderhoof@gmail.com> wrote:\n>\n> From: Tao Klerks <tao@klerks.biz>\n> > Fix the handling of utf8-with-BOM files when importing changes from\n> > p4 to git, and introduce a test that checks it is working correctly\n>\n> I don't really understand the problem that this patch addresses, but I\n> feel it was incorrect to modify the behavior of git-p4 in this way.\n> This change has made git-p4 behave differently than the native\n> Perforce tools.\n\nHi Tzadik, first of all my apologies for the fact that these changes\nbroke your flow - it was clearly not the intent to cause any users to\nhave to change the way they do things (or, worse, make it impossible\nfor them to work, or cause BOM-presence inconsistencies in their\nrepos).\n\nWhen you say \"This change has made git-p4 behave differently than the\nnative Perforce tools\", I suspect there's a misunderstanding, but\nyou've clearly uncovered a git-p4 \"workflow bug\" that this change\nintroduced.\n\nThe git-p4 tooling can be and is used for a number of different things.\n\nIf I understand your usecase correctly, you keep a local git repo that\nyou work in, and others around you continue to use p4; you\n(presumably) don't share your git repo with any other users.\n\nUnder those single-user circumstances, it makes sense that you expect\nyour git repo to contain \"your workspace's understanding\" of the\nperforce repo/contents, rather than \"the actual contents of the repo\"\n- and in fact, if I understand correctly, the fact that your workspace\nhas a specific view on what's in the repo (does UTF-8 BOM stripping)\nmeans that the git-p4 \"submit\"/\"commit\" command finds unexpected\ndifferences between your git files and your p4 workspace files, when\ntrying to submit git changes into your p4 workspace.\n\nIn other use-cases where the git repo is shared between multiple\nusers, or where a perforce-to-git migration is facilitated by this\ntooling, it is critical that the result of importing a perforce\nrepo/branch into git be \"faithful\", retaining the actual contents of\nUTF-8-BOM files - including those first 3 bytes that are ignored by\nmany text-file-processing systems, but significant to others.\n\n>\n> Perforce already has a \"variable\" (setting) to control this behavior.\n> If P4CHARSET is set to \"utf8-bom\", the BOM will be added to files in\n> the local workspace of type \"utf8\".  If P4CHARSET is set to \"utf8\",\n> the BOM will NOT be stored in the local workspace file, even if its\n> type is \"utf8\"!\n\nThis behavior makes sense for workstation tooling like the \"p4\" tool,\nbut to the best of my knowledge it does not make sense for \"repo\ncloning\" tooling.\n\nUnless I'm misunderstanding something, git-p4 was working as you\nwanted before these changes, precisely *because* you had specified\nP4CHARSET=utf8 in your workspace (and you're the only one using your\ngit repo, or all others set P4CHARSET the same way and have the same\nBOMless perspective in their worktrees), so git-p4's buggy clone/sync\nbehavior (swallowing UTF-8 BOMs when importing repo history from\nperforce) matched your p4 workspace's intentionally configured\nbehavior.\n\nThere is a complicating factor here that may be clouding my\nunderstanding: Perforce has two server modes of operation, \"unicode\nenabled server\" and... non-unicode-enabled? Normal?\n\nThe idea that there are utf-8 files stored without BOM (and stored, in\nfact, as type \"text\"), and utf-8 files stored with BOM (as type\n\"utf8\") is a notion of the not-unicode-enabled Perforce servers. It\nmay be that no such distinction exists / can be stored in \"unicode\nenabled servers\" - I need to do some testing with such servers to\nbetter understand how this works. As far as I understand, you must be\nusing a \"unicode enabled server\" as the p4 docs say P4CHARSET must\nalways be unset / set to \"none\" on \"normal\" servers (and must always\nbe set, with \"unicode enabled servers\", even if implicitly to \"auto\").\n\n>\n> Whatever the problem was that motivated this change, it should have\n> been solved using the Perforce variable P4CHARSET, not by modifying\n> the behavior of git-p4.\n\nObviously if the conflict with workflows like yours had been\nunderstood, the change would not have been made as it was.\n\nHowever, I'm not sure that I understand your proposal here to solve a\nproblem \"using the Perforce variable P4CHARSET\". The problem was that,\non standard Perforce servers, when you add a UTF-8-BOM file, it gets\nstored with type \"utf8\". When someone else syncs that file, they get\nback what was put in - a UTF-8-BOM file. This is the way you generally\nexpect/hope a VCS will typically work. Conversely, if you add a UTF-8\nfile that does *not* have the BOM, it will be stored as type \"text\" -\nand someone syncing it will, again, get out what was put in. This is\nperforce client tooling working correctly/normally.\n\ngit-p4, on the other hand, does not use \"p4 sync\", it uses \"p4 print\",\nand with this command, the BOM is *not* included on \"utf8\" type files.\nThis is not dependent on P4CHARSET, it's just an aspect of the\ncontract of \"p4 print\" - it expects the recipient of text-derived\nfiles to parse the p4 \"type\" and  handle any encoding transformations\n(like adding a BOM) accordingly.\n\nAgain, I'm not attempting to defend the breakage - just outlining why\nI don't see how \"using the Perforce variable P4CHARSET\" would solve\nanything.\n\n> This new behavior has made it impossible for\n> me to submit changes to files of type \"utf8\"!  Any attempt fails with\n> \"patch does not apply\" and the erroneously added BOM is the cause.\n\nI will try to understand the \"unicode enabled server\" behavior today\nor tomorrow and see what options might make sense.\n\n> I propose rolling back the patch that introduced this behavior,\n\nJunio is the expert here and has noted it's a little late for that. I\nobviously defer to his expertise as to git's release and backout\nstrategy.\n\nI would like to have a go at understanding what the options are (how\nwe can get correct and functional behavior for all users), before\nproposing a specific course of action.\n\n> and if\n> that is not possible, I would like to submit a patch that adds a new\n> setting in the \"git-p4\" section that will enable users to opt out of\n> this new behavior (for example a boolean setting \"git-p4.noUtf8Bom\").\n\nI suspect that is the easiest tactical solution, but I'd be concerned\nthat this would require users like you (on such unicode-enabled\nservers, with the P4CHARSET=utf8 setting locally), to \"discover\" the\ncause of submission errors they get, and discover the setting that\nwould enable them to work around it. At least in the medium term, I\nwould prefer to find a way to have git-p4 intentionally respect the\n\"P4CHARSET\" setting (with a warning about repo faithfulness if you're\nlosing any data or causing potential repo inconsistency along the\nway).\n\nAs a workaround for you in the immediate term (and sorry, I feel\nsuper-dirty even writing this) the only option I see for \"git-p4\nsubmit\" to work would be for you to, when this happens, temporarily\nchange your P4CHARSET value to \"utf8-bom\" and re-sync the affected\nfiles in your workspace, before you \"git-p4 submit\". I have not tested\nthis, but given my limited understanding of your situation it seems\nlike this would work.\n\nIf you already have a different workaround please let us know what it is!\n\nBest regards,\nTao\n"},{"id":"469048","messageId":"xmqq1qp1wph5.fsf@gitster.g","threadId":"58946","inReplyTo":"CAPMMpoi=6x5VbSh=Lkbi7WJKudGpQS2U_GnJk8GJi+ArJNp2EA@mail.gmail.com","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T23:11:02Z","receivedAt":"2022-12-14T23:11:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tao Klerks <tao@klerks.biz> writes:\n\n> Again, I'm not attempting to defend the breakage - just outlining why\n> I don't see how \"using the Perforce variable P4CHARSET\" would solve\n> anything.\n>\n>> This new behavior has made it impossible for\n>> me to submit changes to files of type \"utf8\"!  Any attempt fails with\n>> \"patch does not apply\" and the erroneously added BOM is the cause.\n>\n> I will try to understand the \"unicode enabled server\" behavior today\n> or tomorrow and see what options might make sense.\n>\n>> I propose rolling back the patch that introduced this behavior,\n>\n> Junio is the expert here and has noted it's a little late for that. I\n> obviously defer to his expertise as to git's release and backout\n> strategy.\n>\n> I would like to have a go at understanding what the options are (how\n> we can get correct and functional behavior for all users), before\n> proposing a specific course of action.\n\nIt sounds like, if your conjecture turns out to be correct in that\nthose P4 users who interact unicode enabled servers would have\nP4CHARSET and others don't, we may not need an extra configuration\nbut pay attention to the P4CHARSET variable (or lack of it) and\nswitch the behaviour.\n\nThanks for a well reasoned response.\n"},{"id":"469264","messageId":"CAPMMpoiLYhjjK73Le1u71e-nzU1cf_dA=i7zYYz3t8+AujgbCA@mail.gmail.com","threadId":"58946","inReplyTo":"xmqq1qp1wph5.fsf@gitster.g","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-12-19T09:09:44Z","receivedAt":"2022-12-19T09:10:40Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Thu, Dec 15, 2022 at 12:11 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Tao Klerks <tao@klerks.biz> writes:\n>\n> > Again, I'm not attempting to defend the breakage - just outlining why\n> > I don't see how \"using the Perforce variable P4CHARSET\" would solve\n> > anything.\n> >\n> >> This new behavior has made it impossible for\n> >> me to submit changes to files of type \"utf8\"!  Any attempt fails with\n> >> \"patch does not apply\" and the erroneously added BOM is the cause.\n> >\n> > I will try to understand the \"unicode enabled server\" behavior today\n> > or tomorrow and see what options might make sense.\n> >\n> >> I propose rolling back the patch that introduced this behavior,\n> >\n> > Junio is the expert here and has noted it's a little late for that. I\n> > obviously defer to his expertise as to git's release and backout\n> > strategy.\n> >\n>\n> It sounds like, if your conjecture turns out to be correct in that\n> those P4 users who interact unicode enabled servers would have\n> P4CHARSET and others don't, we may not need an extra configuration\n> but pay attention to the P4CHARSET variable (or lack of it) and\n> switch the behaviour.\n\nYes, I suspect some sort of detection will be required. There appears\nto be no way to query the server for this \"unicode mode\" directly, but\nyou can force the client to try connecting in the \"wrong\" mode for the\nserver, and catch the corresponding error. Ugly, but effective.\n\n(the reason it's hard to just test for \"P4CHARSET\" is that there are\nseveral ways to set it, not just the environment, and there are\nmultiple versions of the setting, per-connection or global; setting\nthe global override and testing for failure is likely to be safer than\nattempting to understand/evaluate the hierarchy of settings)\n\n> > I would like to have a go at understanding what the options are (how\n> > we can get correct and functional behavior for all users), before\n> > proposing a specific course of action.\n\nI have finally managed to start testing with the \"unicode enabled\nserver\" behavior.\n\nSo far I've learned that:\n * Some of our tests around file content encoding handling do fail\nwith the server in this mode (not necessarily because we're doing the\nwrong thing, but because the server's behavior doesn't match our\nexpectations) these failures may correspond to bugs to be fixed, or\ntests to be adjusted to match appropriate expectations in this\n\"unicode enabled mode\"\n * Our tests around \"git p4 submit\" *don't* seem to fail, even on\nutf-8-bom files - so I have not yet reproduced Tzadik's issue\n\n(I keep placing \"unicode enabled server\" in quotes because I don't\nwant to give the impression that perforce in \"normal\" mode doesn't\nhandle unicode content - it absolutely does, but... differently.)\n\nI definitely need to keep testing around this to understand what the\nright thing to do for Tzadik (and others like him of course) might be.\n\nTzadik, could you provide any more detail about the failing situation?\nOne piece of info that might be particularly helpful is *what is the\nexact/full p4 FileType of the problem file?*\n\nThanks,\nTao\n"},{"id":"469451","messageId":"CAPMMpohHoPwxGTrwUi-WNvhU_+CnKP-rKPcDE-7qTUVBuRWA1g@mail.gmail.com","threadId":"58946","inReplyTo":"CAPMMpoiLYhjjK73Le1u71e-nzU1cf_dA=i7zYYz3t8+AujgbCA@mail.gmail.com","subject":"Re: [PATCH] git-p4: preserve utf8 BOM when importing from p4 to git","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-12-22T08:26:19Z","receivedAt":"2022-12-22T08:26:34Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Mon, Dec 19, 2022 at 10:09 AM Tao Klerks <tao@klerks.biz> wrote:\n>\n> I definitely need to keep testing around this to understand what the\n> right thing to do for Tzadik (and others like him of course) might be.\n>\n> Tzadik, could you provide any more detail about the failing situation?\n> One piece of info that might be particularly helpful is *what is the\n> exact/full p4 FileType of the problem file?*\n\nHi Tzadik, over the past few days I've been making progress towards\nunderstanding the behavior of unicode-enabled perforce servers (and p4\nclient tooling against such servers):\n* Perforce always stores/adds new files containing a utf8 bom as type\n\"utf8\", and always syncs them to the workspace with a BOM, regardless\nof the P4CHARSET env variable. Same goes for utf16 with bom being\nstored as type \"utf16\".\n* The only filetype that gets synced to the workspace differently\ndepending on P4CHARSET is the \"unicode\" type - a type exclusive to\n*specific files* on unicode-enabled servers\n* The P4CHARSET env var is used in three situations that I've\ndetected/understood so far:\n  1. It is used when \"p4 add\"ing new files - if the content of the\nfile is valid/parseable according to the P4CHARSET, and is not a\nbuilt-in more-specific type like utf8-with-bom (perforce type \"utf8\")\nor utf16-with-bom (perforce type \"utf16\"), then it gets parsed\naccording to that charset and stored as type \"unicode\". If the content\nis not parseable according to that charset, nor the more specific\ntypes, then it gets stored as \"text\" or \"binary\".\n  2. It is used when \"p4 sync\"ing files of type \"unicode\" to the\nworkspace. All other types are synced in a \"standard\" way, only type\n\"unicode\" is sensitive to the charset config during \"sync\".\n  3. It is used when \"p4 print\"ing files of type \"unicode\", \"utf8\",\nand \"utf16\" (unless P4COMMANDCHARSET overrides, or a \"-o\" output file\nis specified). This last behavior spells trouble for git-p4...\n* When a file is stored as type \"unicode\", the way it gets\nsynchronized to the workspace always depends on the P4CHARSET env\nvariable. When a \"with-bom\" charset value is configured, like\n\"utf8-bom\" or \"utf16\", the resulting files on disk / in the workspace\nend up indistinguishable from the corresponding perforce native\nwith-bom type \"utf8\" or \"utf16\".\n* If an output file (-o) is specified, \"p4 print\" behaves like \"p4\nsync\" - using P4CHARSET only for file type \"unicode\".\n* If no output file (-o) is specified, the output of \"p4 print\"\ndepends on the P4CHARSET env variable or the more-specific\nP4COMMANDCHARSET if specified, but with a number of provisos:\n** \"p4 print\" will refuse to print using a \"wide\" charset like\n\"utf16\"; it will fall back to utf8.\n** If you configure P4CHARSET (and/or P4COMMANDCHARSET) to a charset\nvalue with bom (\"utf8-bom\" specifically, I guess), you don't in fact\nget the BOM in the \"p4 print\" output.\n** If you configure P4CHARSET (and/or P4COMMANDCHARSET) to a charset\nvalue that cannot express the specific contents of a utf8, utf16 or\nunicode file, \"p4 print\" will fail.\n* The behavior of \"p4 print\" matters in git-p4 because this is the p4\ncommand used to import files; for \"utf16\" files a \"file-targeting\"\nform is used (-o), and for all other file types, the regular form.\n* One other \"weirdness\" in all this is that if you remove the BOM from\na \"utf8\" file and submit, this change is silently swallowed; your\nworkspace copy of the file remains without BOM, but if you force-sync\nthe file, the BOM magically reappears.\n\nThis is all further complicated by the fact that older Perforce server\n(and client?) versions behaved differently! (eg see\nhttps://stackoverflow.com/questions/38320814/bom-issue-in-unicode-perforce-server).\n\nSo far in my testing of git-p4 against a unicode server, I've\nreproduced two concrete \"problem workflows\" specific to this server\nconfiguration:\n\n1: If you have P4CHARSET configured to \"utf8-bom\"\n* \"git p4 clone\" imports your \"unicode\" files without BOM (because \"p4\nprint\" always omits the BOM)\n* \"p4 sync\" syncs your \"unicode\" files *with* BOM - because that's\nwhat P4CHARSET is telling it to do\n* \"git p4 submit\" fails to apply (some) git changes (to \"unicode\"\nfiles) because the git-workspace files do not have a BOM, and the p4\nworkspace files do.\n\n2: If you have P4CHARSET configured to a non-utf8 encoding like \"winansi\":\n* \"git p4 clone\" imports your \"utf8\" files as winansi-encoded (because\n\"p4 print\" respects the configured P4CHARSET), and adds a utf8 bom\n(because of the fix earlier this year)\n* \"p4 sync\" syncs your \"utf8\" files encoded as utf8-with-BOM - because\nthat's the only and correct behavior\n* Some of these files in the git repo may not open correctly in any\ngiven editor, if they have a utf8 bom but are not actually\nutf8-encoded (if they contain characters in the 128+ codepoint range)\n* \"git p4 submit\" fails to apply (some) git changes (to these \"utf8\"\nfiles) because these git-workspace files can have different bytes to\nthe utf8-encoded files in the workspace.\n\nThere seem to be two corresponding \"fundamental bugs\" in \"git-p4 clone\":\n* Files of type \"unicode\" are imported according to the\nclient-configured charset (with some inconsistency around wide\ncharsets like utf16) - correct behavior is hard to define, as perforce\ntreats these files as having no \"fundamental\" encoding, but current\nbehavior is clearly buggy. It's unclear whether properly/consistently\nrespecting the p4-client-configured encoding makes more sense (to\nsupport local-git-repo workflows transparently) by default, or whether\nit makes more to use \"utf8\" by default, to have a consistent and\ncorrect representation of the Perforce repository contents in git by\ndefault (and then the equivalent of P4CHARSET behavior in\nindividual/personal git repos would need to be implemented using git\nsmudge and clean filters, I assume, for \"git p4 submit\" to work\nagainst non-utf8-configured p4 workspaces).\n* If unicode mode is enabled and P4CHARSET is not set to \"utf8\" or\n\"utf8-bom\", files of perforce type \"utf8\" will be imported according\nto the client-configured charset instead of the actual/internal (and\np4-workspace-respected) encoding - and additionally, with \"my fix\",\nthey will still get a utf8 BOM prepended, which can result in corrupt\nfiles in the git repo.\n\nPerplexingly, most of the issues I've identified with unicode-mode\nperforce servers are not caused by my changes: the only *new* problem\nthis year is that, if P4CHARSET is set to something other than \"utf8\"\nor \"utf8-bom\", files of p4 type \"utf8\" will now not only be imported\ninto the git repo with the *wrong* encoding (prior behavior), but will\nadditionally have a utf8 BOM added, resulting in *corrupted* files\n(if/when the charset encoding diverges, eg if the content includes\ncharacters in the 128+ codepoint range for P4CHARSET=winansi).\n\nIt's worth noting there's also an existing workflow failure that\napplies equally to unicode-mode and regular servers, when users delete\nthe BOM from existing \"utf8\" files in their git repo and submit that\nchange into perforce:\n* \"git p4 clone\" imports your \"utf8\" files correctly, as utf8-with-BOM\n(in the case of a unicode-mode perforce server, assume \"P4CHARSET\" is\nset to \"utf8\" or \"utf8-bom\")\n* \"p4 sync\" syncs your \"utf8\" files encoded as utf8-with-BOM - because\nthat's the only and correct behavior\n* The user accidentally or intentionally removes the BOM as part of\nsome edits to the file in the git workspace\n* \"git p4 submit\" applies the change to the p4 workspace successfully,\nit is committed to perforce (but perforce ignores the BOM removal bit)\n* \"git p4 submit\" does its rebase of the git branch on top of the\nnow-submitted-and-reimported p4 changes, but the rebase fails because\nthe changes are different: in the p4 history there was no BOM-removal.\n\nTzadik, could you please confirm:\n* Your p4 client and server versions (\"p4 info\")\n* The perforce \"types\" of files you are concerned with (check with \"p4\nfiles <PATH>\" or in the p4v GUI)\n* Your OS\n* Your P4CHARSET value, if set explicitly to something other than \"auto\"\n\nI will keep exploring how this all works, and proposing fixes to\ngit-p4's vulnerable use of \"p4 print\". The correct outcome/behavior is\nclear (and achievable) for files of perforce type \"utf8\", but I'm not\nconfident as to what the best default behavior will be for type\n\"unicode\" - I imagine it will be to keep current client-specific\nlocal-workflow-supporting behavior, fixing it to support wide\nencodings and BOM-specifying encodings, and recommending that\nP4CHARSET be set to \"utf8\" explicitly for canonical git-p4 clone\noperations / for shared repos.\n\nI have not yet looked into filepath-handling, commit-message-handling,\nusername-handling etc on unicode-enabled servers, but I suspect\nsimilar issues will apply there: we'll have to figure out what the\ncorrect behavior would be for \"canonical\" git repos, whether it should\nbe any different for \"local-workflow-only\" git repos, and how either\ndesirable behavior compares to current behavior for various values of\nP4CHARSET. Everything probably already works correctly when\nP4CHARSET=utf8.\n\nFor the \"reappearing BOM\" problem/behavior of perforce, I'm also not\nsure what the right approach is, even manually. I've tried asking p4\nto reevaluate the \"type\" of the file upon edit, but can't find any way\nto get it to *auto*-detect the new file type. Given that I've never\nseen this issue described before, it doesn't feel hugely important\nthough. I will at least add a failing test case to cover this, when I\nget to it.\n\nFor now, based on the original problem statement/bug report, I cannot\nunderstand what specific problem is affecting Tzadik, and whether or\nhow it was caused by the 2022 utf8 bom-preserving fix to \"git-p4\nclone\".\n\nThanks,\nTao\n"}]}