Skip to content

Commit

Permalink
Fix bugs causing spurious diffs on updates without changes (#1550)
Browse files Browse the repository at this point in the history
* Fix bugs causing spurious diffs on update

Solutions were the same, but some types internally where using pointers instead
of pointed objects for comparison.

WIP

* Separate style checking from experimental Ada switches
  • Loading branch information
mosteo authored Jan 31, 2024
1 parent a9a8fb1 commit f850edd
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 36 deletions.
2 changes: 1 addition & 1 deletion alire.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ windows = { ALIRE_OS = "windows" }

# Some dependencies require precise versions during the development cycle:
[[pins]]
aaa = { url = "https://github.com/mosteo/aaa", commit = "ecc38772bd4a6b469b54c62363766ea1c0e9f912" }
aaa = { url = "https://github.com/mosteo/aaa", commit = "dff61d2615cc6332fa6205267bae19b4d044b9da" }
ada_toml = { url = "https://github.com/mosteo/ada-toml", commit = "da4e59c382ceb0de6733d571ecbab7ea4919b33d" }
clic = { url = "https://github.com/alire-project/clic", commit = "de0330053584bad4dbb3dbd5e1ba939c4e8c6b55" }
dirty_booleans = { url = "https://github.com/mosteo/dirty_booleans", branch = "main" }
Expand Down
19 changes: 18 additions & 1 deletion alire_common.gpr
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,22 @@ abstract project Alire_Common is
when "disabled" => Style_Check_Switches := ();
end case;

type Any_Experimental_Ada_Features is ("enabled", "disabled");
Experimental_Ada_Features : Any_Experimental_Ada_Features :=
external ("ALIRE_EXPERIMENTAL_ADA_FEATURES", "disabled");
-- These should only be enabled temporarily to help with debugging.
-- Production builds are always checked with these disabled.

Experimental_Ada_Switches := ();
case Experimental_Ada_Features is
when "enabled" => Experimental_Ada_Switches :=
("-gnat2022",
"-gnatx",
"-gnatwJ" -- Disable warnings about obsolescent "()"
);
when "disabled" => Experimental_Ada_Switches := ();
end case;

Ada_Common_Switches :=
( "-gnatW8" -- use UTF-8 Encoding for Source Files
, "-s" -- Recompile if compiler Switches Have Changed
Expand All @@ -58,7 +74,8 @@ abstract project Alire_Common is

-- Enable all warnings
"-gnatwa")
& Style_Check_Switches;
& Style_Check_Switches
& Experimental_Ada_Switches;

for Default_Switches ("C") use ("-g", "-O0", "-Wall");
-- Likewise for C units
Expand Down
2 changes: 1 addition & 1 deletion deps/aaa
19 changes: 19 additions & 0 deletions src/alire/alire-dependencies-states.adb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ with Alire.Roots.Optional;

package body Alire.Dependencies.States is

---------
-- "=" --
---------

overriding function "=" (L, R : Stored_Release) return Boolean
is
use type Milestones.Milestone;
Expand All @@ -20,6 +24,21 @@ package body Alire.Dependencies.States is
L.Element.Origin = R.Element.Origin);
end "=";

---------
-- "=" --
---------

overriding function "=" (L, R : State) return Boolean
is
begin
-- Explicit because the implicit one is reporting spurious diffs (bug?)
return
L.Fulfilled = R.Fulfilled
and then L.Transitivity = R.Transitivity
and then L.Pinning = R.Pinning
and then Dependency (L) = Dependency (R);
end "=";

----------------------
-- Optional_Release --
----------------------
Expand Down
7 changes: 6 additions & 1 deletion src/alire/alire-dependencies-states.ads
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ package Alire.Dependencies.States is

type State (<>) is new Dependency with private;

overriding function "=" (L, R : State) return Boolean;

------------------
-- Constructors --
------------------
Expand Down Expand Up @@ -211,7 +213,7 @@ private
return State;

package Link_Holders is
new AAA.Containers.Indefinite_Holders (Softlink);
new AAA.Containers.Indefinite_Holders (Softlink, User_Pins."=");

type Link_Holder is new Link_Holders.Holder with null record;

Expand Down Expand Up @@ -242,6 +244,9 @@ private
-----------

type State (Name_Len : Natural) is new Dependency (Name_Len) with record
-- NOTE: check "=" implementation if adding fields to this record.
-- There seems to be some trouble with the default "=" operator so its
-- overridden.
Fulfilled : Fulfillment_Data;
Pinning : Pinning_Data;
Transitivity : Transitivities := Unknown;
Expand Down
33 changes: 19 additions & 14 deletions src/alire/alire-errors.adb
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,25 @@ package body Alire.Errors is
declare
Line : constant String := Trim (Lines (I));
begin
Trace.Error ((if I > Lines.First_Index then " " else "")
-- Indentation

& (if I < Lines.Last_Index and Line (Line'Last) = '.'
then Line (Line'First .. Line'Last - 1)
else Line)
-- The error proper, trimming unwanted final '.'

& (if I < Lines.Last_Index
and then Line (Line'Last) /= ':'
then ":"
else "")
-- Trailing ':' except for last line
);
if Line /= "" then
Trace.Error
((if I > Lines.First_Index then " " else "")
-- Indentation

& (if I < Lines.Last_Index and Line (Line'Last) = '.'
then Line (Line'First .. Line'Last - 1)
else Line)
-- The error proper, trimming unwanted final '.'

& (if I < Lines.Last_Index
and then Line (Line'Last) /= ':'
then ":"
else "")
-- Trailing ':' except for last line
);
else
Trace.Error (Line);
end if;
end;
end loop;
end Pretty_Print;
Expand Down
2 changes: 1 addition & 1 deletion src/alire/alire-releases-containers.ads
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ package Alire.Releases.Containers is
Release.Satisfies (Dep));

package Release_Holders
is new AAA.Containers.Indefinite_Holders (Releases.Release);
is new AAA.Containers.Indefinite_Holders (Releases.Release, Releases."=");
subtype Release_H is Release_Holders.Holder;

package Crate_Release_Maps is new Ada.Containers.Indefinite_Ordered_Maps
Expand Down
46 changes: 30 additions & 16 deletions src/alire/alire-roots.adb
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,11 @@ package body Alire.Roots is
Containers.Crate_Name_Sets.Empty_Set)
is

-- Pins may be stored with relative paths so we need to ensure being at
-- the root of the workspace:
CD : Directories.Guard (Directories.Enter (Path (This)))
with Unreferenced;

Top_Root : Root renames This;
Pins_Dir : constant Any_Path := This.Pins_Dir;
Linked : Containers.Crate_Name_Sets.Set;
Expand Down Expand Up @@ -1810,6 +1815,11 @@ package body Alire.Roots is
Allowed : Containers.Crate_Name_Sets.Set :=
Alire.Containers.Crate_Name_Sets.Empty_Set)
is
-- Pins may be stored with relative paths so we need to ensure being at
-- the root of the workspace:
CD : Directories.Guard (Directories.Enter (Path (This)))
with Unreferenced;

Old : constant Solutions.Solution :=
(if This.Has_Lockfile
then This.Solution
Expand Down Expand Up @@ -1839,7 +1849,7 @@ package body Alire.Roots is
begin
-- Early exit when there are no changes

if not Alire.Force and not Diff.Contains_Changes then
if not Alire.Force and then not Diff.Contains_Changes then
if not Needed.Is_Complete then
Trace.Warning
("There are missing dependencies"
Expand All @@ -1850,27 +1860,31 @@ package body Alire.Roots is
-- In case manual changes in manifest do not modify the
-- solution.

if not Silent then
if not Silent and then not Diff.Contains_Changes then
Trace.Info ("Nothing to update.");
end if;

else
else -- Forced or there are changes

-- Show changes and optionally ask user to apply them

if not Interact then
declare
Level : constant Trace.Levels :=
(if Silent then Debug else Info);
begin
Trace.Log
("Dependencies automatically updated as follows:",
Level);
Diff.Print (Level => Level);
end;
elsif not Utils.User_Input.Confirm_Solution_Changes (Diff) then
Trace.Detail ("Update abandoned.");
return;
if Diff.Contains_Changes then
if not Interact then
declare
Level : constant Trace.Levels :=
(if Silent then Debug else Info);
begin
Trace.Log
("Dependencies automatically updated as follows:",
Level);
Diff.Print (Level => Level);
end;
elsif not Utils.User_Input.Confirm_Solution_Changes (Diff) then
Trace.Detail ("Update abandoned.");
return;
end if;
elsif not Silent then
Trace.Info ("Nothing to update.");
end if;

end if;
Expand Down
2 changes: 1 addition & 1 deletion src/alire/alire-user_pins.ads
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ private
case Kind is
when To_Git =>
URL : UString;
Branch : UString; -- Optional
Branch : UString; -- Optional
Commit : UString; -- Optional
Local_Path : Unbounded_Absolute_Path;
-- Empty until the pin is locally deployed
Expand Down

0 comments on commit f850edd

Please sign in to comment.