Skip to content

Commit

Permalink
Include hash of dependencies in build hash
Browse files Browse the repository at this point in the history
Since a dependency may use different specs based on config values, environment
variables, GPR externals... we include the hash of dependencies in dependents'
hash inputs.
  • Loading branch information
mosteo committed Aug 31, 2023
1 parent 6879428 commit c5254ee
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 32 deletions.
100 changes: 77 additions & 23 deletions src/alire/alire-builds-hashes.adb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ with Alire.GPR;
with Alire.Hashes.SHA256_Impl;
with Alire.Paths;
with Alire.Roots;
with Alire.Solutions;
with Alire.Dependencies.States;
with Alire.Utils.Text_Files;

package body Alire.Builds.Hashes is
Expand Down Expand Up @@ -75,8 +77,19 @@ package body Alire.Builds.Hashes is
-- of consecutive entries.
end loop;

This.Hashes.Insert (Rel.Name, SHA.Get_Digest (C));
This.Inputs.Insert (Rel.Name, Vars);
-- We can rarely reach here more than once for provided regular
-- releases, so we cannot simply insert once. Instead of including
-- blindly, we double-check things match.
if This.Hashes.Contains (Rel.Name)
and then This.Hashes (Rel.Name) /= SHA.Get_Digest (C)
then
raise Program_Error with
"Conflicting build hashes for release "
& Rel.Milestone.Image;
else
This.Hashes.Include (Rel.Name, SHA.Get_Digest (C));
This.Inputs.Include (Rel.Name, Vars);
end if;
end Compute_Hash;

-----------------
Expand Down Expand Up @@ -153,6 +166,30 @@ package body Alire.Builds.Hashes is
Add => Add'Access);
end Add_Configuration;

----------------------
-- Add_Dependencies --
----------------------

procedure Add_Dependencies is
-- Since specs of dependencies may change due to config variables,
-- or sources selected depending on environment variables/GPR
-- externals, the safest course of action is to also include
-- dependency hashes in our own hash. For dependencies without
-- such things the hash won't change anyway.
begin
for Dep of Rel.Flat_Dependencies loop
for Target of Root.Solution.Releases loop
if Target.Origin.Requires_Build
and then Target.Satisfies (Dep)
then
Add ("dependency",
Target.Milestone.Image,
This.Hashes (Target.Name));
end if;
end loop;
end loop;
end Add_Dependencies;

begin
Trace.Debug (" build hashing: " & Rel.Milestone.TTY_Image);

Expand All @@ -167,25 +204,34 @@ package body Alire.Builds.Hashes is
Add_Externals; -- GPR externals
Add_Environment; -- Environment variables
Add_Compiler; -- Compiler version
Add_Dependencies; -- Hash of dependencies
end if;

-- Dependencies recursive hash? Since a crate can use a dependency
-- config spec, it is possible in the worst case for a crate to
-- require unique builds that include their dependencies hash
-- in their own hash. This is likely a corner case, but we can't
-- currently detect it. Two options are to alway err on the side of
-- caution, always including dependencies hashes, or to add some new
-- info in the manifest saying whose crates config affect the crate.
-- We could also enable this recursive hashing globally or per
-- crate...
-- TBD

-- Final computation
Compute_Hash;

Trace.Debug (" build hashing release complete");
end Compute;

-------------
-- Compute --
-------------

procedure Compute (unused_Root : in out Roots.Root;
Unused_Sol : Solutions.Solution;
State : Dependencies.States.State)
is
begin
if State.Has_Release
and then State.Release.Origin.Requires_Build
then
-- We cannot filter out provided dependencies because all
-- dependencies may be fulfilled by a provided release that
-- doesn't appear otherwise (as non-provided).
Compute (State.Release);
end if;
end Compute;

Context : Environment.Context;

begin
Expand All @@ -197,11 +243,7 @@ package body Alire.Builds.Hashes is

Root.Configuration.Ensure_Complete;

for Rel of Root.Nonabstract_Releases loop -- includes the root release
if Rel.Origin.Requires_Build then
Compute (Rel);
end if;
end loop;
Root.Traverse (Compute'Access);
end Compute;

----------------------
Expand Down Expand Up @@ -244,12 +286,24 @@ package body Alire.Builds.Hashes is
Backup => False);
end Write_Inputs;

begin
for Rel of Root.Nonabstract_Releases loop
if Rel.Origin.Requires_Build then
Write_Inputs (Rel);
------------------
-- Write_Inputs --
------------------

procedure Write_Inputs (Unused_Root : in out Roots.Root;
Unused_Sol : Solutions.Solution;
State : Dependencies.States.State)
is
begin
if State.Has_Release
and then State.Release.Origin.Requires_Build
then
Write_Inputs (State.Release);
end if;
end loop;
end Write_Inputs;

begin
Root.Traverse (Write_Inputs'Access);
end Write_Inputs;

----------
Expand Down
2 changes: 1 addition & 1 deletion testsuite/drivers/builds.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import os
from shutil import rmtree
import subprocess
from drivers.alr import alr_builds_dir, run_alr
from drivers.alr import alr_builds_dir


def clear_builds_dir() -> None:
Expand Down
2 changes: 1 addition & 1 deletion testsuite/tests/build/hashes/compiler-input/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def check_hash(signature: str) -> None:
# Next, check that selecting a compiler results in it being used

# Select the default preferred compiler, in this index is gnat_native=8888
run_alr("toolchain", "--select", "gnat_native")
run_alr("toolchain", "--select", "gnat_native", "gprbuild")
# Clear the build cache so we are able to locate the new hash
clear_builds_dir()
builds.sync()
Expand Down
7 changes: 4 additions & 3 deletions testsuite/tests/build/hashes/config-types/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""

from drivers.alr import alr_with, external_compiler_version, init_local_crate, run_alr
from drivers.builds import hash_input
from drivers.builds import find_hash, hash_input
from drivers.asserts import assert_eq
from drivers import builds

Expand All @@ -13,8 +13,8 @@
alr_with("hello=1.0.1")
builds.sync()

# Chech that the hash inputs contains exactly what we expect it to contain

# Chech that the hash inputs contains exactly what we expect it to contain.
# We cannot know the dependency hash in advance as it depends on the compiler.
assert_eq(
'config:hello.var1=true\n'
'config:hello.var2=str\n'
Expand All @@ -23,6 +23,7 @@
'config:hello.var5=0\n'
'config:hello.var6=0.00000000000000E+00\n'
'config:hello.var7=0.00000000000000E+00\n'
f'dependency:libhello=1.0.0={find_hash("libhello")}\n'
'profile:hello=RELEASE\n'
f'version:gnat_external={external_compiler_version()}\n',
hash_input("hello"))
Expand Down
33 changes: 30 additions & 3 deletions testsuite/tests/build/hashes/hashing-inputs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
other inputs.
"""

import os
import shutil
from drivers.alr import alr_with, external_compiler_version, init_local_crate, run_alr
from drivers.builds import find_hash, hash_input
from drivers.asserts import assert_eq, assert_match
from drivers import builds
from drivers.helpers import content_of

run_alr("config", "--set", "--global", "dependencies.shared", "true")
init_local_crate()
alr_with("libhello")
alr_with("hello")

# Build the crate in default mode, so dependencies are in RELEASE mode
run_alr("build")
Expand All @@ -30,12 +32,12 @@
# Check that the hashes are different
assert hash1 != hash2, "Hashes should be different"

# Chech that the hash inputs contains exactly what we expect it to contain.
# Check that the hash inputs contains exactly what we expect it to contain.
# This includes environment variables, GPR externals set or observed, build
# profile, compiler version.

assert_eq(
'config:libhello.var1=true\n' # crate config var
'config:libhello.var1=false\n' # crate config var (set by hello)
'environment:TEST_ENV=myenv\n' # plain env var set
'external:TEST_FREEFORM_UNSET=default\n' # declared unset GPR external
'external:TEST_GPR_EXTERNAL=gpr_ext_B\n' # declared set GPR external
Expand All @@ -45,4 +47,29 @@
# compiler version
hash_input("libhello"))

# Check the hash inputs of a crate with dependencies itself (hello -> libhello).
# We cannot know the dependency hash in advance as it depends on the compiler.
assert_eq(
'config:hello.var1=true\n'
'config:hello.var2=str\n'
'config:hello.var3=A\n'
'config:hello.var4=0\n'
'config:hello.var5=0\n'
'config:hello.var6=0.00000000000000E+00\n'
'config:hello.var7=0.00000000000000E+00\n'
f'dependency:libhello=1.0.0={find_hash("libhello")}\n'
'profile:hello=VALIDATION\n'
f'version:gnat_external={external_compiler_version()}\n',
hash_input("hello"))

# Bonus: check the hash inputs for the root crate, used for config regeneration
# Using find_hash here ensures that the hash in the dir name matches the one in
# the inputs of a different crate that depends on it.
assert_eq(
f'dependency:hello=1.0.1={find_hash("hello")}\n'
'profile:xxx=VALIDATION\n'
f'version:gnat_external={external_compiler_version()}\n',
content_of(os.path.join("alire", "build_hash_inputs"))
)

print("SUCCESS")
2 changes: 1 addition & 1 deletion testsuite/tests/crate_config/no-rebuilds/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

# Switch to another profile and build must happen
p = run_alr("build", "--validation", quiet=False)
assert_match('.*\[link\] xxx.adb', p.out)
assert_match('.*\[Ada\]\s+xxx.adb', p.out)

# Use same profile, nothing should be recompiled
p = run_alr("build", "--validation", quiet=False)
Expand Down

0 comments on commit c5254ee

Please sign in to comment.