Fix bugs for corner cases with shared dependencies (#1453)

* Fix a couple of bugs related to new build system

* Test for the fixed bug

* Self-review

.github/workflows/ci-toolchain.yml

@@ -45,23 +45,23 @@ jobs:
     # We can start using the alr we just built
 
     - name: Update dependencies
-      run: ./bin/alr -n update
+      run: ./bin/alr -d -n update
 
     - name: Show dependencies/pins
-      run: ./bin/alr -n -q with --solve || ./bin/alr -n -v -d with --solve
+      run: ./bin/alr -d -n -q with --solve || ./bin/alr -n -v -d with --solve
 
     - name: Show build environment, with debug fallback
-      run: ./bin/alr -n printenv || ./bin/alr -n -v -d printenv
+      run: ./bin/alr -d -n printenv || ./bin/alr -n -v -d printenv
 
     - shell: bash
       run: mv ./bin ./bin-old
       # Windows doesn't allow to replace a running exe so the next command fails otherwise
 
     - name: SELF-BUILD
-      run: ./bin-old/alr -n build
+      run: ./bin-old/alr -d -n build
 
     - name: Show built version
-      run: ./bin/alr -n version
+      run: ./bin/alr -d -n version
 
     # Run the testsuite with the just build alr. The testsuite picks the proper
     # alr in the ./bin/alr location.

src/alire/alire-roots.adb

@@ -130,14 +130,25 @@ package body Alire.Roots is
       begin
 
          if not State.Has_Release then
-            Put_Info (State.As_Dependency.TTY_Image & ": no build needed.");
+            Put_Info ("Skipping build of " & State.As_Dependency.TTY_Image
+                      & ": not a release", Detail);
             return;
          end if;
 
          declare
             Release : constant Releases.Release := State.Release;
          begin
 
+            --  Skip releases that have no deployment location and hence can't
+            --  run actions.
+            if not Release.Origin.Is_Index_Provided then
+               Put_Info
+                 ("Skipping actions and build of "
+                  & Release.Milestone.TTY_Image
+                  & ": origin is system/external", Detail);
+               return;
+            end if;
+
             --  Run post-fetch, it will be skipped if already ran
             Properties.Actions.Executor.Execute_Actions
               (This,
@@ -151,7 +162,13 @@ package body Alire.Roots is
                Properties.Actions.Pre_Build);
 
             --  Actual build
-            Call_Gprbuild (Release);
+            if Release.Origin.Requires_Build then
+               Call_Gprbuild (Release);
+            else
+               Put_Info
+                 ("Skipping build of " & Release.Milestone.TTY_Image
+                  & ": release has no sources.", Detail);
+            end if;
 
             --  Post-build must run always
             Properties.Actions.Executor.Execute_Actions
@@ -177,25 +194,19 @@ package body Alire.Roots is
       This.Configuration.Ensure_Complete;
       --  For proceeding to build, the configuration must be complete
 
-      --  Ensure sources and configurations are up to date
+      --  Ensure sources are up to date
 
-      if Builds.Sandboxed_Dependencies then
-         This.Generate_Configuration (Full => Force);
-         --  Will regenerate on demand only those changed
-
-      elsif not Builds.Sandboxed_Dependencies then
+      if not Builds.Sandboxed_Dependencies then
          This.Sync_Builds;
-         --  Changes in configuration may require new build dirs
+         --  Changes in configuration may require new build dirs.
+      end if;
 
-         This.Configuration.Generate_Config_Files (This,
-                                                   Release (This),
-                                                   Full => Force);
-         --  Generate the config for the root crate only, the previous sync
-         --  takes care of the rest.
+      --  Ensure configurations are in place and up-to-date
 
-         This.Build_Hasher.Write_Inputs (This);
-         --  Now, after the corresponding config files are in place
-      end if;
+      This.Generate_Configuration (Full => Force);
+      --  Will regenerate on demand only those changed. For shared
+      --  dependencies, will also generate any missing configs not generated
+      --  during sync, such as for linked releases and the root release.
 
       This.Export_Build_Environment;
 
@@ -820,10 +831,26 @@ package body Alire.Roots is
       end Sync_Release;
 
    begin
+      --  If no dependency exists, or is "regular" (has a hash), the root might
+      --  remain unhashed, which causes problems later on, so just in case
+      --  compute all hashes now.
+      This.Compute_Build_Hashes;
+
       --  Visit dependencies in safe order
       This.Traverse (Doing => Sync_Release'Access);
    end Sync_Builds;
 
+   --------------------------
+   -- Compute_Build_Hashes --
+   --------------------------
+
+   procedure Compute_Build_Hashes (This : in out Root) is
+      Unused_Root_Hash : constant String := This.Build_Hash (This.Name);
+      --  This triggers hash computation for all releases in the Root
+   begin
+      null;
+   end Compute_Build_Hashes;
+
    -----------------------------
    -- Sync_Pins_From_Manifest --
    -----------------------------

src/alire/alire-roots.ads

@@ -142,7 +142,9 @@ package Alire.Roots is
    function Requires_Build_Sync (This : in out Root;
                                  Rel  : Releases.Release)
                                  return Boolean
-     with Pre => This.Solution.Contains_Release (Rel.Name);
+     with Pre =>
+       This.Solution.Contains_Release (Rel.Name) or else
+       raise Program_Error with "Release not in solution: " & Rel.Name_Str;
    --  Says if the release requires a build copy taking into account everything
 
    function Nonabstract_Crates (This : in out Root)
@@ -429,4 +431,7 @@ private
    procedure Sync_Builds (This : in out Root);
    --  Sync from vault to final build location, and generate config
 
+   procedure Compute_Build_Hashes (This : in out Root);
+   --  Trigger computation of build hashes
+
 end Alire.Roots;

src/alire/alire-solutions.adb

@@ -56,7 +56,7 @@ package body Alire.Solutions is
 
    function Contains_Release (This  : Solution;
                               Crate : Crate_Name) return Boolean
-   is (This.Depends_On (Crate) and then This.State (Crate).Is_Solved);
+   is (This.Depends_On (Crate) and then This.State (Crate).Has_Release);
 
    ----------------
    -- Dependency --

src/alire/alire-solutions.ads

@@ -188,8 +188,8 @@ package Alire.Solutions is
 
    function Contains_Release (This  : Solution;
                               Crate : Crate_Name) return Boolean;
-   --  Say if Crate is among the solved releases for this solution. It will
-   --  return False if the solution does not even depend on Crate.
+   --  Say if Crate is among the releases (solved or linked) for this solution.
+   --  It will return False if the solution does not even depend on Crate.
 
    function Crates (This : Solution) return Name_Set;
    --  Dependency name closure, independent of the status in the solution, as

src/alr/alr-commands-build.adb

@@ -88,9 +88,7 @@ package body Alr.Commands.Build is
 
       --  And redirect to actual execution procedure
 
-      if not Execute (Cmd, Args,
-                      Export_Build_Env => True)
-      then
+      if not Execute (Cmd, Args) then
          Reportaise_Command_Failed ("Compilation failed.");
       end if;
    end Execute;
@@ -100,8 +98,7 @@ package body Alr.Commands.Build is
    -------------
 
    function Execute (Cmd              : in out Commands.Command'Class;
-                     Args             :        AAA.Strings.Vector;
-                     Export_Build_Env :        Boolean)
+                     Args             :        AAA.Strings.Vector)
                      return Boolean
    is
    begin
@@ -115,7 +112,6 @@ package body Alr.Commands.Build is
          Timer : Stopwatch.Instance;
       begin
          if Cmd.Root.Build (Args,
-                            Export_Build_Env,
                             Saved_Profiles => Cmd not in Build.Command'Class)
            --  That is, we apply the saved profiles unless the user is
            --  explicitly invoking `alr build`.

src/alr/alr-commands-build.ads

@@ -20,9 +20,8 @@ package Alr.Commands.Build is
    procedure Execute (Cmd  : in out Command;
                       Args :        AAA.Strings.Vector);
 
-   function Execute (Cmd              : in out Commands.Command'Class;
-                     Args             :        AAA.Strings.Vector;
-                     Export_Build_Env :        Boolean)
+   function Execute (Cmd  : in out Commands.Command'Class;
+                     Args :        AAA.Strings.Vector)
                      return Boolean;
    --  Returns True if compilation succeeded. For invocations after some other
    --  command that already has set up the build environment we need to avoid

src/alr/alr-commands-run.adb

@@ -134,8 +134,7 @@ package body Alr.Commands.Run is
          --  COMPILATION  --
          if not Cmd.No_Compile then
             if not Commands.Build.Execute (Cmd,
-                                           Args => AAA.Strings.Empty_Vector,
-                                           Export_Build_Env => True)
+                                           Args => AAA.Strings.Empty_Vector)
             then
                Reportaise_Command_Failed ("Build failed");
             end if;

testsuite/drivers/builds.py

@@ -6,7 +6,14 @@
 import os
 from shutil import rmtree
 import subprocess
-from drivers.alr import alr_builds_dir
+from drivers.alr import alr_builds_dir, run_alr
+
+
+def enable_shared() -> None:
+    """
+    Enable shared builds
+    """
+    run_alr("config", "--global", "--set", "dependencies.shared", "true")
 
 
 def clear_builds_dir() -> None:

testsuite/tests/build/hashes/linked-dependency/test.py

@@ -0,0 +1,17 @@
+"""
+check that a linked dependency causes no trouble in shared builds
+"""
+
+from drivers.alr import alr_with, init_local_crate, run_alr
+from drivers import builds
+# from drivers.asserts import assert_eq, assert_match
+
+builds.enable_shared()
+
+init_local_crate()
+init_local_crate("dep", enter=False)
+alr_with("dep", path="dep")
+
+run_alr("build")  # Should succeed
+
+print('SUCCESS')

testsuite/tests/build/hashes/linked-dependency/test.yaml

@@ -0,0 +1,3 @@
+driver: python-script
+indexes:
+    build_hash_index: {}

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

@@ -56,7 +56,10 @@ def check_in(file : str, expected : str) -> bool:
 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
+# check_in(f'{nbase}/obj/b__hello.ads', files)          # build took place
+# The build actually doesn't take place because the dependency is not used.
+# Due to a former bug, where all deps were built, the previous check succeeded.
+# The line is left in case this bug reappears, so it's easier to re-understand.
 
 # And that the crate usual cache dir doesn't exist
 assert not os.path.exists(alr_workspace_cache())