Merge System::Process.make_env_block into System::Env (win32) (#16605)

Environment variable keys are case-insensitive on Windows. This is currently emulated by using a `Hash` with upcased keys to key/value pairs.

This patch simplifies this with case-insensitive string comparisons and consolidates the two methods into a single one in the `System::Env` module.

The `System::Env.make_env_block` method is now close to the `System::Env.make_envp` method for UNIX targets.

Author: ysbaddaden

src/crystal/system/win32/env.cr

@@ -81,15 +81,40 @@ module Crystal::System::Env
     end
   end
 
-  # Used internally to create an input for `CreateProcess` `lpEnvironment`.
-  def self.make_env_block(env : Enumerable({String, String}))
+  # Used internally to create an input for `CreateProcessW` `lpEnvironment`.
+  def self.make_env_block(env, clear_env)
+    # If neither clearing nor adding anything, use the default behavior of
+    # inheriting everything.
+    return Pointer(UInt16).null if !env && !clear_env
+
     # NOTE: the entire string contains embedded null bytes so we can't use
     # `System.to_wstr` here
     String.build do |io|
-      env.each do |(key, value)|
-        check_valid_key(key)
-        io << key.check_no_null_byte("key") << '=' << value.check_no_null_byte("value") << '\0'
+      unless clear_env
+        each do |key, value|
+          # skip override
+          next if env.try(&.any? { |k, _| k.compare(key, case_insensitive: true) == 0 })
+
+          io << key.check_no_null_byte("key")
+          io << '='
+          io << value.check_no_null_byte("value")
+          io << '\0'
+        end
       end
+
+      env.try(&.each do |key, value|
+        check_valid_key(key)
+
+        # skip deletion
+        next if value.nil?
+
+        io << key.check_no_null_byte("key")
+        io << '='
+        io << value.check_no_null_byte("value")
+        io << '\0'
+      end)
+
+      # terminate the block
       io << '\0'
     end.to_utf16.to_unsafe
   end

src/crystal/system/win32/process.cr

@@ -302,7 +302,7 @@ struct Crystal::System::Process
 
     if LibC.CreateProcessW(
          nil, System.to_wstr(prepared_args), nil, nil, true, LibC::CREATE_SUSPENDED | LibC::CREATE_UNICODE_ENVIRONMENT,
-         make_env_block(env, clear_env), chdir.try { |str| System.to_wstr(str) } || Pointer(UInt16).null,
+         Env.make_env_block(env, clear_env), chdir.try { |str| System.to_wstr(str) } || Pointer(UInt16).null,
          pointerof(startup_info), pointerof(process_info)
        ) == 0
       error = WinError.value
@@ -457,29 +457,6 @@ struct Crystal::System::Process
   def self.chroot(path)
     raise NotImplementedError.new("Process.chroot")
   end
-
-  protected def self.make_env_block(env, clear_env : Bool) : UInt16*
-    # If neither clearing nor adding anything, use the default behavior of inheriting everything.
-    return Pointer(UInt16).null if !env && !clear_env
-
-    # Emulate case-insensitive behavior using a Hash like {"KEY" => {"kEy", "value"}, ...}
-    final_env = {} of String => {String, String}
-    unless clear_env
-      Crystal::System::Env.each do |key, val|
-        final_env[key.upcase] = {key, val}
-      end
-    end
-    env.try &.each do |(key, val)|
-      if val
-        # Note: in the case of overriding, the last "case-spelling" of the key wins.
-        final_env[key.upcase] = {key, val}
-      else
-        final_env.delete key.upcase
-      end
-    end
-    # The "values" we're passing are actually key-value pairs.
-    Crystal::System::Env.make_env_block(final_env.each_value)
-  end
 end
 
 private def close_handle(handle)