From 8243841325be7938a7f3e6aafa311b335cf2d9a1 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Fri, 15 Sep 2023 09:19:28 +0200 Subject: [PATCH] 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 | 10 +-- src/alire/alire-roots.adb | 61 +++++++++++++------ src/alire/alire-roots.ads | 7 ++- src/alire/alire-solutions.adb | 2 +- src/alire/alire-solutions.ads | 4 +- src/alr/alr-commands-build.adb | 8 +-- src/alr/alr-commands-build.ads | 5 +- src/alr/alr-commands-run.adb | 3 +- testsuite/drivers/builds.py | 9 ++- .../build/hashes/linked-dependency/test.py | 17 ++++++ .../build/hashes/linked-dependency/test.yaml | 3 + testsuite/tests/config/shared-deps/test.py | 5 +- 12 files changed, 95 insertions(+), 39 deletions(-) create mode 100644 testsuite/tests/build/hashes/linked-dependency/test.py create mode 100644 testsuite/tests/build/hashes/linked-dependency/test.yaml diff --git a/.github/workflows/ci-toolchain.yml b/.github/workflows/ci-toolchain.yml index 7a0046aa6..41d7442f2 100644 --- a/.github/workflows/ci-toolchain.yml +++ b/.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. diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index ab79d6160..2184c42da 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -130,7 +130,8 @@ 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; @@ -138,6 +139,16 @@ package body Alire.Roots is 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 -- ----------------------------- diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index ceda622fe..c55a5985e 100644 --- a/src/alire/alire-roots.ads +++ b/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; diff --git a/src/alire/alire-solutions.adb b/src/alire/alire-solutions.adb index 7fce6794f..3654acc39 100644 --- a/src/alire/alire-solutions.adb +++ b/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 -- diff --git a/src/alire/alire-solutions.ads b/src/alire/alire-solutions.ads index cacac1885..8b553e340 100644 --- a/src/alire/alire-solutions.ads +++ b/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 diff --git a/src/alr/alr-commands-build.adb b/src/alr/alr-commands-build.adb index 69efa6b1a..e4327b58d 100644 --- a/src/alr/alr-commands-build.adb +++ b/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`. diff --git a/src/alr/alr-commands-build.ads b/src/alr/alr-commands-build.ads index 99058ecac..74d6d636b 100644 --- a/src/alr/alr-commands-build.ads +++ b/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 diff --git a/src/alr/alr-commands-run.adb b/src/alr/alr-commands-run.adb index 6c6517198..a001e4f5b 100644 --- a/src/alr/alr-commands-run.adb +++ b/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; diff --git a/testsuite/drivers/builds.py b/testsuite/drivers/builds.py index 81c56f43d..d8479df71 100644 --- a/testsuite/drivers/builds.py +++ b/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: diff --git a/testsuite/tests/build/hashes/linked-dependency/test.py b/testsuite/tests/build/hashes/linked-dependency/test.py new file mode 100644 index 000000000..2dbf5ecc5 --- /dev/null +++ b/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') diff --git a/testsuite/tests/build/hashes/linked-dependency/test.yaml b/testsuite/tests/build/hashes/linked-dependency/test.yaml new file mode 100644 index 000000000..8e25447d9 --- /dev/null +++ b/testsuite/tests/build/hashes/linked-dependency/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + build_hash_index: {} diff --git a/testsuite/tests/config/shared-deps/test.py b/testsuite/tests/config/shared-deps/test.py index 9adf1a9df..ed458638e 100644 --- a/testsuite/tests/config/shared-deps/test.py +++ b/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())