Fix TemporaryFileSwap regression where file_path could not be Path

[EliahKagan]

As noted in #1770 (comment), in 9e86053 (#1770) I accidentally introduced a regression in git.index.util.TemporaryFileSwap where the file_path parameter could no longer be a Path, even though it was (and remained) annotated in a way compatible with that.

Passing a Path object instead of str failed with TypeError, with a message like this (with WindowsPath instead of PosixPath on Windows, but otherwise the same):

TypeError: unsupported operand type(s) for +: 'PosixPath' and 'str'

Although much code that results in TemporaryFileSwap being used is under test, which would reveal some kinds of regressions, TemporaryFileSwap itself did not have any tests. It is public, so even if a Path were never passed to it in GitPython's own use of it, applications that use GitPython may be making direct use of it and are justified in assuming it accepts both a str and a str-based Path, as annotated.

This pull request:

• Adds a general test of TemporaryFileSwap. It does not test every code path that is supposed to be supported, such as code paths involving the file not existing in the first place, or the temporary file being deleted before it would be swapped back (those are deliberately covered in the code and may thus be valuable to test, and perhaps I, or someone else, could add tests for that later). However, I parameterized the test by the type of the file_path argument passed to TemporaryFileSwap, and the test shows both that it works when the type is str and that it fails, as described above, when the type is Path.
• Fixes the bug, and also changes how mkstemp is called to make it less unidiomatic. (However, it is still opening the file to reserve its name, and then immediately closing it, so this does not address the original misgivings noted in #1770 (review). See my comment in that review thread.)

There are some further details, which I'd consider less essential but potentially of value, in the commit messages.

[Byron]

Thanks a lot for the fix and its discovery in the fist place!

Of course one would expect that each standard library item that is handling items in the file system uses PathLike to not enforce an encoding on the provided input, but maybe some time in the future that will all be fixed in Python itself.

[EliahKagan]

Of course one would expect that each standard library item that is handling items in the file system uses PathLike to not enforce an encoding on the provided input, but maybe some time in the future that will all be fixed in Python itself.

Somewhat related to this topic, I've recently posted #1374 (comment). If my understanding as expresed there is mistaken or otherwise confused, I'd be pleased to make corrections.

Specifically in regard to PathLike, I don't fully understand why PathLike would avoid enforcing an encoding, or why an encoding would have to be enforced when using str. Part of my confusion is that, in the context of GitPython, there are two things PathLike could refer to.

• I think you mean the PathLike this PR concerned, which is git.types.PathLike and is defined (though using an older notation for backward compatibility) as str | os.PathLike[str].
• The other possibility is os.PathLike itself, which covers any type that provides a __fspath__ method that can be called with no arguments (besides self) and returns a str or bytes object. In annotations, when it is guaranteed to return str, os.PathLike[str] can be used, and when it is guaranteed to return bytes, os.PathLike[bytes] can be used.

(str | os.PathLike[str] is not redundant, because str, like bytes, does not itself have a __fspath__ method.)

My understanding of the current recommendation in Python for filesystem operations is to use str-based types for paths whenever possible, because this is more portable and avoids assuming any encoding. I infer this from these official tips taken together with some specific points about paths such as the note in the pathlib.Path docs that "Calling bytes is only recommended under Unix. Under Windows, the unicode form is the canonical representation of filesystem paths."

Both str itself and types that type-check as os.PathLike[str], such as a Path[str], are str-based. As far as I am aware, there are no encoding differences between what str and Path[str] store and express.

On Windows, there is occasionally the further issue that a "Unicode string" in the Windows API, though intended always to be valid UTF-16LE--which Python converts to and from str behind the scenes, I believe in practice performing a conversion to and from UTF-8 at least in CPython, but possibly using a UTF-32 encoding under some circumstances--can contain any sequence of 16-bit wchart_t values (wchar_t is 2 bytes on Windows, unlike the usual 4), including invalid surrogate pairs. Then conversion to UTF-8 may fail or produce something incorrect.

I think this ties in with some of what was discussed there.

One thing I am unsure about is if git can be reliably be made to disregard Windows's active code page settings and output UTF-8. I expect this to be relevant whether git is run with its standard streams treated as text or as binary data, since with text the encoding might not be what is expected; and with binary data the encoding, once the data are decoded, which they have to be at least some of the time, might not be what is expected.

Another thing I am unsure of, which represents a bigger gap in my understanding, is how much setting LC_ALL and related environment variables to C is guaranteed to achieve for git, even on POSIX systems. Can data that cannot be represented in ASCII be mangled in this scenario? Certainly this makes the messages parseable, in the sense of preventing translated strings from being used, but I am unsure about its effects on filenames, branch and tag names, etc. I know that some systems have added a non-equivalent C.UTF-8 locale, while others have not.

When I first saw some code like this, which I think is is the sort of thing you were alluding to there--which is at issue in #1631 and, based on testing on a zh-CN localized Windows Server 2022 virtual machine, seems to be at issue in #1362--my first thought was that this could be solved using the usual approach of both (a) using str for everything that is conceptually text, everywhere, and (b) passing encoding="utf-8" (as in #1748) to every function that is being called in a way that may entail the encoding or decoding of text and that accepts an encoding argument.

But because I'm not sure this can always in practice be done for interacting with git subprocesses, I am getting the idea that fixing things may be much more involved than that. (Perhaps #641 is related to this. We don't have to use the codecs module for much anymore, but that seems to be the idea, though I think more code would have to be changed.)

[Byron]

Thanks for reminding me that PathLike is a GitPython invention which boils down to str (it seems). I always hoped PathLike would be something that doesn't boil down to some Python controlled encoding, but instead to an unspecified OS-encoding.

(More about that to follow in a future paragraph)

On Windows, there is occasionally the further issue that a "Unicode string" in the Windows API, though intended always to be valid UTF-16LE--which Python converts to and from str behind the scenes, I believe in practice performing a conversion to and from UTF-8 at least in CPython, but possibly using a UTF-32 encoding under some circumstances--can contain any sequence of 16-bit wchart_t values (wchar_t is 2 bytes on Windows, unlike the usual 4), including invalid surrogate pairs. Then conversion to UTF-8 may fail or produce something incorrect.

It's amazing that you are aware of this as I only learned it recently when digging into the details of the Rust OsString type, which led to the very curious finding of the WTF encoding that Rust uses specifically on Windows to allow fastest-possible conversions to String if possible.

Another thing I am unsure of, which represents a bigger gap in my understanding, is how much setting LC_ALL and related environment variables to C is guaranteed to achieve for git, even on POSIX systems. Can data that cannot be represented in ASCII be mangled in this scenario? Certainly this makes the messages parseable, in the sense of preventing translated strings from being used, but I am unsure about its effects on filenames, branch and tag names, etc. I know that some systems have added a non-equivalent C.UTF-8 locale, while others have not.

For all I know, that variable is used to control the git CLI interface language, as I never tried to alter how git handles encodings of data. As far as I know, this is pre-determined everywhere, with the addition that paths are treated specifically on Unix (a bunch of bytes, no encoding), MacOS (deal with precomposed unicode) and Windows (convert paths to UTF8, and ignore failures(!!)).

When I first saw some code like this, which I think is is the sort of thing you were alluding to there--which is at issue in #1631 and, based on testing on a zh-CN localized Windows Server 2022 virtual machine, seems to be at issue in #1362--my first thought was that this could be solved using the usual approach of both (a) using str for everything that is conceptually text, everywhere, and (b) passing encoding="utf-8" (as in #1748) to every function that is being called in a way that may entail the encoding or decoding of text and that accepts an encoding argument.

Using str everywhere would certainly be nice (at least if the encoding is forced/assured to be UTF-8), and maybe at least for handling paths it would be correct most of the time, and more often than now. For all I know, str has an encoding though, the python interpreter global encoding, which makes it hard to use in practice. Maybe this changed, or maybe there are details, and all these won't matter as long as GitPython can control the encoding.

Practically, GitPython still won't work correctly though.

What gitoxide does is the following:

• If the encoding is unknown or dynamic (like with commit messages), use BString/bstr, which makes the string easily printable and processable if it is indeed ascii/UTF-8, but won't enforce any encoding nonetheless. It's a bunch of bytes.
• When dealing with filesystem paths, standard Path-types are used. When going from operating system paths to Git, these will have to be converted to BString, which is always fine on Unix, with mild conversion on MacOS and with major, fallible conversion on Windows. It's fallible only because of illformed UTF-8 which possibly still exists, but admittedly gitoxide will just panic as it is implemented now (which is probably better than ignoring the issue like git seems to do).
• The above conversion will be performed in reverse as needed to get back from git-paths to OS paths.

For all I can gather, the best, most unassuming way to deal with paths in GitPython would be to use bytes, which is probably terrible to handle. At some point, one will have to assume some encoding to work with these, and in fact git does assume ASCII, which is why it's so fine with UTF-8 as well.

That was interesting to me to write all this down here, and I hope it makes enough sense to you to help with mapping any/some/all of it into the Python world. My conclusion would be that we'd want bstr in Python.