Fixes to reversal of environment updates

master_changes.md

@@ -64,6 +64,12 @@ users)
 ## Clean
 
 ## Env
+  * [BUG] Fix reverting of environment variables, principally on Windows [#5935 @dra27 fix #5838]
+  * [BUG] Fix splitting environment variables [#5935 @dra27]
+  * [BUG] When opam creates an empty variable then appends/prepends a value, ensure no additional separator is added [#5935 @dra27 - fix #5925]
+  * [BUG] Fix `x-env-path-rewrite` splitting of values when reverting [#5935 @dra27 - fix #5838]
+  * [BUG] Rework the logic of := and =: so that an empty entry is correctly preserved on multiple updates [#5935 @dra27 - fix #5926]
+  * [BUG] Fix incorrect reverting of `=+` and `=:` [#5935 @dra27 - fix #5926]
 
 ## Opamfile
 
@@ -77,7 +83,6 @@ users)
   * Reset the "jobs" config variable when upgrading from opam 2.1 to 2.2, instead of 2.0 to 2.1 [#5904 @kit-ty-kate - fix #5816]
 
 ## Sandbox
-
 ## VCS
 
 ## Build
@@ -118,8 +123,15 @@ users)
   * tree: add a test for packages that have variables in their transitive dependencies [#5919 @rjbou]
   * tree: add test for `opam tree pkg --with-test --no-switch` [#5919 @rjbou]
   * Update opam root version test with root version bump [#5904 @rjbou]
+  * env tests: use `sort` to increase stability of the `opam env` output [#5935 @dra27 @rjbou]
+  * env.win32: add mixed slashes test [#5935 @dra27]
+  * env.win32: add test for environment revert not working correctly for Unix-like variables on Windows [#5935 @dra27]
+  * env.win32: add regression test for reverting additions to PATH-like variables [#5935 @dra27]
+  * env tests: add regression test for append/prepend operators to empty environment variables [#5925, #5935 @dra27]
+  * env.win32: add regression test for handling the empty entry in PATH-like variables [#5926, #5935 @dra27]
 
 ### Engine
+  * Add `sort` command [#5935 @dra27]
 
 ## Github Actions
 
@@ -140,3 +152,4 @@ users)
 ## opam-format
 
 ## opam-core
+  * `OpamStd.String`: add `split_quoted` that preserves quoted separator [#5935 @dra27]

src/core/opamStd.ml

@@ -710,6 +710,26 @@ module OpamString = struct
       |_ -> []
     in List.rev (aux acc0 tokens)
 
+  let split_quoted path sep =
+    let length = String.length path in
+    let rec f acc index current last normal =
+      if (index : int) = length then
+        let current = current ^ String.sub path last (index - last) in
+        List.rev (if current <> "" then current::acc else acc)
+      else
+      let c = path.[index]
+      and next = succ index in
+      if (c : char) = sep && normal || c = '"' then
+        let current = current ^ String.sub path last (index - last) in
+        if c = '"' then
+          f acc next current next (not normal)
+        else
+        let acc = if current = "" then acc else current::acc in
+        f acc next "" next true
+      else
+        f acc next current last normal in
+    f [] 0 "" 0 true
+
   let fold_left f acc s =
     let acc = ref acc in
     for i = 0 to String.length s - 1 do acc := f !acc s.[i] done;
@@ -877,24 +897,8 @@ module OpamSys = struct
   let path_sep = if Sys.win32 then ';' else ':'
 
   let split_path_variable ?(clean=true) =
-    if Sys.win32 then fun path ->
-      let length = String.length path in
-      let rec f acc index current last normal =
-        if index = length then
-          let current = current ^ String.sub path last (index - last) in
-          List.rev (if current <> "" then current::acc else acc)
-        else let c = path.[index]
-          and next = succ index in
-          if c = ';' && normal || c = '"' then
-            let current = current ^ String.sub path last (index - last) in
-            if c = '"' then
-              f acc next current next (not normal)
-            else
-            let acc = if current = "" then acc else current::acc in
-            f acc next "" next true
-          else
-            f acc next current last normal in
-      f [] 0 "" 0 true
+    if Sys.win32 then
+      fun path -> OpamString.split_quoted path ';'
     else fun path ->
       let split = if clean then OpamString.split else OpamString.split_delim in
       split path path_sep

src/core/opamStd.mli

@@ -314,6 +314,12 @@ module String : sig
       contiguous delimiters) *)
   val split_delim: string -> char -> string list
 
+  (** Splits a variable at the given character, but allowing double-quote
+      characters to protect the delimiter.
+
+      [split_quoted "foo\";\"bar;baz" ';' = ["foo;bar"; "baz"]] *)
+  val split_quoted: string -> char -> string list
+
   val fold_left: ('a -> char -> 'a) -> 'a -> string -> 'a
 
   val is_hex: string -> bool