Append to preexisting MSYS env var even if ill-formed

[EliahKagan]

Fixes #1574

When the MSYS variable is absent, we treat this the same as when it is present but empty. However, as described in #1574, an MSYS variable that is present but whose value contains an unpaired surrogate would also be replaced entirely, rather than appending to its old value. This changes that, to instead append, retaining whatever was there even if it was ill-formed Unicode.

An alternative change could be to panic when the old value is ill-formed Unicode. The change made here allows and appends to the old value, rather than panicking or keeping and documenting the previous behavior of discarding the old value, because the appended sequence winsymlinks:nativestrict is effective at causing fixture scripts to attempt to create actual symlinks even if the preceding code point is an unpaired Unicode high surrogate.

---

To avoid confusion related to #1358, I tested without setting GIX_TEST_IGNORE_ARCHIVES, relying on how some fixtures must still run because archive generation is suppressed for them in .gitignore files. That failure would occur in connection with the MSYS variable even when run this way was observed in #1443 and #1444, but I have also verified that.

This PR adds and then removes tests, which were unsuitable for the reasons noted in #1578, and which I removed rather than attempting to fix due to the concerns in #1577 combined with there being little doubt, in this case, that the code using OsString::push works as it seems. If those abandoned tests are not wanted in the history, I have no objection to rebasing to remove those commits, which I would be pleased to do on request.

This gist contains transcripts of the manual verification done to decide on and check over this approach. The scripts run in a normal environment, as well as in an environment in which the old value of MSYS has a trailing unpaired Unicode high surrogate. Failures occur due to the absence of MSYS when the code that sets it is removed, which verifies both that this code does not break setting it, and that the appended space character and winsymlinks option takes effect properly even if it immediately follows an unpaired high surrogate that "tries" to combine with the next code point. Testing with a panic! temporarily added, by the same technique as #1574, confirms that we really are concatenating.

All full test runs have an unrelated failure of jj_realistic_needs_to_be_more_clever locally, which is due to #1575 and can therefore be disregarded in the transcripts of local test runs here. Between tests, I removed generated fixture script output by using the technique discussed in #1435 of running gix clean -xd -m '*generated*' -e, which is shown in the gist transcripts. I did it that way rather than by running the stronger command gix clean -xde so the unpaired surrogate in the MSYS environment variable would not stall the build due to rust-lang/libz-sys#215. The command used is strong enough, but to doubly ensure insufficient cleaning would not cause misinterpreted results, I ran the tests with the expected failure modification last of all the full runs, so that if left-over fixture output was going to cause tests to wrongly pass, then it would happen with the tests I was verifying should fail.

[Byron]

Thanks a lot!

I should definitely have used push and var_os from the beginning, and will try to remember that (at least by now) OsString has some ways of altering it. In my head, it's still so that OsStr is known to be able to do nothing, so it's hard to work with.

[EliahKagan]

that (at least by now) OsString has some ways of altering it

It occurs to me that I do not always check when features are introduced. I am hoping that if this feature came in later than the project MSRV, then one of the clippy checks on CI would catch it and fail. (If that is not the case, please let me know.)