Make confirmation prompt depend on location of dirty files

src/alire/alire-user_pins.adb

@@ -229,20 +229,7 @@ package body Alire.User_Pins is
             use CLIC.User_Input;
             Result : Outcome := VCSs.Git.Handler.Update (Destination, Branch);
          begin
-            if not Result.Success
-              and then VCSs.Git.Handler.Status (Destination) in VCSs.Git.Dirty
-              and then Query
-                (Question =>
-                   "Do you want to discard local uncommitted changes in '"
-                   & Destination
-                   & "'?"
-                   & New_Line
-                   & "These changes were probably generated by Alire, so "
-                   & "select 'Yes' unless you made them yourself.",
-                 Valid    => (Yes | No => True, others => False),
-                 Default  => Yes)
-                = Yes
-            then
+            if not Result.Success then
                --  One reason why the update may fail is if there are
                --  uncommitted changes in the local clone which would conflict
                --  with the update, so we discard such changes and try again.
@@ -252,9 +239,56 @@ package body Alire.User_Pins is
                --  is conceivable the user might have made changes to the
                --  checkout in the 'alire/cache/pins' directory themselves, so
                --  we require user confirmation before discarding anything.
-
-               VCSs.Git.Discard_Uncommitted (Repo => Destination).Assert;
-               Result := VCSs.Git.Handler.Update (Destination, Branch);
+               declare
+                  Paths : constant AAA.Strings.Set :=
+                    VCSs.Git.Handler.Dirty_Files
+                      (Destination, Include_Untracked => True);
+
+                  List_Sep : constant String := New_Line & "  ";
+                  Paths_List : constant String :=
+                    List_Sep & Paths.To_Vector.Flatten (List_Sep);
+
+                  Alire_Generated_Dirs_Only : constant Boolean :=
+                    (for all Path of Paths =>
+                       AAA.Strings.Has_Prefix (Path, "alire/")
+                       or else AAA.Strings.Has_Prefix (Path, "config/"));
+                  --  'git status' yields '/' separated paths, even on Windows.
+
+                  Question : constant String :=
+                    "Updating the pin '"
+                    & Crate.As_String
+                    & "' will discard local uncommitted changes in '"
+                    & Destination
+                    & "' to the following:"
+                    & Paths_List
+                    & New_Line
+                    & (if Alire_Generated_Dirs_Only
+                       then
+                         "These changes affect only Alire's automatically "
+                         & "generated files, which are safe to overwrite."
+                         & New_Line
+                         & "Do you want to proceed?"
+                       else
+                         "These changes include files which were not "
+                         & "automatically generated by Alire."
+                         & New_Line
+                         & "Are you sure you want to proceed?");
+                  Default  : constant Answer_Kind :=
+                    (if Force or else Alire_Generated_Dirs_Only then Yes
+                     else No);
+               begin
+                  if Paths.Length not in 0
+                    and then Query
+                      (Question => Question,
+                       Valid    => (Yes | No => True, others => False),
+                       Default  => Default)
+                      = Yes
+                  then
+                     VCSs.Git.Discard_Uncommitted
+                       (Repo => Destination, Discard_Untracked => True).Assert;
+                     Result := VCSs.Git.Handler.Update (Destination, Branch);
+                  end if;
+               end;
             end if;
 
             if not Result.Success then

src/alire/alire-vcss-git.adb

@@ -296,11 +296,16 @@ package body Alire.VCSs.Git is
    -- Discard_Uncommitted --
    -------------------------
 
-   function Discard_Uncommitted (Repo : Directory_Path) return Outcome
+   function Discard_Uncommitted (Repo : Directory_Path;
+                                 Discard_Untracked : Boolean := False)
+                                 return Outcome
    is
       Guard : Directories.Guard (Directories.Enter (Repo)) with Unreferenced;
    begin
       Run_Git (Empty_Vector & "reset" & "-q" & "--hard" & "HEAD");
+      if Discard_Untracked then
+         Run_Git (Empty_Vector & "clean" & "-fd");
+      end if;
       return Outcome_Success;
    exception
       when E : others =>
@@ -626,41 +631,72 @@ package body Alire.VCSs.Git is
       end;
    end Remote_Commit;
 
-   ------------
-   -- Status --
-   ------------
+   -----------------
+   -- Dirty_Files --
+   -----------------
 
-   function Status (This : VCS;
-                    Repo : Directory_Path)
-                    return States
+   function Dirty_Files (This              : VCS;
+                         Repo              : Directory_Path;
+                         Include_Untracked : Boolean := False)
+                         return AAA.Strings.Set
    is
       Guard  : Directories.Guard (Directories.Enter (Repo)) with Unreferenced;
 
       Output : constant AAA.Strings.Vector :=
-        Run_Git_And_Capture (Empty_Vector & "status" & "--porcelain");
+        Run_Git_And_Capture (Empty_Vector & "status" & "-z" & "--no-renames");
 
-      Untracked_File : Natural := 0;
-      Tracked_File   : Natural := 0;
+      Lines  : constant AAA.Strings.Vector :=
+        (if Output.Length in 0 then Empty_Vector
+         else Split (Output.First_Element, Character'Val (0)));
+      --  'git status -z' splits "lines" on the null character, not newline.
+
+      Paths  : AAA.Strings.Set := Empty_Set;
    begin
+      if Output.Length not in 0 | 1 then
+         --  'git status -z' splits on null character, so should only yield one
+         --  line of output.
+         Raise_Checked_Error
+           ("Unexpected output from 'git status -z': "
+            & Output.Flatten (New_Line & "  "));
+      end if;
 
-      for Line of Output loop
+      for Line of Lines loop
          if Contains (Line, "GNAT-TEMP-") then
             --  Turns out the temporary file we use to capture the output of
             --  "git status" makes git to return a dirty tree. We filter these
             --  out then.
             null;
-         elsif Has_Prefix (Line, "??") then
-            Untracked_File := Untracked_File + 1;
+         elsif Line = "" then
+            --  The output of 'git status -z' includes a trailing null byte,
+            --  so the last element of Lines is an irrelevant empty string.
+            null;
+         elsif not Include_Untracked and then Has_Prefix (Line, "??") then
+            --  Ignore untracked files if Include_Untracked is False.
+            null;
          else
-            Tracked_File := Tracked_File + 1;
+            --  'git status -z' yields lines of the form "XY <path>", where
+            --  "X" and "Y" are single-character status codes.
+            --  We therefore strip the first three characters of each line to
+            --  get the path.
+            Paths.Include (Line (Line'First + 3 .. Line'Last));
          end if;
       end loop;
+      return Paths;
+   end Dirty_Files;
 
-      if Tracked_File /= 0 then
-         --  There are added/modified tracked files
-         return Dirty;
-      else
+   ------------
+   -- Status --
+   ------------
+
+   function Status (This : VCS;
+                    Repo : Directory_Path)
+                    return States
+   is
+   begin
+      if Dirty_Files (This, Repo, Include_Untracked => False).Length in 0 then
          return Clean;
+      else
+         return Dirty;
       end if;
    end Status;
 

src/alire/alire-vcss-git.ads

@@ -104,8 +104,23 @@ package Alire.VCSs.Git is
    --  Add and commit all changes in a given repo; commiter will be set to the
    --  user email stored in our config.
 
-   function Discard_Uncommitted (Repo : Directory_Path) return Outcome;
-   --  Reset all uncommitted changes to tracked files
+   not overriding
+   function Dirty_Files (This              : VCS;
+                         Repo              : Directory_Path;
+                         Include_Untracked : Boolean := False)
+                         return AAA.Strings.Set;
+   --  Return the paths of any files with uncommitted changes.
+   --
+   --  Ignored files are not included. Untracked files are not included unless
+   --  Include_Untracked is True.
+
+   function Discard_Uncommitted (Repo : Directory_Path;
+                                 Discard_Untracked : Boolean := False)
+                                 return Outcome;
+   --  Reset all uncommitted changes to tracked files, and optionally also
+   --  untracked files.
+   --
+   --  Ignored files are not discarded.
 
    function Push (Repo   : Directory_Path;
                   Remote : String;

testsuite/drivers/helpers.py

@@ -198,8 +198,9 @@ def init_git_repo(path):
     assert run(["git", "checkout", "-b", "master"]).returncode == 0
     git_init_user()
 
-    # Workaround for Windows, where somehow we get undeletable files in temps:
-    with open(".gitignore", "wt") as file:
+    # Workaround for Windows, where somehow we get undeletable files in temps
+    # (we use mode 'a' because a '.gitignore' may already be present):
+    with open(".gitignore", "at") as file:
         file.write("*.tmp\n")
 
     head = commit_all(".")

testsuite/tests/pin/branch-update-dirty/test.py

@@ -1,48 +1,50 @@
 """
-Check updating a branch pin when 'config/*' files are tracked.
+Check updating a branch pin when 'config/*' files are tracked or users make
+manual changes to the cached clone.
 """
 
 
 import os
 import re
+import shutil
 import subprocess
 
 from drivers.alr import run_alr, alr_pin, init_local_crate
 from drivers.asserts import assert_match, assert_in_file, assert_not_substring
-from drivers.helpers import git_branch, git_commit_file, init_git_repo
+from drivers.helpers import git_commit_file, init_git_repo, replace_in_file
+
+
+def run(args):
+    p = subprocess.run(args, capture_output=True)
+    p.check_returncode()
+    return p
 
 
 # Create crate yyy, with git tracking the 'config/*' files
 init_local_crate("yyy")
 yyy_path = os.getcwd()
-with open(".gitignore") as f:
-    gitignore_content = f.read()
-gitignore_content = gitignore_content.replace("/config/\n", "")
-with open(".gitignore", "w") as f:
-    f.write(gitignore_content)
+replace_in_file(".gitignore", "/config/\n", "")
 init_git_repo(".")
-default_branch = git_branch()
 os.chdir("..")
 
 # Create and build another crate, with yyy's default branch added as a pin
 init_local_crate()
 xxx_path = os.getcwd()
-alr_pin("yyy", url=f"git+file:{yyy_path}", branch=default_branch)
+alr_pin("yyy", url=f"git+file:{yyy_path}")
 run_alr("build")
 
-# Verify that the cached copy of yyy has a dirty repo (due to changes to the
-# 'config/*' files)
+# Verify that the cached copy of yyy has a dirty repo (due to Alire's changes to
+# the 'config/*' files during the build)
 cached_yyy_path = os.path.join(xxx_path, "alire", "cache", "pins", "yyy")
+gpr_rel_path = os.path.join("config", "yyy_config.gpr")
+gpr_rel_path_pattern = r"config/yyy_config\.gpr" # with '/', as returned by Git
 os.chdir(cached_yyy_path)
-p = subprocess.run(["git", "status"], capture_output=True)
-p.check_returncode()
-assert_match(r".*modified:\s*config/yyy_config\.gpr", p.stdout.decode())
+p = run(["git", "status", "--porcelain"])
+assert_match(rf".* M {gpr_rel_path_pattern}", p.stdout.decode())
 
 # Add commits to yyy's default branch, including a change to a file in 'config/'
 os.chdir(yyy_path)
-git_commit_file(
-    "Change_config", "config/yyy_config.gpr", "This is a new addition\n", "a"
-)
+git_commit_file("Change_config", gpr_rel_path, "This is a new addition\n", "a")
 git_commit_file("Add_test_file", "test_file", "This is a new file\n")
 
 # Check that the dirty repo doesn't prevent updating the pin (subject to user
@@ -51,10 +53,11 @@
 p = run_alr("update")
 assert_match(
     (
-        ".*Do you want to discard local uncommitted changes in '"
-        rf"{re.escape(cached_yyy_path)}'\?"
-        ".*These changes were probably generated by Alire, so select 'Yes' "
-        r"unless you made them yourself\."
+        ".*Updating the pin 'yyy' will discard local uncommitted changes in "
+        f"'{re.escape(cached_yyy_path)}' to the following:"
+        rf".*[\r\n]+  {gpr_rel_path_pattern}"
+        r".*[\r\n]+These changes affect only Alire's automatically generated "
+        r"files, which are safe to overwrite\.[\r\n]+Do you want to proceed\?"
     ),
     p.out
 )
@@ -64,18 +67,51 @@
 
 # Reset the cached clone by only one commit and repeat the above. This time the
 # update should work without user confirmation, as there will be no conflict
-# (only 'test_file' needs to be updated).
+# (only 'test_file' needs to be updated, which is not dirty).
 os.chdir(cached_yyy_path)
-p = subprocess.run(["git", "reset", "--hard", "HEAD~"]).check_returncode()
+run(["git", "reset", "--hard", "HEAD~"])
+shutil.rmtree("config") # So 'alr build' auto-generates the same files as above.
 os.chdir(xxx_path)
 run_alr("build")
 os.chdir(cached_yyy_path)
-p = subprocess.run(["git", "status"], capture_output=True)
-p.check_returncode()
-assert_match(r".*modified:\s*config/yyy_config\.gpr", p.stdout.decode())
+p = run(["git", "status", "--porcelain"])
+assert_match(rf".* M {gpr_rel_path_pattern}", p.stdout.decode())
 os.chdir(xxx_path)
 p = run_alr("update")
-assert_not_substring("Do you want to discard local uncommitted changes", p.out)
+assert_not_substring("will discard local uncommitted changes", p.out)
+assert_in_file(os.path.join(cached_yyy_path, "test_file"), "This is a new file")
+
+# Reset by one commit and repeat the process again, but write to 'test_file' so
+# that it is dirty and there is a conflict. Since this time the change is not in
+# one of Alire's auto-generated subdirectories, the prompt should be different.
+#
+# This also checks that conflicts involving untracked files are treated the same
+# as those with tracked files.
+os.chdir(cached_yyy_path)
+run(["git", "reset", "--hard", "HEAD~"])
+shutil.rmtree("config")
+os.chdir(xxx_path)
+run_alr("build")
+os.chdir(cached_yyy_path)
+with open("test_file", "w") as f:
+    f.write("This file is now dirty.\n")
+p = run(["git", "status", "--porcelain"])
+assert_match(rf".* M {gpr_rel_path_pattern}", p.stdout.decode())
+assert_match(r".*\?\? test_file", p.stdout.decode())
+os.chdir(xxx_path)
+prompt = (
+    ".*Updating the pin 'yyy' will discard local uncommitted changes in "
+    f"'{re.escape(cached_yyy_path)}' to the following:"
+    rf".*[\r\n]+  {gpr_rel_path_pattern}.*[\r\n]+  test_file"
+    r".*[\r\n]+These changes include files which were not automatically "
+    r"generated by Alire\.[\r\n]+Are you sure you want to proceed\?[\r\n]+"
+)
+# The default response should be 'No', so 'alr -n update' will fail.
+p = run_alr("update", complain_on_error=False)
+assert_match(prompt + "Using default: No", p.out)
+# Supplying '--force' should change the default to 'Yes'.
+p = run_alr("-f", "update")
+assert_match(prompt + "Using default: Yes", p.out)
 assert_in_file(os.path.join(cached_yyy_path, "test_file"), "This is a new file")
 
 

testsuite/tests/publish/private-indexes/test.py

@@ -66,9 +66,7 @@ def test(
     os.chdir("remote")
     run_alr("init", "--bin", "xxx")
     os.chdir("xxx")
-    # Adjust the values of maintainers-logins and user.github_login if required
-    if github_user is not None:
-        run(["alr", "settings", "--set", "user.github_login", github_user])
+    # Adjust the value of maintainers-logins if required
     if maint_logins is not None:
         with open("alire.toml", "a") as f:
             f.write(f"maintainers-logins = {maint_logins}\n")
@@ -85,6 +83,9 @@ def test(
         os.makedirs(local_path)
         os.chdir(local_path)
         run(["git", "clone", url, local_path])
+        # Adjust the value of user.github_login if required
+        if github_user is not None:
+            run_alr("settings", "--set", "user.github_login", github_user)
 
         # Run alr
         p = run_alr_interactive(