Cleanup after shared releases impl (#1448)

* Omit default mention in shared deps help

* Remove Export_Env arguments all-around

Now that we delay all actions to build time, we can set environment just at
that time and avoid complications.

* Debug workflow and dependency hashes

* Self-review

.github/workflows/ci-toolchain.yml

@@ -50,8 +50,8 @@ jobs:
     - name: Show dependencies/pins
       run: ./bin/alr -n -q with --solve || ./bin/alr -n -v -d with --solve
 
-    - name: Show build environment
-      run: ./bin/alr -n printenv
+    - name: Show build environment, with debug fallback
+      run: ./bin/alr -n printenv || ./bin/alr -n -v -d printenv
 
     - shell: bash
       run: mv ./bin ./bin-old
@@ -76,4 +76,4 @@ jobs:
 
     - name: Run testsuite
       run: cd testsuite; ./run.py -E
-      shell: bash
+      shell: bash

src/alire/alire-builds-hashes.adb

@@ -1,6 +1,7 @@
 with Alire.Crate_Configuration.Hashes;
 with Alire.Directories;
 with Alire.Environment.Loading;
+with Alire.Errors;
 with Alire.GPR;
 with Alire.Hashes.SHA256_Impl;
 with Alire.Paths;
@@ -193,9 +194,16 @@ package body Alire.Builds.Hashes is
                   if Target.Origin.Requires_Build
                     and then Target.Satisfies (Dep)
                   then
-                     Add ("dependency",
-                          Target.Milestone.Image,
-                          This.Hashes (Target.Name));
+                     if This.Contains (Target.Name) then
+                        Add ("dependency",
+                             Target.Milestone.Image,
+                             This.Hashes (Target.Name));
+                     else
+                        raise Program_Error with Errors.Set
+                          (Rel.Milestone.Image & " depends on "
+                           & Target.Milestone.Image
+                           & " but hash is not yet computed?");
+                     end if;
                   end if;
                end loop;
             end loop;

src/alire/alire-builds-hashes.ads

@@ -10,6 +10,8 @@ package Alire.Builds.Hashes is
    procedure Clear (This : in out Hasher);
    --  Remove any cached hashes
 
+   function Contains (This : in out Hasher; Name : Crate_Name) return Boolean;
+
    function Is_Empty (This : Hasher) return Boolean;
    --  Says if the Hasher has been used or not
 
@@ -57,4 +59,11 @@ private
       Inputs : Crate_Input_Maps.Map;
    end record;
 
+   --------------
+   -- Contains --
+   --------------
+
+   function Contains (This : in out Hasher; Name : Crate_Name) return Boolean
+   is (This.Hashes.Contains (Name));
+
 end Alire.Builds.Hashes;

src/alire/alire-config-builtins.ads

@@ -11,7 +11,7 @@ package Alire.Config.Builtins is
       Def  => False,
       Help =>
         "When true, dependencies are downloaded and built in a shared "
-      & "location inside the global cache. When false (default), "
+      & "location inside the global cache. When false, "
       & "dependencies are sandboxed in each workspace.");
 
    Distribution_Disable_Detection : constant Builtin := New_Builtin

src/alire/alire-roots.adb

@@ -39,7 +39,6 @@ package body Alire.Roots is
 
    function Build (This             : in out Root;
                    Cmd_Args         : AAA.Strings.Vector;
-                   Export_Build_Env : Boolean;
                    Build_All_Deps   : Boolean := False;
                    Saved_Profiles   : Boolean := True)
                    return Boolean
@@ -198,9 +197,7 @@ package body Alire.Roots is
          --  Now, after the corresponding config files are in place
       end if;
 
-      if Export_Build_Env or else not Builds.Sandboxed_Dependencies then
-         This.Export_Build_Environment;
-      end if;
+      This.Export_Build_Environment;
 
       This.Traverse (Build_Single_Release'Access);
 
@@ -235,7 +232,15 @@ package body Alire.Roots is
          This.Build_Hasher.Compute (This);
       end if;
 
-      return This.Build_Hasher.Hash (Name);
+      if This.Build_Hasher.Contains (Name) then
+         return This.Build_Hasher.Hash (Name);
+      else
+         Trace.Error
+           ("Requested build hash of release " & Name.As_String
+            & " not among solution states:");
+         This.Solution.Print_States ("   ", Error);
+         raise Program_Error;
+      end if;
    end Build_Hash;
 
    --------------
@@ -253,7 +258,6 @@ package body Alire.Roots is
      (This           : in out Root;
       Prefix         : Absolute_Path;
       Build          : Boolean := True;
-      Export_Env     : Boolean := True;
       Print_Solution : Boolean := True)
    is
       use AAA.Strings;
@@ -411,12 +415,11 @@ package body Alire.Roots is
 
       if Build then
          Assert (This.Build (Cmd_Args         => AAA.Strings.Empty_Vector,
-                             Export_Build_Env => Export_Env,
                              Build_All_Deps   => True),
                  Or_Else => "Build failed, cannot perform installation");
       end if;
 
-      if Export_Env then
+      if not Build then
          This.Export_Build_Environment;
       end if;
 
@@ -744,37 +747,11 @@ package body Alire.Roots is
 
    begin
 
-      --  Prepare environment for any post-fetch actions. This must be done
-      --  after the lockfile on disk is written, since the root will read
-      --  dependencies from there. Post-fetch may happen even with shared
-      --  builds for linked and binary dependencies.
-
-      if Builds.Sandboxed_Dependencies then
-         This.Export_Build_Environment;
-      else
-         null;
-         --  When using shared dependencies we have a conflict between crates
-         --  "in-place" (without syncing, e.g. links/binaries), which should
-         --  have its post-fetch run immediately, and regular crates, which
-         --  get the post-fetch run after sync. Since the complete environment
-         --  cannot be known for the former until build time (as config could
-         --  be incomplete otherwise), we need to delay post-fetch for all
-         --  crates to build time, in a follow-up PR. Meanwhile, in some corner
-         --  cases post-fetch could fail when using shared deps (in-place
-         --  crates with a post-fetch that relies on the environment).
-
-         --  TODO: delay post-fetch for binary/linked crates to the build
-         --  moment too. Do this for both sandboxed/shared, for the sake
-         --  of simplicity?
-      end if;
-
-      --  Visit dependencies in a safe order to be fetched, and their actions
-      --  ran
+      --  Visit dependencies in a safe order to be fetched
 
       This.Traverse (Doing => Deploy_Release'Access);
 
-      --  Show hints for missing externals to the user after all the noise of
-      --  dependency post-fetch compilations.
+      --  Show hints for missing externals to the user
 
       This.Solution.Print_Hints (This.Environment);
 
@@ -843,9 +820,6 @@ package body Alire.Roots is
       end Sync_Release;
 
    begin
-      --  Prepare environment for any post-fetch actions
-      This.Export_Build_Environment;
-
       --  Visit dependencies in safe order
       This.Traverse (Doing => Sync_Release'Access);
    end Sync_Builds;

src/alire/alire-roots.ads

@@ -255,7 +255,6 @@ package Alire.Roots is
 
    function Build (This             : in out Root;
                    Cmd_Args         : AAA.Strings.Vector;
-                   Export_Build_Env : Boolean;
                    Build_All_Deps   : Boolean := False;
                    Saved_Profiles   : Boolean := True)
                    return Boolean;
@@ -287,7 +286,6 @@ package Alire.Roots is
      (This           : in out Root;
       Prefix         : Absolute_Path;
       Build          : Boolean := True;
-      Export_Env     : Boolean := True;
       Print_Solution : Boolean := True);
    --  Call gprinstall on the releases in solution using --prefix=Prefix
 

src/alr/alr-commands-get.adb

@@ -190,12 +190,9 @@ package body Alr.Commands.Get is
                  (Crate   =>  Cmd.Root.Name,
                   Profile =>  Alire.Utils.Switches.Release);
 
-               --  The complete build environment has been set up already by
-               --  Deploy_Dependencies, so we must not do it again.
                Build_OK := Cmd.Root.Build
                  (Cmd_Args         =>  AAA.Strings.Empty_Vector,
-                  Saved_Profiles   => False,
-                  Export_Build_Env => False);
+                  Saved_Profiles   => False);
             end if;
          else
             Build_OK := True;

src/alr/alr-commands-install.adb

@@ -63,8 +63,7 @@ package body Alr.Commands.Install is
                   & " is already installed, use " & TTY.Terminal ("--force")
                   & " to reinstall");
             when New_Install | Reinstall | Replace =>
-               Cmd.Root.Install (Prefix     => Prefix,
-                                 Export_Env => True);
+               Cmd.Root.Install (Prefix => Prefix);
          end case;
 
       else

testsuite/drivers/helpers.py

@@ -256,6 +256,12 @@ def replace_in_file(filename : str, old : str, new : str):
         file.write(old_contents.replace(old, new))
 
 
+def neutral_path(path : str) -> str:
+    """
+    Return a path with all separators replaced by '/'.
+    """
+    return path.replace('\\', '/')
+
 class FileLock():
     """
     A filesystem-level lock for tests executed from different threads but

testsuite/tests/config/shared-deps/test.py

@@ -9,7 +9,12 @@
 from drivers.alr import (alr_builds_dir, alr_vault_dir, alr_with,
                          alr_workspace_cache, init_local_crate, run_alr)
 from drivers.asserts import assert_contents, assert_file_exists
-from drivers.helpers import lines_of
+from drivers.helpers import contents, lines_of, neutral_path
+
+
+def check_in(file : str, expected : str) -> bool:
+    assert file in expected, f"Missing file '{file}' in\n{expected}"
+
 
 vault_dir = alr_vault_dir()
 build_dir = alr_builds_dir()
@@ -46,21 +51,12 @@
 run_alr("build")
 base = builds.find_dir("hello_1.0.1_filesystem")
 
-assert_contents(base,
-                [f'{base}/alire',
-                 f'{base}/alire.toml',
-                 f'{base}/alire/build_hash_inputs',
-                 f'{base}/alire/flags',
-                 f'{base}/alire/flags/complete_copy',
-                 f'{base}/alire/flags/post_fetch_done',
-                 f'{base}/config',
-                 f'{base}/config/hello_config.ads',
-                 f'{base}/config/hello_config.gpr',
-                 f'{base}/config/hello_config.h',
-                 f'{base}/hello.gpr',
-                 f'{base}/obj',
-                 f'{base}/src',
-                 f'{base}/src/hello.adb'])
+# There's too much object files and the like, check a few critical files:
+files = contents(base)  # This returns "normalized" paths (with '/' separators)
+nbase = neutral_path(base)
+check_in(f'{nbase}/config/hello_config.ads', files)     # config was generated
+check_in(f'{nbase}/alire/flags/post_fetch_done', files) # actions were run
+check_in(f'{nbase}/obj/b__hello.ads', files)            # build took place
 
 # And that the crate usual cache dir doesn't exist
 assert not os.path.exists(alr_workspace_cache())