Skip to content

Commit

Permalink
Test for fix
Browse files Browse the repository at this point in the history
  • Loading branch information
mosteo committed Oct 24, 2023
1 parent f21a22f commit 02310c9
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 20 deletions.
21 changes: 20 additions & 1 deletion src/alire/alire-environment-formatting.adb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
2 changes: 2 additions & 0 deletions src/alire/alire-os_lib.ads
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ package Alire.OS_Lib with Preelaborate is

-- 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);
Expand Down
30 changes: 16 additions & 14 deletions src/alire/alire-properties-environment.adb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
with Alire.OS_Lib;
with Alire.TOML_Keys;
with Alire.Utils.Did_You_Mean;

Expand Down Expand Up @@ -74,18 +73,20 @@ package body Alire.Properties.Environment is
Env : TOML_Value;

----------------
-- path_check --
-- Path_Check --
----------------

function Path_Check (S : String) return String is
procedure Path_Check (Var, S : String) is
begin
-- We expect something resembling a portable path
if (for some Char of S => Char = '\') then
Raise_Checked_Error
("Forbidden '\' character in environment path; use '/' instead");
end if;

return OS_Lib.To_Native (S);
-- 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
Expand Down Expand Up @@ -126,10 +127,11 @@ package body Alire.Properties.Environment is
Actions_Suggestion (Action_Image));
end;

-- We consider values as possibly containing paths, so we do
-- translation to native at this time.
-- We consider values as possibly containing paths, so we check
-- that path separators are portable

Var.Value := +Path_Check (Val.As_String);
Path_Check (+Name, Val.As_String);
Var.Value := +Val.As_String;

-- Pop entry to avoid upper "unexpected key" errors

Expand Down Expand Up @@ -158,7 +160,7 @@ package body Alire.Properties.Environment is
-- Create the VAR.action.Value nested tables

Child.Set (AAA.Strings.To_Lower_Case (This.Action'Img),
Create_String (OS_Lib.To_Portable (Value (This))));
Create_String (Value (This)));

Result.Set (Name (This),
Child);
Expand Down
2 changes: 1 addition & 1 deletion src/alire/alire-properties-environment.ads
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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)"."..."]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"."..."]
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version = "1.2"
13 changes: 13 additions & 0 deletions testsuite/tests/index/env-bad-path/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
Detect a bad path in an environment variable
"""

from drivers.alr import run_alr
from drivers.asserts import assert_match

# Attempting to load the crate to show it should fail but only for the VAR3 bad variable

assert_match(".*VAR3: forbidden",
run_alr("show", "crate", complain_on_error=False).out)

print('SUCCESS')
5 changes: 5 additions & 0 deletions testsuite/tests/index/env-bad-path/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
driver: python-script
build_mode: both
indexes:
my_index:
in_fixtures: false
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
description = "Sample crate"
name = "crate"
version = "1.0.0"
licenses = []
maintainers = ["[email protected]"]
maintainers-logins = ["someone"]

[environment]
PATH.append = "${CRATE_ROOT}/crate_test_bin"

[origin]
url = "file:../../../crates/crate"
1 change: 1 addition & 0 deletions testsuite/tests/index/env-path/my_index/index/index.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version = "1.2"
30 changes: 30 additions & 0 deletions testsuite/tests/index/env-path/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""
Paths in environment values should use native path separators in the index, and
be converted to the native separator for usage.
"""

import os

from drivers.alr import crate_dirname, run_alr
from drivers.asserts import assert_match
from drivers.helpers import content_of

# Check that the output of `alr show` shows the proper slashes

# Show uses the path with portable separators, as the conversion is done just
# for printenv output.
assert_match(f".*{os.sep}crate_test_bin",
run_alr("show", "crate").out)

# Similarly, if we retrieve the crate, the output to the manifest must be in
# portable format (always forward slashes).
run_alr("get", "crate")
assert_match(f".*/crate_test_bin",
content_of(f"{crate_dirname('crate')}/alire.toml"))

# When obtained via printenv, the path must be native
os.chdir(crate_dirname('crate'))
assert_match(f".*PATH=[^\n]*{os.sep}crate_test_bin",
run_alr("printenv").out)

print('SUCCESS')
5 changes: 5 additions & 0 deletions testsuite/tests/index/env-path/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
driver: python-script
build_mode: both
indexes:
my_index:
in_fixtures: false

0 comments on commit 02310c9

Please sign in to comment.