Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bugs causing spurious diffs on updates without changes #1550

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's good to change language version when disabling style checks.
I fear this is going to have unexpected consequences that will make your life harder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it a separate scenario then. In any case all of our checks have checks enabled so it shouldn't slip through.

"-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
Loading