From 8972bc568a646cc1c7ba8ece7aad5d018368a37e Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 30 Sep 2024 13:19:40 +0200 Subject: [PATCH 1/2] Fix: preserve attributes on file copy (#1768) * fix: preserve attributes on file copy * Self-review --- alire.toml | 2 +- deps/den | 2 +- src/alire/alire-directories.adb | 156 +++++------------- src/alire/alire-directories.ads | 10 -- .../my_index/crates/crate/.emptydir | 0 .../my_index/crates/crate/.gitignore | 4 + .../my_index/crates/crate/alire.toml | 12 ++ .../my_index/crates/crate/crate.gpr | 22 +++ .../my_index/crates/crate/myscript.sh | 1 + .../my_index/crates/crate/src/crate.adb | 4 + .../my_index/index/cr/crate/crate-1.0.0.toml | 13 ++ .../sync-attrs/my_index/index/index.toml | 1 + testsuite/tests/cache/sync-attrs/test.py | 19 +++ testsuite/tests/cache/sync-attrs/test.yaml | 8 + 14 files changed, 123 insertions(+), 131 deletions(-) create mode 100644 testsuite/tests/cache/sync-attrs/my_index/crates/crate/.emptydir create mode 100644 testsuite/tests/cache/sync-attrs/my_index/crates/crate/.gitignore create mode 100644 testsuite/tests/cache/sync-attrs/my_index/crates/crate/alire.toml create mode 100644 testsuite/tests/cache/sync-attrs/my_index/crates/crate/crate.gpr create mode 100755 testsuite/tests/cache/sync-attrs/my_index/crates/crate/myscript.sh create mode 100644 testsuite/tests/cache/sync-attrs/my_index/crates/crate/src/crate.adb create mode 100644 testsuite/tests/cache/sync-attrs/my_index/index/cr/crate/crate-1.0.0.toml create mode 100644 testsuite/tests/cache/sync-attrs/my_index/index/index.toml create mode 100644 testsuite/tests/cache/sync-attrs/test.py create mode 100644 testsuite/tests/cache/sync-attrs/test.yaml diff --git a/alire.toml b/alire.toml index b853e5fa8..b5da93505 100644 --- a/alire.toml +++ b/alire.toml @@ -74,7 +74,7 @@ commit = "56bbdc008e16996b6f76e443fd0165a240de1b13" [pins.den] url = "https://github.com/mosteo/den" -commit = "b12e8461bf41e2cfe199c8196b45fa4fc213a6aa" +commit = "653a4c9ba4469d7e1a8896088789b6514ecdf834" [pins.dirty_booleans] url = "https://github.com/mosteo/dirty_booleans" diff --git a/deps/den b/deps/den index b12e8461b..653a4c9ba 160000 --- a/deps/den +++ b/deps/den @@ -1 +1 @@ -Subproject commit b12e8461bf41e2cfe199c8196b45fa4fc213a6aa +Subproject commit 653a4c9ba4469d7e1a8896088789b6514ecdf834 diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index 8824cd6d8..38249592e 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -14,6 +14,7 @@ with Alire.VFS; with Alire.Utils; with Den.Filesystem; +with Den.Iterators; with Den.Walk; with GNAT.String_Hash; @@ -105,62 +106,44 @@ package body Alire.Directories is ---------- procedure Copy (Src_Folder, Dst_Parent_Folder : String; - Excluding : String := "") is - use Ada.Directories; - Search : Search_Type; - Item : Directory_Entry_Type; + Excluding : String := "") + is + use all type Den.Kinds; begin - Start_Search (Search, Src_Folder, "*"); - while More_Entries (Search) loop - Get_Next_Entry (Search, Item); - if Simple_Name (Item) /= Excluding then - -- Recurse for subdirectories - if Kind (Item) = Directory and then - Simple_Name (Item) /= "." and then Simple_Name (Item) /= ".." - then - declare - Subfolder : constant String := - Compose (Dst_Parent_Folder, - Simple_Name (Item)); - begin - if not Exists (Subfolder) then - Ada.Directories.Create_Directory (Subfolder); - end if; - Copy (Full_Name (Item), Subfolder, Excluding); - end; - - -- Copy for files - elsif Kind (Item) = Ordinary_File then - Copy_File (Full_Name (Item), - Compose (Dst_Parent_Folder, Simple_Name (Item))); + for Simple_Item of Den.Iterators.Iterate (Src_Folder) loop + declare + Full_Item : constant Den.Path := Src_Folder / Simple_Item; + begin + if Simple_Item /= Excluding then + -- Recurse for subdirectories + if Den.Kind (Full_Item) = Den.Directory then + declare + Subfolder : constant String := + Dst_Parent_Folder / Simple_Item; + begin + if not Den.Exists (Subfolder) then + Den.Filesystem.Create_Directory (Subfolder); + end if; + Copy (Full_Item, Subfolder, Excluding); + end; + + -- Copy for files/links + elsif Den.Kind (Full_Item) in File | Softlink then + Den.Filesystem.Copy + (Full_Item, + Dst_Parent_Folder / Simple_Item); + + else + Raise_Checked_Error + ("Cannot copy item of kind " & Den.Kind (Full_Item)'Image + & ": " & Full_Item); + + end if; end if; - end if; + end; end loop; - End_Search (Search); end Copy; - --------------- - -- Copy_Link -- - --------------- - - procedure Copy_Link (Src, Dst : Any_Path) is - use AAA.Strings; - use all type Platforms.Operating_Systems; - Keep_Links : constant String - := (case Platforms.Current.Operating_System is - when Linux => "-d", - when FreeBSD | OpenBSD | MacOS => "-R", - when others => - raise Program_Error with "Unsupported operation"); - begin - -- Given that we are here because Src is indeed a link, we should be in - -- a Unix-like platform able to do this. - OS_Lib.Subprocess.Checked_Spawn - ("cp", - To_Vector (Keep_Links) - & Src & Dst); - end Copy_Link; - ----------------- -- Create_Tree -- ----------------- @@ -443,41 +426,6 @@ package body Alire.Directories is Den.Scrub (Child)); end Find_Relative_Path; - ---------------------- - -- Find_Single_File -- - ---------------------- - - function Find_Single_File (Path : String; - Extension : String) - return String - is - use Ada.Directories; - Search : Search_Type; - File : Directory_Entry_Type; - begin - Start_Search (Search => Search, - Directory => Path, - Pattern => "*" & Extension, - Filter => (Ordinary_File => True, others => False)); - if More_Entries (Search) then - Get_Next_Entry (Search, File); - return Name : constant String := - (if More_Entries (Search) - then "" - else Full_Name (File)) - do - End_Search (Search); - end return; - else - End_Search (Search); - return ""; - end if; - exception - when Name_Error => - Trace.Debug ("Search path does not exist: " & Path); - return ""; - end Find_Single_File; - ---------------- -- Initialize -- ---------------- @@ -876,39 +824,9 @@ package body Alire.Directories is end if; end if; - -- We use GNAT.OS_Lib here as some binary packages contain softlinks - -- to .so libs that we must copy too, and these are troublesome - -- with regular Ada.Directories (that has no concept of softlink). - -- Also, some of these softlinks are broken and although they are - -- presumably safe to discard, let's just go for an identical copy. - - if GNAT.OS_Lib.Is_Symbolic_Link (Src) then - Trace.Debug (" Merge (softlink): " & Src); - - Copy_Link (Src, Dst); - if not GNAT.OS_Lib.Is_Symbolic_Link (Dst) then - Raise_Checked_Error ("Failed to copy softlink: " - & TTY.URL (Src) - & " to " & TTY.URL (Dst) - & " (dst not a link)"); - end if; - else - begin - Adirs.Copy_File (Source_Name => Src, - Target_Name => Dst, - Form => "preserve=all_attributes"); - exception - when E : others => - Trace.Error - ("When copying " & Src & " (" & Den.Kind (Src)'Image - & ") --> " & Dst & ": "); - Trace.Error - ("Src item was: " - & Item & " (" & Den.Kind (Item)'Image & ")"); - Log_Exception (E, Error); - raise; - end; - end if; + Den.Filesystem.Copy (Src, Dst); + -- This copy should preserve both softlinks and attributes + end Merge; begin diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index 250b4b709..f32920938 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -32,10 +32,6 @@ package Alire.Directories is -- equivalent to "cp -r src/* dst/". Excluding may be a single name that -- will not be copied (if file) or recursed into (if folder). - procedure Copy_Link (Src, Dst : Any_Path) - with Pre => GNAT.OS_Lib.Is_Symbolic_Link (Src); - -- Copy a softlink into a new place preserving its relative path to target - function Current return String renames Ada.Directories.Current_Directory; function Parent (Path : Any_Path) return String @@ -79,12 +75,6 @@ package Alire.Directories is function Find_Relative_Path_To (Path : Any_Path) return Any_Path; -- Same as Find_Relative_Path (Parent => Current, Child => Path) - function Find_Single_File (Path : String; - Extension : String) - return String; - -- Finds a single file in a folder with the given extension and return its - -- absolute path. If more than one, or none, returns "". - function Is_Directory (Path : Any_Path) return Boolean; -- Returns false for non-existing paths too diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/.emptydir b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/.emptydir new file mode 100644 index 000000000..e69de29bb diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/.gitignore b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/.gitignore new file mode 100644 index 000000000..5866d7bfa --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/.gitignore @@ -0,0 +1,4 @@ +/obj/ +/bin/ +/alire/ +/config/ diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/alire.toml b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/alire.toml new file mode 100644 index 000000000..18d2d0821 --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/alire.toml @@ -0,0 +1,12 @@ +name = "crate" +description = "Sample crate with an action" +version = "0.1.0-dev" + +authors = ["Alejandro R. Mosteo"] +maintainers = ["Alejandro R. Mosteo "] +maintainers-logins = ["mosteo"] +licenses = "MIT OR Apache-2.0 WITH LLVM-exception" +website = "" +tags = [] + +executables = ["crate"] diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/crate.gpr b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/crate.gpr new file mode 100644 index 000000000..29bbd90b8 --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/crate.gpr @@ -0,0 +1,22 @@ +with "config/crate_config.gpr"; +project Crate is + + for Source_Dirs use ("src/", "config/"); + for Object_Dir use "obj/" & Crate_Config.Build_Profile; + for Create_Missing_Dirs use "True"; + for Exec_Dir use "bin"; + for Main use ("crate.adb"); + + package Compiler is + for Default_Switches ("Ada") use Crate_Config.Ada_Compiler_Switches; + end Compiler; + + package Binder is + for Switches ("Ada") use ("-Es"); -- Symbolic traceback + end Binder; + + package Install is + for Artifacts (".") use ("share"); + end Install; + +end Crate; diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/myscript.sh b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/myscript.sh new file mode 100755 index 000000000..eeca002cf --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/myscript.sh @@ -0,0 +1 @@ +echo "SCRIPT RUN" \ No newline at end of file diff --git a/testsuite/tests/cache/sync-attrs/my_index/crates/crate/src/crate.adb b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/src/crate.adb new file mode 100644 index 000000000..27b9f460a --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/crates/crate/src/crate.adb @@ -0,0 +1,4 @@ +procedure Crate is +begin + null; +end Crate; diff --git a/testsuite/tests/cache/sync-attrs/my_index/index/cr/crate/crate-1.0.0.toml b/testsuite/tests/cache/sync-attrs/my_index/index/cr/crate/crate-1.0.0.toml new file mode 100644 index 000000000..cb71c1928 --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/index/cr/crate/crate-1.0.0.toml @@ -0,0 +1,13 @@ +description = "Sample crate" +name = "crate" +version = "1.0.0" +licenses = [] +maintainers = ["any@bo.dy"] +maintainers-logins = ["someone"] + +[[actions]] +command = ["sh", "-c", "./myscript.sh"] +type = "pre-build" + +[origin] +url = "file:../../../crates/crate" diff --git a/testsuite/tests/cache/sync-attrs/my_index/index/index.toml b/testsuite/tests/cache/sync-attrs/my_index/index/index.toml new file mode 100644 index 000000000..c2a2c7dbc --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/my_index/index/index.toml @@ -0,0 +1 @@ +version = "1.2" diff --git a/testsuite/tests/cache/sync-attrs/test.py b/testsuite/tests/cache/sync-attrs/test.py new file mode 100644 index 000000000..afad4a5fd --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/test.py @@ -0,0 +1,19 @@ +""" +Check that executable attributes are synchronized correctly. We have a +dependency which launches a shell script in its pre-build step; this fails +unless the script is executable, which it should be after syncing. +""" + +from drivers.alr import run_alr, init_local_crate, alr_with +from drivers.asserts import assert_eq + +# Init a crate that will have the test crate as a dependency, thus causing its +# syncing. + +init_local_crate() +alr_with("crate", manual=False, update=False) # Delay syncing to capture output +p = run_alr("build", "--stop-after=pre-build") # Gain some testing time by not building + +assert_eq(p.out, "SCRIPT RUN\n") + +print("SUCCESS") diff --git a/testsuite/tests/cache/sync-attrs/test.yaml b/testsuite/tests/cache/sync-attrs/test.yaml new file mode 100644 index 000000000..79bbf3107 --- /dev/null +++ b/testsuite/tests/cache/sync-attrs/test.yaml @@ -0,0 +1,8 @@ +driver: python-script +build_mode: shared +control: + - [SKIP, "skip_unix", "Test is Unix-only"] +indexes: + compiler_only_index: {} + my_index: + in_fixtures: false \ No newline at end of file From 02888809f592e98adaf7182e72262d74309af2b4 Mon Sep 17 00:00:00 2001 From: tali auster Date: Wed, 2 Oct 2024 04:04:36 -0700 Subject: [PATCH 2/2] fix: use /tmp dir if $HOME isn't writable (#1770) * fix: use tmp dir if $HOME isn't writable * Test for running without home --------- Co-authored-by: Alejandro R. Mosteo --- src/alire/alire-platforms-common.adb | 24 +++++++++++ src/alire/alire-platforms-common.ads | 10 +---- src/alr/alr-commands-version.adb | 1 + .../tests/dockerized/misc/no-home/test.py | 40 ++++++++++++++++++ .../tests/dockerized/misc/no-home/test.yaml | 7 ++++ testsuite/tests/misc/no-home/test.py | 42 +++++++++++++++++++ testsuite/tests/misc/no-home/test.yaml | 6 +++ 7 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 testsuite/tests/dockerized/misc/no-home/test.py create mode 100644 testsuite/tests/dockerized/misc/no-home/test.yaml create mode 100644 testsuite/tests/misc/no-home/test.py create mode 100644 testsuite/tests/misc/no-home/test.yaml diff --git a/src/alire/alire-platforms-common.adb b/src/alire/alire-platforms-common.adb index 358e1ab68..f395df497 100644 --- a/src/alire/alire-platforms-common.adb +++ b/src/alire/alire-platforms-common.adb @@ -1,4 +1,5 @@ with AAA.Enum_Tools; +with GNAT.OS_Lib; with Alire.OS_Lib.Subprocess; @@ -71,4 +72,27 @@ package body Alire.Platforms.Common is end Detect; end Machine_Hardware_Name; + ---------------------- + -- Unix_Home_Folder -- + ---------------------- + + function Unix_Home_Folder return String + is + Home_Var : constant String := OS_Lib.Getenv ("HOME", "unset"); + Maybe_Windows : constant Boolean := Home_Var = "unset" + and then GNAT.OS_Lib.Directory_Separator = '\'; + begin + if Maybe_Windows then + raise Checked_Error with + "$HOME is not set, you might be running an" + & " `alr` built for a non-Windows OS"; + elsif Home_Var = "unset" or else + not GNAT.OS_Lib.Is_Write_Accessible_File (Home_Var) + then + return "/tmp"; + else + return Home_Var; + end if; + end Unix_Home_Folder; + end Alire.Platforms.Common; diff --git a/src/alire/alire-platforms-common.ads b/src/alire/alire-platforms-common.ads index 707176934..08a8db8e7 100644 --- a/src/alire/alire-platforms-common.ads +++ b/src/alire/alire-platforms-common.ads @@ -19,15 +19,7 @@ private package Alire.Platforms.Common is -- Unix_Home_Folder -- --------------------- - function Unix_Home_Folder return String - is (if OS_Lib.Getenv ("HOME", "unset") = "unset" and then - GNAT.OS_Lib.Directory_Separator = '\' - then - raise Checked_Error with - "$HOME is not set, you might be running an" - & " `alr` built for a non-Windows OS" - else - OS_Lib.Getenv ("HOME", Default => "/tmp")); + function Unix_Home_Folder return String; ---------------------- -- Unix_Temp_Folder -- diff --git a/src/alr/alr-commands-version.adb b/src/alr/alr-commands-version.adb index a89057d9e..a72c5ad21 100644 --- a/src/alr/alr-commands-version.adb +++ b/src/alr/alr-commands-version.adb @@ -98,6 +98,7 @@ package body Alr.Commands.Version is Add (""); Add ("CONFIGURATION"); + Add ("home folder:", Alire.Platforms.Folders.Home); Add ("settings folder:", Alire.Settings.Edit.Path); Add ("cache folder:", Alire.Cache.Path); Add ("vault folder:", Paths.Vault.Path); diff --git a/testsuite/tests/dockerized/misc/no-home/test.py b/testsuite/tests/dockerized/misc/no-home/test.py new file mode 100644 index 000000000..5be7cacee --- /dev/null +++ b/testsuite/tests/dockerized/misc/no-home/test.py @@ -0,0 +1,40 @@ +""" +Verify that alr can run with no HOME set, or with it set to an +unwritable/nonexistent directory. +NOTE: This same test is duplicated to be run under docker as a regular user. +""" + +import os +from drivers.alr import run_alr + + +def check_no_home(): + # Verify that alr is using the fallback home directory. + p = run_alr("--format", "version") + assert """{ + "key": "home folder", + "value": "/tmp" + }""" in p.out, "Unexpected output: " + p.out + + +# Remove HOME +os.environ.pop("HOME", None) +check_no_home() + +# Set HOME to a nonexistent directory +fake_home = "/tmp/fake_home_for_alr" +assert not os.path.exists(fake_home) +os.environ["HOME"] = fake_home +check_no_home() + +# Set HOME to an unwritable directory +# Under our docker, we should be a regular user and this should succeed +os.makedirs(fake_home) +os.chmod(fake_home, 0o444) +check_no_home() +# Cleanup +os.chmod(fake_home, 0o777) +os.rmdir(fake_home) + + +print("SUCCESS") diff --git a/testsuite/tests/dockerized/misc/no-home/test.yaml b/testsuite/tests/dockerized/misc/no-home/test.yaml new file mode 100644 index 000000000..0e7e8b46b --- /dev/null +++ b/testsuite/tests/dockerized/misc/no-home/test.yaml @@ -0,0 +1,7 @@ +driver: docker-wrapper +build_mode: both +control: + - [SKIP, "skip_docker", "Test is Docker-only"] + - [SKIP, "skip_unix", "Test is Unix-only"] +indexes: + compiler_only_index: {} diff --git a/testsuite/tests/misc/no-home/test.py b/testsuite/tests/misc/no-home/test.py new file mode 100644 index 000000000..3a18bd1d0 --- /dev/null +++ b/testsuite/tests/misc/no-home/test.py @@ -0,0 +1,42 @@ +""" +Verify that alr can run with no HOME set, or with it set to an +unwritable/nonexistent directory. +NOTE: This same test is duplicated to be run under docker as a regular user. +""" + +import os +from drivers.alr import run_alr + + +def check_no_home(): + # Verify that alr is using the fallback home directory. + p = run_alr("--format", "version") + assert """{ + "key": "home folder", + "value": "/tmp" + }""" in p.out, "Unexpected output: " + p.out + + +# Remove HOME +os.environ.pop("HOME", None) +check_no_home() + +# Set HOME to a nonexistent directory +fake_home = "/tmp/fake_home_for_alr" +assert not os.path.exists(fake_home) +os.environ["HOME"] = fake_home +check_no_home() + +# Set HOME to an unwritable directory +# This will fail if running under root, in which case we skip this part. +# This is tested properly under docker +if os.getuid() != 0: + os.makedirs(fake_home) + os.chmod(fake_home, 0o444) + check_no_home() + # Cleanup + os.chmod(fake_home, 0o777) + os.rmdir(fake_home) + + +print("SUCCESS") diff --git a/testsuite/tests/misc/no-home/test.yaml b/testsuite/tests/misc/no-home/test.yaml new file mode 100644 index 000000000..9a541fdd1 --- /dev/null +++ b/testsuite/tests/misc/no-home/test.yaml @@ -0,0 +1,6 @@ +driver: python-script +build_mode: both +control: + - [SKIP, "skip_unix", "Test is Unix-only"] +indexes: + compiler_only_index: {}