Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Append to preexisting MSYS env var even if ill-formed #1580

Merged
merged 3 commits into from
Sep 6, 2024

Commits on Sep 5, 2024

  1. Configuration menu
    Copy the full SHA
    fbd4908 View commit details
    Browse the repository at this point in the history

Commits on Sep 6, 2024

  1. fix: Append to preexisting MSYS env var even if ill-formed

    The value of an environment variable as obtained by the facilities
    in `std::env` is not always well-formed Unicode. Specifically, on
    Windows the values of environment variables, like paths, are
    natively UTF-16LE strings except that unpaired surrogate code
    points can also occur. An `&OsStr` on Windows may accordingly not
    quite be UTF-8.
    
    When the `MSYS` variable is absent, we treat this the same as when
    it is present but empty. However, as described in GitoxideLabs#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. This commit 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.
    EliahKagan committed Sep 6, 2024
    Configuration menu
    Copy the full SHA
    8dc5d7a View commit details
    Browse the repository at this point in the history
  2. Remove configure_command_msys* tests at least for now

    Because they are not really testing the right thing, due to the
    distinction between inherited and `.env()`-specified environment.
    
    See GitoxideLabs#1578.
    
    While for `configure_command_clears_external_config` this slightly
    diminishes but largely leaves intact the value of the test, the
    `configure_command_msys` and `configure_command_msys_extends` tests
    are really verifying almost nothing, and the latter (for this
    reason) cannot pass.
    
    A variant of it for testing the preceding fix to append to
    arbitrary environment variable values would likewise not be able to
    pass. So that is not added at this time.
    
    Because the `configure_command_clears_external_config` test remains
    useful, it is not removed.
    EliahKagan committed Sep 6, 2024
    Configuration menu
    Copy the full SHA
    c38d9b9 View commit details
    Browse the repository at this point in the history