Intepret environment values as paths

This is the likely intended behavior. Down the road we can have a way to have
literal values in these strings.

doc/catalog-format-spec.md

@@ -316,6 +316,10 @@ static, i.e. they cannot depend on the context.
    PATH.append = "${DISTRIB_ROOT}/usr/bin"
    ```
 
+   Path fragments in this table must use portable format, that is, '/' for path
+   separation. Alire will take care of using the native separator when setting
+   these variables.
+
    Predefined variables are provided by Alire and will be replaced in the
    value:
 
@@ -325,6 +329,9 @@ static, i.e. they cannot depend on the context.
      be the `msys2` installation directory (e.g.
      `C:\Users\user_name\.cache\alire\msys2`).
 
+   The escaping `"\$"` can be used to prevent the expansion of a
+   dollar-bracketed expression.
+
    Environment entries can use dynamic expressions:
 
    ```toml

src/alire/alire-environment-formatting.adb

@@ -1,5 +1,6 @@
 with Ada.Strings.Unbounded; use Ada.Strings.Unbounded;
 
+with Alire.OS_Lib;
 with Alire.Platforms.Current;
 
 package body Alire.Environment.Formatting is
@@ -81,6 +82,23 @@ package body Alire.Environment.Formatting is
          end if;
       end Replace;
 
+      ---------------
+      -- To_Native --
+      ---------------
+      --  Replace forward slashes with native slashes on Windows, unless they
+      --  are an escape sequence.
+      function To_Native (S : String) return String is
+      begin
+         case OS_Lib.Dir_Separator is
+            when '/' => return S;
+            when '\' => null;
+            when others => raise Unimplemented with
+                 "Unknown OS with dir separator: " & OS_Lib.Dir_Separator;
+         end case;
+
+         return AAA.Strings.Replace (S, "/", "" & OS_Lib.Dir_Separator);
+      end To_Native;
+
       Result : Unbounded_String := To_Unbounded_String (Value);
       From   : Natural := 1;
       To     : Natural;
@@ -107,7 +125,8 @@ package body Alire.Environment.Formatting is
          From := 1;
       end loop;
 
-      return To_String (Result);
+      --  For final usage, we use the native separator
+      return To_Native (+Result);
    end Format;
 
 end Alire.Environment.Formatting;

src/alire/alire-os_lib-subprocess.adb

@@ -10,8 +10,6 @@ with GNAT.OS_Lib;
 
 package body Alire.OS_Lib.Subprocess is
 
-   use AAA.Strings;
-
    function To_Argument_List
      (Args : AAA.Strings.Vector)
       return GNAT.OS_Lib.Argument_List_Access;

src/alire/alire-os_lib.ads

@@ -1,3 +1,7 @@
+private with AAA.Strings;
+
+with GNATCOLL.OS.Constants;
+
 package Alire.OS_Lib with Preelaborate is
 
    function "/" (L, R : String) return String;
@@ -20,4 +24,52 @@ package Alire.OS_Lib with Preelaborate is
    --  Return the location of an executable if found on PATH, or "" otherwise.
    --  On Windows, no need to append ".exe" as it will be found without it.
 
+   Forbidden_Dir_Separator : constant Character :=
+                               (case GNATCOLL.OS.Constants.Dir_Sep is
+                                   when '/' => '\',
+                                   when '\' => '/',
+                                   when others =>
+                                      raise Unimplemented
+                                        with "Unknown dir separator");
+
+   --  For things that may contain path fragments but are not proper paths
+
+   Dir_Separator : Character renames GNATCOLL.OS.Constants.Dir_Sep;
+
+   subtype Native_Path_Like is String
+     with Dynamic_Predicate =>
+       (for all Char of Native_Path_Like => Char /= Forbidden_Dir_Separator);
+
+   subtype Portable_Path_Like is String
+     with Dynamic_Predicate =>
+       (for all Char of Portable_Path_Like => Char /= '\');
+
+   function To_Portable (Path : Native_Path_Like) return Portable_Path_Like;
+
+   function To_Native (Path : Portable_Path_Like) return Native_Path_Like;
+
+private
+
+   use AAA.Strings;
+   use all type GNATCOLL.OS.OS_Type;
+
+   ----------------------
+   -- To_Portable_Like --
+   ----------------------
+
+   function To_Portable (Path : Native_Path_Like)
+                              return Portable_Path_Like
+   is (case GNATCOLL.OS.Constants.OS is
+          when MacOS | Unix => Path,
+          when Windows      => Replace (Path, "\", "/"));
+
+   --------------------
+   -- To_Native_Like --
+   --------------------
+
+   function To_Native (Path : Portable_Path_Like) return Native_Path_Like
+   is (case GNATCOLL.OS.Constants.OS is
+          when MacOS | Unix => Path,
+          when Windows      => Replace (String (Path), "/", "\"));
+
 end Alire.OS_Lib;

src/alire/alire-properties-environment.adb

@@ -71,6 +71,24 @@ package body Alire.Properties.Environment is
       use type Conditional.Properties;
       use TOML;
       Env : TOML_Value;
+
+      ----------------
+      -- Path_Check --
+      ----------------
+
+      procedure Path_Check (Var, S : String) is
+      begin
+         --  We expect something resembling a portable path, but we admit "\$"
+         --  as an escape sequence.
+         for I in S'Range loop
+            if S (I) = '\' and then (I = S'Last or else S (I + 1) /= '$') then
+               Raise_Checked_Error
+                 (Var & ": forbidden '\' character in environment path; "
+                  & "use '/' instead");
+            end if;
+         end loop;
+      end Path_Check;
+
    begin
       if From.Unwrap.Kind /= TOML_Table then
          From.Checked_Error
@@ -87,7 +105,7 @@ package body Alire.Properties.Environment is
          for Name of Env.Keys loop
             declare
                Var  : Variable;   -- The env. var. being parsed
-               Val  : TOML_Value; -- The env. var. value
+               Val  : TOML_Value; -- The env. var. action. value
             begin
                Var.Name := Name;
 
@@ -109,8 +127,10 @@ package body Alire.Properties.Environment is
                           Actions_Suggestion (Action_Image));
                end;
 
-               --  Value (already type checked in previous pop)
+               --  We consider values as possibly containing paths, so we check
+               --  that path separators are portable
 
+               Path_Check (+Name, Val.As_String);
                Var.Value := +Val.As_String;
 
                --  Pop entry to avoid upper "unexpected key" errors

src/alire/alire-properties-environment.ads

@@ -41,7 +41,7 @@ private
    type Variable is new Property with record
       Action : Actions;
       Name   : UString;
-      Value  : UString;
+      Value  : UString; -- Value with portable path separators
    end record;
 
 end Alire.Properties.Environment;

src/alire/alire-publish.adb

@@ -25,15 +25,14 @@ with Alire.TOML_Index;
 with Alire.TOML_Keys;
 with Alire.TOML_Load;
 with Alire.User_Pins.Maps;
+with Alire.Utils.Tools;
 with Alire.Utils.TTY;
 with Alire.Utils.User_Input.Query_Config;
 with Alire.VCSs.Git;
 with Alire.VFS;
 
 with CLIC.User_Input;
 
-with GNATCOLL.OS.Constants;
-
 with Semantic_Versioning;
 
 with TOML.File_IO;
@@ -675,8 +674,9 @@ package body Alire.Publish is
             & String'("--exclude=./alire")
             --  Exclude top-level alire folder, before applying prefix
 
-            --  exclude .git and the like, with workaround for macOS bsd tar
-            & (if GNATCOLL.OS.Constants.OS in GNATCOLL.OS.MacOS
+            --  exclude .git and the like, with workaround for bsdtar used by
+            --  macOS and Windows without MSYS2
+            & (if Utils.Tools.Is_BSD_Tar
                then Empty_Vector
                     & "--exclude=./.git"
                     & "--exclude=./.hg"

src/alire/alire-toml_adapters.adb

@@ -257,8 +257,8 @@ package body Alire.TOML_Adapters is
    ----------------------
 
    function Pop_Single_Table (Queue : Key_Queue;
-                              Value : out TOML.TOML_Value;
-                              Kind  : TOML.Any_Value_Kind) return String
+                              Value : out TOML.TOML_Value)
+                              return String
    is
       use TOML;
    begin
@@ -274,14 +274,28 @@ package body Alire.TOML_Adapters is
 
       Value := Queue.Value.Get (Queue.Value.Keys (1));
 
+      return Key : constant String := +Queue.Value.Keys (1) do
+         Queue.Value.Unset (Queue.Value.Keys (1));
+      end return;
+   end Pop_Single_Table;
+
+   ----------------------
+   -- Pop_Single_Table --
+   ----------------------
+
+   function Pop_Single_Table (Queue : Key_Queue;
+                              Value : out TOML.TOML_Value;
+                              Kind  : TOML.Any_Value_Kind) return String
+   is
+      use TOML;
+      Key : constant String := Queue.Pop_Single_Table (Value);
+   begin
       if Value.Kind /= Kind then
          Queue.Checked_Error ("expected a single entry of type "
                               & Kind'Img & ", but got a " & Value.Kind'Img);
       end if;
 
-      return Key : constant String := +Queue.Value.Keys (1) do
-         Queue.Value.Unset (Queue.Value.Keys (1));
-      end return;
+      return Key;
    end Pop_Single_Table;
 
    -----------------------

src/alire/alire-toml_adapters.ads

@@ -98,8 +98,16 @@ package Alire.TOML_Adapters with Preelaborate is
    --  intended use is to process keys beginning with "case(" in the table.
 
    function Pop_Single_Table (Queue : Key_Queue;
-                              Value : out TOML.TOML_Value;
-                              Kind  : TOML.Any_Value_Kind) return String;
+                              Value : out TOML.TOML_Value)
+                              return String;
+   --  For constructions like [parent.child.grandchild], where only one child
+   --  is allowed. Child is returned as String, and Value is set to granchild.
+   --  Raises Checked_Error if Queue is not a table, or it doesn't contain
+   --  exactly one key.
+
+   function Pop_Single_Table (Queue  : Key_Queue;
+                              Value  : out TOML.TOML_Value;
+                              Kind   : TOML.Any_Value_Kind) return String;
    --  For constructions like [parent.child.grandchild], where we known that
    --  only one child can exist. Will raise Checked_Error if any of these
    --  happens: Queue is not a table; Queue doesn't have exactly one key; Value

src/alire/alire-utils-tools.adb

@@ -1,3 +1,5 @@
+with AAA.Strings;
+
 with Alire.OS_Lib.Subprocess; use Alire.OS_Lib.Subprocess;
 with Alire.OS_Lib;
 with Alire.Platforms.Current;
@@ -157,4 +159,18 @@ package body Alire.Utils.Tools is
       Install_From_Distrib (Tool, Fail);
    end Check_Tool;
 
+   ----------------
+   -- Is_BSD_Tar --
+   ----------------
+
+   function Is_BSD_Tar return Boolean is
+      use AAA.Strings;
+   begin
+      return Contains
+        (To_Lower_Case
+           (Checked_Spawn_And_Capture
+                ("tar", To_Vector ("--version")).Flatten),
+         "bsdtar");
+   end Is_BSD_Tar;
+
 end Alire.Utils.Tools;

src/alire/alire-utils-tools.ads

@@ -18,4 +18,7 @@ package Alire.Utils.Tools is
    --  Check if a required executable tool is available in PATH.
    --  If not, try to install it. If unable and Fail, abort, otherwise return
 
+   function Is_BSD_Tar return Boolean;
+   --  Say if the tar in PATH is the bsdtar variant, which lacks some features
+
 end Alire.Utils.Tools;

src/alire/alire-vfs.ads

@@ -1,8 +1,8 @@
 with Ada.Containers.Vectors;
 
 with Alire.Directories;
+private with Alire.OS_Lib;
 
-private with GNATCOLL.OS.Constants;
 with GNATCOLL.VFS;
 with AAA.Strings; use AAA.Strings;
 
@@ -85,8 +85,6 @@ package Alire.VFS is
 
 private
 
-   use all type GNATCOLL.OS.OS_Type;
-
    -----------------
    -- Is_Portable --
    -----------------
@@ -96,14 +94,18 @@ private
        and then
        not Check_Absolute_Path (Path));
 
+   -----------------
+   -- To_Portable --
+   -----------------
+
+   function To_Portable (Path : Relative_Path) return Portable_Path
+   is (Portable_Path (OS_Lib.To_Portable (Path)));
+
    ---------------
    -- To_Native --
    ---------------
 
    function To_Native (Path : Portable_Path) return Relative_Path
-   is (case GNATCOLL.OS.Constants.OS is
-          when MacOS | Unix => Relative_Path (Path),
-          when Windows      => Relative_Path
-                                 (Replace (String (Path), "/", "\")));
+   is (Relative_Path (OS_Lib.To_Native (OS_Lib.Portable_Path_Like (Path))));
 
 end Alire.VFS;

testsuite/drivers/asserts.py

@@ -141,4 +141,4 @@ def assert_substring(target: str, text: str):
     Check that a string is contained in another string
     """
     assert target in text, \
-        f"Missing expected string '{target}' in text:\n{text}"
+        f"Missing expected string '{target}' in text:\n{text}"

testsuite/fixtures/build_hash_index/gn/gnat_native/gnat_native-8888.0.0.toml

@@ -6,8 +6,7 @@ maintainers-logins = ["mylogin"]
 provides = ["gnat=8888.0"]
 
 # Although the compiler is fake, we use this path in some tests
-environment.'case(os)'.'windows'.TEST_PATH.append = '${CRATE_ROOT}\bin'
-environment.'case(os)'.'...'.TEST_PATH.append = '${CRATE_ROOT}/bin'
+environment.TEST_PATH.append = '${CRATE_ROOT}/bin'
 
 # Test dynamic expression, but for all OSes
 [origin."case(os)"."..."]

testsuite/fixtures/toolchain_index/gn/gnat_native/gnat_native-8888.0.0.toml

@@ -6,8 +6,7 @@ maintainers-logins = ["mylogin"]
 provides = ["gnat=8888.0"]
 
 # Although the compiler is fake, we use this path in some tests
-environment.'case(os)'.'windows'.TEST_PATH.append = '${CRATE_ROOT}\bin'
-environment.'case(os)'.'...'.TEST_PATH.append = '${CRATE_ROOT}/bin'
+environment.TEST_PATH.append = '${CRATE_ROOT}/bin'
 
 # Test dynamic expression, but for all OSes
 [origin."case(os)"."..."]

testsuite/tests/index/env-bad-path/my_index/index/cr/crate/crate-1.0.0.toml

@@ -0,0 +1,14 @@
+description = "Sample crate"
+name = "crate"
+version = "1.0.0"
+licenses = []
+maintainers = ["[email protected]"]
+maintainers-logins = ["someone"]
+
+[environment]
+VAR1.set = "${CRATE_ROOT}/crate_test_bin"    # OK
+VAR2.set = "\\${CRATE_ROOT}/crate_test_bin"  # OK, escape the $ to avoid expansion
+VAR3.set = "${CRATE_ROOT}\\bin"              # BAD, non-portable path
+
+[origin]
+url = "file:../../../crates/crate"

testsuite/tests/index/env-bad-path/my_index/index/index.toml

@@ -0,0 +1 @@
+version = "1.2"