From f850edd8a3d6108c65b74d2b4748f775bf866ca1 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 31 Jan 2024 18:24:10 +0100 Subject: [PATCH] Fix bugs causing spurious diffs on updates without changes (#1550) * 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 --- alire.toml | 2 +- alire_common.gpr | 19 +++++++++- deps/aaa | 2 +- src/alire/alire-dependencies-states.adb | 19 ++++++++++ src/alire/alire-dependencies-states.ads | 7 +++- src/alire/alire-errors.adb | 33 ++++++++++-------- src/alire/alire-releases-containers.ads | 2 +- src/alire/alire-roots.adb | 46 ++++++++++++++++--------- src/alire/alire-user_pins.ads | 2 +- 9 files changed, 96 insertions(+), 36 deletions(-) diff --git a/alire.toml b/alire.toml index e0d1f913..ea1e48bd 100644 --- a/alire.toml +++ b/alire.toml @@ -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" } diff --git a/alire_common.gpr b/alire_common.gpr index 217159f3..e479c0c3 100644 --- a/alire_common.gpr +++ b/alire_common.gpr @@ -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 @@ -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 diff --git a/deps/aaa b/deps/aaa index ecc38772..dff61d26 160000 --- a/deps/aaa +++ b/deps/aaa @@ -1 +1 @@ -Subproject commit ecc38772bd4a6b469b54c62363766ea1c0e9f912 +Subproject commit dff61d2615cc6332fa6205267bae19b4d044b9da diff --git a/src/alire/alire-dependencies-states.adb b/src/alire/alire-dependencies-states.adb index 44e75ab1..47e923a3 100644 --- a/src/alire/alire-dependencies-states.adb +++ b/src/alire/alire-dependencies-states.adb @@ -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; @@ -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 -- ---------------------- diff --git a/src/alire/alire-dependencies-states.ads b/src/alire/alire-dependencies-states.ads index 3b7b2742..52c74e3b 100644 --- a/src/alire/alire-dependencies-states.ads +++ b/src/alire/alire-dependencies-states.ads @@ -33,6 +33,8 @@ package Alire.Dependencies.States is type State (<>) is new Dependency with private; + overriding function "=" (L, R : State) return Boolean; + ------------------ -- Constructors -- ------------------ @@ -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; @@ -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; diff --git a/src/alire/alire-errors.adb b/src/alire/alire-errors.adb index 7b27a2b0..4913ffe0 100644 --- a/src/alire/alire-errors.adb +++ b/src/alire/alire-errors.adb @@ -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; diff --git a/src/alire/alire-releases-containers.ads b/src/alire/alire-releases-containers.ads index 9a60bbf8..07d841c1 100644 --- a/src/alire/alire-releases-containers.ads +++ b/src/alire/alire-releases-containers.ads @@ -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 diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index a1708d8c..5cdf01b4 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -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; @@ -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 @@ -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" @@ -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; diff --git a/src/alire/alire-user_pins.ads b/src/alire/alire-user_pins.ads index d1469628..ab90332c 100644 --- a/src/alire/alire-user_pins.ads +++ b/src/alire/alire-user_pins.ads @@ -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 -- 2.39.5