This expands the CI test matrix in the main testing workflow totest on both Ubuntu and Windows, instead of just Ubuntu.It does not attempt to merge in the Cygwin workflow at this time,which may or may not end up being useful to do in the future.The new Windows test jobs all fail currently: the runs fail as aresult of various tests being consistently unable to pass but notyet being marked xfail (or skip). It should be feasible to markthese xfail with informative messages, but this commit doesn'tinclude that.

8 of the tests that fail on native Windows systems fail due toIndexFile.from_tree being broken on Windows, causinggitpython-developers#1630.This commit marks those tests as xfail. This is part, though notall, of the changes to get CI test jobs for native Windows thatare passing, to guard against new regressions and to allow the codeand tests to be gradually fixed (see discussion ingitpython-developers#1654).When fixing the bug, this commit can be reverted.

This other GitCommandError on Windows is not related toIndexFile.from_tree whose 8 related failing tests were marked xfailin the preceding commit.Also, test_clone_command_injection should not be confused withtest_clone_from_command_injection, which passes on all platforms.The problem here appears to be that, on Windows, the path of thedirectory GitPython is intended to clone to -- when the possiblesecurity vulnerability this test checks for is absent -- is notvalid. This suggests the bug may only be in the test and that thecode under test may be working on Windows. But the test does notestablish that, for which it would need to test with a payloadclearly capable of creating the file unexpected_path refers to whenrun on its own.This doesn't currently seem to be reported as a bug, but somegeneral context about the implementation can be examined ingitpython-developers#1518where it was introduced, andgitpython-developers#1531 where it was modified.

Everything attempted in the test case method is actually workingand passes, but when the tearDown method calls shutil.rmtree, itfails with "Access is denied" and raises PermissionError. Thereason and exception are accordingly noted in the xfail decoration.While adding a pytest import to apply the pytest xfail mark, I alsoimproved grouping/sorting of other imports in the test_diff module.

The test fails when attempting to evaluate the expression:    sm.move(new_path).nameas part of the assertion:    assert sm.move(new_path).name == new_pathBut it is the call to sm.move that fails, not the assertion itself.The condition is never evaluated, because the subexpression raisesPermissionError. Details are in the xfail reason string.This test_submodule.TestSubmodule.test_rename method should not beconfused with test_config.TestBase.test_rename, which passes on allplatforms.On CI this has XPASS status on Python 3.7 and possibly some otherversions. This should be investigated and, if possible, the xfailmarkings should be made precise. (When working on this changebefore a rebase, I had thought only 3.7 xpassed, and that the XPASSwas only on CI, never locally. However, I am unable to reproducethat or find CI logs of it, so some of these observations may havebeen mistaken and/or or specific to a narrow environment.)

As noted, the second of the config._included_paths() assertionsfails, which is in the "Ensure that config is included if path ismatching git_dir" sub-case.It is returning 0 on Windows. THe GitConfigParser._has_includesfunction returns the expression:    self._merge_includes and len(self._included_paths())Since _merge_includes is a bool, it appears the first branch of the"and" is True, but then _included_paths returns an empty list.

In modules soon to be modified, so if subsequent commits are laterreverted, these import tweaks are not automatically undone.

The test mostly works on Windows, but it fails because the tmp_filepath is expected to appear in remote_url with backslash separators,but (forward) slashes appear instead.

The tests of unsafe options are among those introduced originallyingitpython-developers#1521. They are regression tests forgitpython-developers#1515 (CVE-2022-24439).The unsafe options tests are paired: a test for the usual, defaultbehavior of forbidding the option, and a test for the behavior whenthe option is explicitly allowed. In each such pair, both tests usea payload that is intended to produce the side effect of a file ofa specific name being created in a temporary directory.All the tests work on Unix-like systems. On Windows, the tests ofthe *allowed* cases are broken, and this commit marks them xfail.However, this has implications for the tests of the default, securebehavior, because until the "allowed" versions work on Windows, itwill be unclear if either are using a payload that is effective andthat corresponds to the way its effect is examined.What *seems* to happen is this: The "\" characters in the path aretreated as shell escape characters rather than literally, with theeffect of disappearing in most paths since most letters lackspecial meaning when escaped. Also, "touch" is not a native Windowscommand, and the "touch" command provided by Git for Windows islinked against MSYS2 libraries, causing it to map (some?)occurrences of ":" in filenames to a separate code point in thePrivate Use Area of the Basic Multilingual Plane. The result is apath with no directory separators or drive letter. It denotes afile of an unintended name in the current directory, which is neverthe intended location. The current directory depends on GitPythonimplementation details, but at present it's the top-level directoryof the rw_repo working tree. A new unstaged file, named like"C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can beobserved there (this is how "git status" will format the name).Fortunately, this and all related tests are working on other OSes,and the affected code under test does not appear highly dependenton OS. So the fix is *probably* fully working on Windows as well.

Along with the version and platform information that is alreadyshown. This is to make debugging easier.

Only on native Windows systems (since this is for debugging WSL).The Windows CI runners have WSL itself installed but no WSL systemsinstalled. So some of these commands may fail if they call thebash.exe in the System32 directory, but this should still beilluminating. Results after a WSL system is set up will likely bemore important, but doing this first allows for comparison.The reason PATH directories are no longer listed is to decreasenoise.

This makes three of the four hook-related tests pass instead offailing, and changes the way the fourth fails.GitPython uses bash.exe to run hooks that are (or appear to be)shell scripts. On many Windows systems, this is the bash.exe in thesystem32 directory, which delegates to bash in a WSL system if atleast one such system is installed (for the current user), andgives an error otherwise. It may be a bug that GitPython ends upusing this bash.exe when WSL is installed but no WSL systems exist,since that is actually a fairly common situation. One place thathappened was on the GitHub Actions runners used for Windows jobs.Those runners have WSL available, and are capable of running WSL 1systems (not currently WSL 2 systems), but no WSL systems wereactually installed.This commit fixes that cause of failure, for all four tests ithappened in, by setting up a Debian WSL system on the test runner.(This increases the amount of time it takes Windows jobs to run,but that might be possible to improve on.) Three of those fourtests now pass, while the other fails for another reason.The newly passing tests are:- test/test_index.py::TestIndex::test_commit_msg_hook_fail- test/test_index.py::TestIndex::test_pre_commit_hook_fail- test/test_index.py::TestIndex::test_pre_commit_hook_successThe test that still fails, but differently, is:- test/test_index.py::TestIndex::test_commit_msg_hook_successI had previously found that test to fail on a local WSL 2 system,and attempted to add a suitable xfail marking in881456b (gitpython-developers#1679).But the condition I wrote there *appears* to have a bug related tothe different orders in which subproces.Popen and shutil.which findexecutables, causing it not always to detect when the WSL-relatedbash.exe is the one the Popen call in git.index.fun.run_commit_hookwill use.

881456b (gitpython-developers#1679) expanded the use of shutil.which in test_index.pyto attempt to mark test_commit_msg_hook_success xfail when bash.exeis a WSL bash wrapper in System32 (because that test currentlyis not passing when the hook is run via bash in a WSL system, whichthe WSL bash.exe wraps). But this was not reliable, due tosignificant differences between shell and non-shell search behaviorfor executable commands on Windows. As the new docstring notes,lookup due to Popen generally checks System32 before going throughdirectories in PATH, as this is how CreateProcessW behaves.-https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw-python/cpython#91558 (comment)The technique I had used in881456b also had the shortcoming ofassuming that a bash.exe in System32 was the WSL wrapper. But sucha file may exist on some systems without WSL, and be a bashinterpreter unrelated to WSL that may be able to run hooks.In addition, one common situation, which was the case on CI beforea step to install a WSL distribution was added, is that WSL ispresent but no WSL distributions are installed. In that situationbash.exe is found in System32, but it can't be used to run anyhooks, because there's no actual system with a bash in it to wrap.This was not covered before. Unlike some conditions that preventa WSL system from being used, such as resource exhaustion blockingit from being started, the absence of a WSL system should probablynot fail the pytest run, for the same reason as we are trying notto have the complete *absence* of bash.exe fail the pytest run.Both conditions should be xfail. Fortunately, the error messagewhen no distribution exists is distinctive and can be checked for.There is probably no correct and reasonable way to check LBYL-stylewhich executable file bash.exe resolves to by using shutil.which,due to shutil.which and subprocess.Popen's differing seach ordersand other subtleties. So this adds code to do it EAFP-style usingsubprocess.run (which itself uses Popen, so giving the sameCreateProcessW behavior). It tries to run a command with bash.exewhose output pretty reliably shows if the system is WSL or not.We may fail to get to the point of running that command at all, ifbash.exe is not usable, in which case the failure's details tell usif bash.exe is absent (xfail), present as the WSL wrapper with no WSLsystems (xfail), or has some other error (not xfail, so the user canbecome aware of and proably fix the problem). If we do get to thatpoint, then a file that is almost always present on both WSL 1 andWSL 2 systems and almost always absent on any other system ischecked for, to distinguish whether the working bash shell operatesin a WSL system, or outside any such system as e.g. Git Bash does.Seehttps://superuser.com/a/1749811 on various techniques forchecking for WSL, including the /proc/sys/fs/binfmt_misc/WSLInteroptechnique used here (which seems overall may be the most reliable).Although the Windows CI runners have Git Bash, and this is even thebash.exe that appears first in PATH (giving rise to the problemwith shutil.which detailed above), it would be a bit awkward totest the behavior when Git Bash is actually used to run hooks onCI, because of how Popen selects the System32 bash.exe first,whether or not any WSL distribution is present. However, localtesting shows that when Git Bash's bash.exe is selected, all fourhook tests in the module pass, both before and after this change,and furthermore that bash.exe is correctly detected as "native",continuing to avoid an erroneous xfail mark in that case.

- Make the design of the enum simpler.- Move some informaton to the check method's docstring.- Call the method once instead of in each decoration.Doing the check only once makes things a little faster, but themore important reason is that the checks can become stale veryquickly in some situations. This is unlikely to be an issue on CI,but locally WSL may fail (or not fail) to start up when bash.exeis used in a check, then not fail (or fail) when actaully neededin the test, if the reason it fails is due to resource usage, suchas most RAM being used on the system.Although this seems like a reason to do the check multiple times,doing it multiple times in the decorations still does it ahead ofwhen any of the tests is actually run. In contrast, with the changehere, we may get outdated results by the time the tests run, butthe xfail effects of the check are always consistent with eachother and are no longer written in a way that suggests they aremore current than they really are.

This reverts "Install WSL system on CI for hook tests",commit9717b8d.Assuming both cases are found to be working, the removed step willeither be brought back or replaced with a very similar step thatinstalls a different distribution.

This undoes "Temporarily don't install WSL system to test xfail"(cabb572). It keeps Debian as the distribution. (Although the DebianWSL system installs pretty fast already, it may still make sense totry switching to Alpine in the future. But that might need to waituntilVampire/setup-wsl#50 is fixed.)This also removes most of the commands in the WSL debugging step,since the related machinery in test_index.py (_WinBashStatus) seemsto be in okay shape, condenses the smaller number of commands thatare retained there, and makes much less extensive reduction in thegeneral version and platform information commands as well. This isto make workflow output easier to read, understand, and navigate.

This causes full "failure" output to be printed when a test markedxfail unexpectedly passes, and for the test run to be consideredfailing as a result.The immediate purpose of this change is to facilitate efficientidentification of recently introduced wrong or overbroad xfailmarkings.This behavior may eventually become the pytest default (seegitpython-developers#1728and references therein), and this could be retained even after thecurrent xpassing tests are investigated, to facilitate timelydetection of tests marked xfail of code that is newly working.(Individual tests decorated `@pytest.mark.xfail` can still beallowed to unexpectedly pass without it being treated like a testfailure, by passing strict=False explicitly.)

The xfail mark was added in42a3d74, where the XPASS status on 3.7was observed but not included in the condition. It turns out thisseems to XPASS on a much wider range of versions: all but 3.12.Currently 3.12.0 is the latest stable version and no testing hasbeen done with alpha for 3.13. Most likely whatever causes thistest to fail on 3.12 would also apply to 3.13, but because I don'tunderstand the cause, I don't want to guess that, and instead wrotethe new condition to expect failure only on 3.12.* versions.

For its immediate goal, this facilitated identifying the XPASSstatus of TestSubmodule.test_rename and verifying that the revisedxfail condition eliminated it.In the test output, XPASS is written as a failure, with strictxfail behavior noted. Contributors less familiar with marking testsxfail may mistake this to mean some behavior of the code under testwas broken. So if strict_xfail is to be enabled, it might be bestto do that in a pull request devoted to it, and maybe even to addto the docs, about how to recognize and handle newly xpassingtests.This reverts commit0f8cd4c.

This further improves the condition that was corrected in82c361e.Testing on Python 3.13.0 alpha 2 shows the same failure as on 3.12(that I'm at least right now consistently unable to produce on anylower versions).In addition, on both versions of CPython on Windows, the failureseems to consistently resolve if two gc.collect() are placed justabove the code that calls sm.move(). A single call is consistentlyinsufficient. I haven't included any such calls in this commit,since the focus here is on fixing xfail markings, and becuse such achange may benefit from being evaluated separately and may meritfurther accompanying changes. But that this behavior is exhibitedon both 3.12 and the 3.13 alpha further supports removing theupper bound on the xfail marking.

This changes _WinBashStatus from an enum to a sum type so that,rather than having a global _WinBashStatus.ERROR_WHILE_CHECKINGobject whose error_or_process attribute may be absent and, ifpresent, carries an object set on the most recent call to check(),we get a _WinBashStatus.ErrorWhileChecking instance that carriesan error_or_process attribute specific to the call.Ordinarily this would not make a difference, other than that theglobal attribute usage was confusing in the code, because typically_WinBashStatus.check() is called only once. However, whendebugging, having the old attribute on the same object be clobberedis undesirable.This adds the sumtypes library as a test dependency, to allow theenum to be rewritten as a sum type without loss of clarity. This isa development-only dependency; the main dependencies are unchanged.

This removes text=True from the subprocess.run call, changing strliterals to bytes where appropriate and (less importantly) using"%r" instead of "%s" in log messages so it's clear that printingthe repr of a bytes object is, at least for now, intentional.The reason for this is that the encoding of error messages producedby running the WSL bash.exe, when it attempts but fails to use aWSL system, varies based on what error occurred. When no systemsare installed, the output can be decoded as UTF-8. When an errorfrom "deeper down" is reported, at least for Bash/Service errors,the output is encoded in UTF-16LE, and attempting to decode it asUTF-8 interleaves lots of null characters in the best case. With abytes object, loss of information is avoided, and it is clear oninspection that the output requires decoding.The most common case of such an error is *probably*:    Insufficient system resources exist to complete the requested service.    Error code: Bash/Service/CreateInstance/CreateVm/HCS/0x800705aaHowever, that is tricky to produce intentionally on some systems.To produce a test error, "wsl --shutdown" can be run repeatedlywhile a _WinBashStatus.check() call is in progress. This produces:    The virtual machine or container was forcefully exited.    Error code: Bash/Service/0x80370107Assuming the output always includes the text "Error code:", it maybe feasible to reliably detect which cases it is. This couldespecially improve the log message. But for now that is not done.In adddition to changing from text to binary mode, this commit alsotemporarily removes the setup-wsl step from the CI workflow again,to verify on CI that the text-to-binary change doesn't break theWslNoDistro case. Manual testing shows the other cases still work.

This affects the test suite only. It improves _WinBashStatus.When bash.exe is the WSL wrapper, and it reports an error thatprevents bash in a WSL system from ever actually being run, themessage may be encoded in a narrow or multibyte encoding, or mayinstead be in Windows's native UTF-16LE encoding. This was partlyhandled before, by assuming the message indicating the absence ofany installed WSL distribuions could be interpreted as UTF-8, butmatching against bytes and not actually decoding it or other errormessages. That presented a few problems, which are rememedied here:- It turns out that this "Windows Subsystem for Linux has no  installed distributions" message actually can be in UTF-16LE too.  This happens when it is part of a message that also reports a  textual error code like:  Bash/Service/CreateInstance/GetDefaultDistro/WSL_E_DEFAULT_DISTRO_NOT_FOUND  Therefore, narrow-encoding matching was not sufficient.- Logged messages were hard to understand, because they were  printing the reprs of bytes objects, which sometimes were  UTF-16LE and thus had many "\x00" interspersed in them.- The cases were not broken down elegantly. The ErrorWhileChecking  case could hold a CompletedProcess, a CalledProcessError, or an  OSError. This is now replaced with a WinError case for the rare  scenario where CreateProcessW fails in a completely unexpected  way, and a CheckError case for when the exit status or output of  bash.exe indicates an error other than one we want to handle.  CheckError has two attributes: `process` for the CompletedProcess  (CalledProcessError is no longer used, and CompletedProcess has  `returncode` and other attributes that provide the same info),  and `message` for the decoded output.

Instead of searching for an English sentence from the errormessage, this searches for the specific aka.ms short URL itcontains in all languages, which points to a page about downloadingdistributions for WSL from the Microsoft Store (Windows Store).The URL is controlled and hosted by Microsoft and intended as apermalink. Thus this approach should be more reliable even forEnglish, as the specific wording of the message might be rephrasedover time, so this may have a lower risk of false negatives.Because it only makes sense to give a link for obtaining adistribution in an error message when no distribution is installedalready, the risk of false positives should also be fairly low.The URL is included in the output telling the user they have nodistributions installed even on systems like Windows Server wherethe Microsoft Store is not itself available, and even in situationswhere WSL has successfully downloaded and unpacked a distributionbut could not run it the first time because it is using WSL 2 butthe necessary virtualization is unavailable. Because these areincluded among the situations where the hook tests should be markedxfail, it is suitable for the purpose at hand.Furthermore, while this only works if we correctly guess if theencoding is UTF-16LE or not, it should be *fairly* robust againstincorrect decoding of the message where a single-byte encoding istreated as UTF-8, or UTF-8 is treated as a single-byte encoding, orone single-byte encoding is treated as another (i.e., wrong ANSIcode page). But some related encoding subtleties remain to beresolved.Although reliability is likely improved even for English, tofaciliate debugging the WslNoDistro case now carries analogous`process` and `message` arguments to the `CheckError` case.On some Windows systems, wsl.exe exists in System32 but bash.exedoes not. This is not inherently a problem for run_commit_hook asit currently stands, which is just trying to use whatever bash.exeis available. (It is not clear that WSL should be preferred.) Thisis currently so, on some systems, where WSL is not really set up;wsl.exe can be used to do so. This is a situation where no WSLdistributions are present, but because the bash.exe WSL wrapper isalso absent, it is not a problem.However, I had wrongly claimed in the _WinBashStatus.checkdocstring that bash.exe is in System32 whenever WSL is present. Sothis also revises the docstring to say only that this is usually so(plus some further revision for clarity).

Windows does not have a direct analogue of LANG=C or LC_ALL=C. Someprograms give them special treatment, but they do not affect theway localized behavior of the Windows API operates. In particular,the bash.exe WSL wrapper, as well as wsl.exe and wslconfig.exe, donot produce their own localized messages (for errors notoriginating in a running distribution) when they are set. Windowsalso provides significant localization through localized versionsof Windows, so changing language settings in Windows, evensystem-wide, does not always produce the same effect that many ormost Windows users who use a particular language would experience.Various encodings may appear when bash.exe is WSL-related but givesits own error message. Such a message is often in UTF-16LE, whichis what Windows uses internally, and preserves any localization.That is the more well behaved scenario and already was detected;this commit moves, but does not change, the code for that.The situation where it is not UTF-16LE was previously handled bytreating it as UTF-8. Because the default strict error treatmentwas used, this would error out in test discovery in some localizedsetups, preventing all tests in test_index from running, includingthe majority of them that are not related to hooks. This fixes thatby doing better detection that should decode the mesages correctlymost of the time, that should in practice decode them well enoughto tell (by the aka.ms URL) if the message is complaining aboutthere being no installed distribution all(?) of the time, and thatshould avoid breaking unrelated tests even if that can't be done.An English non-UTF-16LE message appears on GitHub Actions CI whenno distribution is installed. Testing of this situation on otherlanguages was performed in a virtual machine on a developmentmachine.That the message is output in a narrow character set some of thetime when bash.exe produces it appears to be a limitation of thebash.exe wrapper. In particular, with a zh-CN version of Windows(and with the language not changed to anything else), a localizedmessage in Simplified Chinese was correctly printed when runningwsl.exe, but running bash.exe produced literal "?" characters inplace of Chinese characters (it was not a display or font issue,and they were "?" and not Unicode replacement characters). Thechange here does not overcome that; the literal "?" characters willbe included. But "https://aka.ms/wslstore" is still present if itis an error about not having any distributions, so the correctstatus is still inferred.For more information on code pages in Windows, see:https://serverfault.com/a/836221The following alternatives to all or part of the approach takenhere were considered but, at least for now, not done, because theywould not clearly be simpler or more correct:- Abandoning this "run bash.exe and see what it shows us" approach  altogether and instead reimplementing the rules CreateProcessW  uses, to find if the bash.exe the system finds is the one in  System32, and then, if so, checking the metadata in that  executable to determine if it's the WSL wrapper. I believe that  would be even more complex to do correctly than it seems; the  behavior noted in the WinBashStatus docstring and recent commit  messages is not the whole story. The documented search order for  CreateProcessW seems not to be followed in some circumstances.  One is that the Windows Store version of Python seems always to  forgo the usual System32 search that precedes seaching directories  in PATH. It looks like there may also be some subtleties in which  directories 32-bit builds search in.- Using chardet. Although the chardet library is excellent, it is  not clear that the code needed to bridge this highly specific use  case to it would be simpler than the code currently in use. Some  of the work might still need to be done by other means; when I  tested it out for this, this did not detect the UTF-16LE messages  as such for English. (They are often also valid UTF-8, because  interleaving null characters is, while strange, permitted.)- Also calling wsl.exe and/or wslconfig.exe. It's still necessary  to call bash.exe, because it may not be the WSL bash, even on a  system with WSL fully set up. Furthermore, those tools' output  seem to vary in some complex ways, too. Using only one subprocess  for the detection seemed the simplest. Even using "wsl --list"  would introduce significant additional logic. Sometimes its  output is a list of distributions, sometimes it is an error  message, and if WSL is not set up it may be a help message.- Using the Windows API to check for WSL systems.https://learn.microsoft.com/en-us/windows/win32/api/wslapi/ does  not currently include functions to list registered distributions.- Attempting to get wsl.exe to produce an English message using  Windows API techniques like those used in Locale Emulator. This  would be complicated, somewhat unintuitive and error prone to do  in Python, and I am not sure how well it would work on a system  that does not have an English language pack installed.- Checking on disk for WSL distributions in the places they are  most often expected to be. This would intertwine WinBashStatus  with deep details of how WSL actually operates, and this seems  like the sort of thing that is likely to change in the future.However, there may be a more straightforward way to do this (thatis at least as correct and that remains transparent to debug).Especially if the WinBashStatus class remains in test_index forlong (rather than just being used to aid in debugging existing testfalures and possible subsequent design decisions for making commithooks work more robustly on Windows in GitPython), then this maybe worth revisiting.Thus it is *not* with the intention of treating WinBashStatus as a"stable" part of the test suite that it is renamed from_WinBashStatus. This is instead done because:- Like HOOKS_SHEBANG, it makes sense to import it when figuring out  how the tests work or debugging them.- When debugging, it is intended that it be imported to call  check() and examine the resulting `process` and `message`  information, at least in the CheckError case.

So the Windows test jobs can run the three out of four hook teststhat are able to pass with WSL bash, and so that we are able toobserve the expected failure on the one that still fails.This undoes the removal of the setup-wsl step ind5ed266, now thatthe test suite's bash.exe status checking logic, used for xfailmarks (and also usable in debugging), is internationalized. The CIstep was omitted during some of this process, to make sureWinBashStatus would still detect the specific situation on theGitHub Actions runner that occurs when no WSL distro is available.(This commit thus has much of the same relationship tod5ed266as2875ffa had tocabb572.)

- Factor out the code to get the Windows ACP to a helper function,  and comment to explain why it doesn't use locale.getencoding.- Remove nonessential material in the WinBashStatus.check docstring  and reword the rest for clarity.- Drop reStructuredText notation in the WinBashStatus docstrings,  because in this case it seems to be making them harder to read in  the code (we are not generating Sphinx documentation for tests.)- Revise the comments on specific steps in WinBashStatus._decode  for accuracy and clarity.

This removes the parenthesized examples from the per-step commentsin the WinBashStatus._decode helper.

Byron left a comment