Skip to content

Commit

Permalink
Warn on suspicious caret use
Browse files Browse the repository at this point in the history
This warning is shown at most once per run, and is an optional blocker during
`alr with` and `alr publish`.

A new `Alire.Warnings` package allows simple emission of warnings only once.
  • Loading branch information
mosteo committed Jan 28, 2021
1 parent 2bb0872 commit eb912ae
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 33 deletions.
64 changes: 62 additions & 2 deletions src/alire/alire-config.ads
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ package Alire.Config with Preelaborate is
-- When set to True, distro will be reported as unknown, and in turn no
-- native package manager will be used.

Warning_Caret : constant Config_Key := "warning.caret";
-- Set to false to disable warnings about caret/tilde use for ^0 deps.

end Keys;

private
Expand Down Expand Up @@ -135,7 +138,64 @@ private

function Image (Val : TOML.TOML_Value) return String;

type String_Access is access String;
Config_Path : String_Access;
function "+" (Source : String) return Ada.Strings.Unbounded.Unbounded_String
renames Ada.Strings.Unbounded.To_Unbounded_String;

Builtins : constant array (Natural range <>) of Builtin_Entry :=
(
(+Keys.User_Name,
Cfg_String,
+("User full name. Used for the authors and " &
"maintainers field of a new crate.")),
(+Keys.User_Email,
Cfg_Email,
+("User email address. Used for the authors and" &
" maintainers field of a new crate.")),
(+Keys.User_Github_Login,
Cfg_GitHub_Login,
+("User GitHub login/username. Used to for the maintainers-logins " &
"field of a new crate.")),

(+Keys.Editor_Cmd,
Cfg_String,
+("Editor command and arguments for editing crate code (alr edit)." &
" The executables and arguments are separated by a single space" &
" character. The token ${GPR_FILE} is replaced by" &
" a path to the project file to open.")),

(+"msys2.do_not_install",
Cfg_Bool,
+("If true, Alire will not try to automatically" &
" install msys2 system package manager. (Windows only)")),

(+"msys2.install_dir",
Cfg_Absolute_Path,
+("Directory where Alire will detect and/or install" &
" msys2 system package manager. (Windows only)")),

(+"auto-gpr-with",
Cfg_Bool,
+("If true, Alire will automatically add/edit a list of 'with' " &
"statements in the root GPR project file based on the " &
"dependencies of the crate.")),

(+Keys.Update_Manually,
Cfg_Bool,
+("If true, Alire will not attempt to update dependencies even after "
& "the manifest is manually edited, or when no valid solution has "
& "been ever computed. All updates have to be manually requested "
& "through `alr update`")),

(+Keys.Distribution_Disable_Detection,
Cfg_Bool,
+("If true, Alire will report an unknown distribution and will not"
& " attempt to use the system package manager.")),

(+Keys.Warning_Caret,
Cfg_Bool,
+("If true, Alire will warn about the use of caret (^) "
& "for pre-1 dependencies."))

);

end Alire.Config;
3 changes: 2 additions & 1 deletion src/alire/alire-properties-licenses.adb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
with Alire.Errors;
with Alire.Warnings;

package body Alire.Properties.Licenses is

Expand Down Expand Up @@ -95,7 +96,7 @@ package body Alire.Properties.Licenses is
Value : constant TOML.TOML_Value := From.Pop;
begin
if Value.Kind = TOML_Array then
Trace.Warning
Warnings.Warn_Once
(From.Message
("Array of license in manifest is deprecated. " &
"License should be a single string containing a " &
Expand Down
7 changes: 6 additions & 1 deletion src/alire/alire-publish.adb
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,8 @@ package body Alire.Publish is
Recommend : Utils.String_Vector; -- Optional
Missing : Utils.String_Vector; -- Mandatory

Caret_Pre_1 : Boolean := False; -- To warn about this

function Tomify (S : String) return String renames TOML_Adapters.Tomify;
begin

Expand Down Expand Up @@ -538,6 +540,8 @@ package body Alire.Publish is
end if;
end loop;

Caret_Pre_1 := Release.Check_Caret_Warning;

if not Missing.Is_Empty then
Ada.Text_IO.New_Line;
Raise_Checked_Error ("Missing required properties: "
Expand All @@ -551,7 +555,8 @@ package body Alire.Publish is
if Utils.User_Input.Query
("Do you want to proceed with this information?",
Valid => (Yes | No => True, others => False),
Default => (if Force or else Recommend.Is_Empty
Default => (if Force or else
(Recommend.Is_Empty and then not Caret_Pre_1)
then Yes
else No)) /= Yes
then
Expand Down
33 changes: 33 additions & 0 deletions src/alire/alire-releases.adb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ with Ada.Directories;
with Ada.Strings.Fixed;
with Ada.Text_IO;

with Alire.Config;
with Alire.Crates;
with Alire.Defaults;
with Alire.Errors;
Expand All @@ -10,6 +11,7 @@ with Alire.Requisites.Booleans;
with Alire.TOML_Expressions;
with Alire.TOML_Load;
with Alire.Utils.YAML;
with Alire.Warnings;

with GNAT.IO; -- To keep preelaborable

Expand Down Expand Up @@ -58,6 +60,37 @@ package body Alire.Releases is
return Enumerate (R.Dependencies.Evaluate (P));
end Flat_Dependencies;

-------------------------
-- Check_Caret_Warning --
-------------------------
-- Warn of ^0.x dependencies that probably should be ~0.x
function Check_Caret_Warning (This : Release) return Boolean is
use Alire.Utils;
Warning_Id : constant String := "caret or tilde";
Newline : constant String := ASCII.LF & " ";
begin
for Dep of This.Flat_Dependencies loop
if Config.Get (Config.Keys.Warning_Caret, Default => True) and then
Utils.Contains (Dep.Versions.Image, "^0")
then
Warnings.Warn_Once
("Possible tilde intended instead of caret for a 0.x version."
& Newline
& "Alire does not change the meaning of caret and tilde"
& " for pre/post-1.0 versions."
& Newline
& "The suspicious dependency is: " & TTY.Version (Dep.Image)
& Newline
& "You can disable this warning by setting the option "
& TTY.Emph (Config.Keys.Warning_Caret) & " to false.",
Warning_Id);
return True;
end if;
end loop;

return False;
end Check_Caret_Warning;

---------------
-- Extending --
---------------
Expand Down
4 changes: 4 additions & 0 deletions src/alire/alire-releases.ads
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ package Alire.Releases is
-- Say if the property exists in any branch of the property tree. Key is
-- case-insensitive.

function Check_Caret_Warning (This : Release) return Boolean;
-- Check if this release contains a ^0.x dependency, and warn about it.
-- Returns whether a warning was emitted.

private

use Semantic_Versioning;
Expand Down
7 changes: 7 additions & 0 deletions src/alire/alire-warnings.adb
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,11 @@ package body Alire.Warnings is
end if;
end Warn_Once;

--------------------
-- Already_Warned --
--------------------

function Already_Warned (ID : String) return Boolean
is (Already_Emitted.Contains (ID));

end Alire.Warnings;
5 changes: 5 additions & 0 deletions src/alire/alire-warnings.ads
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package Alire.Warnings with Preelaborate is

-- TODO: thread-unsafe

procedure Warn_Once (Text : String;
ID : String := "";
Level : Trace.Levels := Trace.Warning);
-- Emit a warning just once. If it has been already seen, do not warn
-- again. ID is used to determine if a warning has already been emitted
-- or, when not given, the actual warning text.

function Already_Warned (ID : String) return Boolean;
-- Says if a warning has been already emitted in the current run

end Alire.Warnings;
29 changes: 2 additions & 27 deletions src/alire/alire-workspace.adb
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,11 @@ with Alire.Paths;
with Alire.Properties.Actions.Executor;
with Alire.Roots;
with Alire.Solutions.Diffs;
with Alire.Utils.TTY;
with Alire.Warnings;

package body Alire.Workspace is

use type Conditional.Dependencies;

-------------------------
-- Check_Caret_Warning --
-------------------------
-- Warn of ^0.x dependencies that probably should be ~0.x
procedure Check_Caret_Warning (Root : Roots.Root) is
use Alire.Utils;
Warning_Id : constant String := "caret or tilde";
begin
for Dep of Root.Release.Flat_Dependencies loop
if Utils.Contains (Dep.Versions.Image, "^0") then
Warnings.Warn_Once
("Possible tilde instead of caret intended for a 0.x version.",
Warning_Id & "1");
Warnings.Warn_Once
("Alire interprets caret and tilde the same way "
& "for both pre/post-1 versions.",
Warning_Id & "2");
Warnings.Warn_Once
("The suspicious dependency is: " & TTY.Version (Dep.Image),
Warning_Id & "3");
end if;
end loop;
end Check_Caret_Warning;

-------------------------
-- Deploy_Dependencies --
-------------------------
Expand Down Expand Up @@ -154,7 +128,8 @@ package body Alire.Workspace is
-- taking advantage that this procedure is called whenever a change
-- to dependencies is happening.

Check_Caret_Warning (Root);
pragma Assert (Root.Release.Check_Caret_Warning or else True);
-- We don't care about the return value here

end Deploy_Dependencies;

Expand Down
23 changes: 21 additions & 2 deletions src/alr/alr-commands-withing.adb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ with Semantic_Versioning.Extended;

package body Alr.Commands.Withing is

package Semver renames Semantic_Versioning;

Switch_URL : constant String := "--use";

procedure Replace_Current
Expand Down Expand Up @@ -145,14 +147,18 @@ package body Alr.Commands.Withing is
procedure Detect_Softlink (Path : String) is
Root : constant Alire.Roots.Optional.Root :=
Alire.Roots.Optional.Detect_Root (Path);
use all type Semver.Point;
begin
if Root.Is_Valid then
if Root.Value.Is_Stored then
-- Add a dependency on ^(detected version) (i.e., safely
-- upgradable)
-- upgradable) or ~(detected version) (if pre-1.0).
Add_Softlink
(Dep_Spec => Root.Value.Release.Name_Str
& "^" & Root.Value.Release.Version.Image,
& (if Semver.Major (Root.Value.Release.Version) = 0
then "~"
else "^")
& Root.Value.Release.Version.Image,
Path => Path);
else
Reportaise_Command_Failed
Expand Down Expand Up @@ -247,8 +253,21 @@ package body Alr.Commands.Withing is

Deps_Diff : constant Alire.Dependencies.Diffs.Diff :=
Alire.Dependencies.Diffs.Between (Old_Deps, New_Deps);

use Alire.Utils.User_Input;
begin

-- First of all, warn about dubious caret

if New_Root.Release.Check_Caret_Warning and then
Query
(Question => "Do you want to continue with that dependency?",
Valid => (Yes | No => True, others => False),
Default => No) = No
then
Reportaise_Command_Failed ("Abandoned by user");
end if;

-- Show changes to apply

Trace.Info ("Requested changes:");
Expand Down

0 comments on commit eb912ae

Please sign in to comment.