Fix updating pins on crates which track Alire-generated files

src/alire/alire-user_pins.adb

@@ -9,6 +9,8 @@ with Alire.VFS;
 
 with AAA.Strings;
 
+with CLIC.User_Input;
+
 with GNAT.OS_Lib;
 
 package body Alire.User_Pins is
@@ -199,7 +201,7 @@ package body Alire.User_Pins is
          Trace.Detail ("Checking out pin " & Utils.TTY.Name (Crate) & " at "
                        & TTY.URL (Destination));
 
-         --  If the fetch URL has been changed, fall back to checkout
+         --  If the fetch URL has been changed, do a fresh 'git clone'.
          --
          --  Note that VCSs.Git.Clone converts the URL to a git-friendly form
          --  with VCSs.Repo, so this is what the output of 'git config' should
@@ -223,11 +225,78 @@ package body Alire.User_Pins is
          Put_Info ("Pulling " & Utils.TTY.Name (Crate)
                    & " branch " & TTY.URL (Branch) & "...");
 
-         if not VCSs.Git.Handler.Update (Destination, Branch).Success then
-            Raise_Checked_Error
-              ("Update of repository at " & TTY.URL (Destination)
-               & " failed, re-run with -vv -d for details");
-         end if;
+         declare
+            use CLIC.User_Input;
+            Result : Outcome := VCSs.Git.Handler.Update (Destination, Branch);
+         begin
+            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.
+               --
+               --  This generally happens if the crate's repo tracks
+               --  Alire-generated files (e.g. those in 'config/'). However, it
+               --  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.
+               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
+               Raise_Checked_Error
+                 ("Update of repository at " & TTY.URL (Destination)
+                  & " failed, re-run with -vv -d for details");
+            end if;
+         end;
       end Update;
 
    begin

src/alire/alire-vcss-git.adb

@@ -292,6 +292,26 @@ package body Alire.VCSs.Git is
          return Alire.Errors.Get (E);
    end Commit_All;
 
+   -------------------------
+   -- Discard_Uncommitted --
+   -------------------------
+
+   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 =>
+         return Alire.Errors.Get (E);
+   end Discard_Uncommitted;
+
    ----------
    -- Push --
    ----------
@@ -611,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");
+
+      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.
 
-      Untracked_File : Natural := 0;
-      Tracked_File   : Natural := 0;
+      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,6 +104,24 @@ 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.
 
+   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;
                   Force  : Boolean := False;
@@ -184,6 +202,9 @@ package Alire.VCSs.Git is
                     return Outcome;
    --  Update and track Branch, if given.
    --
+   --  Does not discard uncommitted changes, so will fail if there are local
+   --  changes which conflict with the update.
+   --
    --  Raises Checked_Error when the repo has multiple remotes configured and
    --  Branch is not the same as the current HEAD.
 

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

@@ -0,0 +1,118 @@
+"""
+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_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()
+replace_in_file(".gitignore", "/config/\n", "")
+init_git_repo(".")
+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}")
+run_alr("build")
+
+# 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 = 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", 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
+# confirmation)
+os.chdir(xxx_path)
+p = run_alr("update")
+assert_match(
+    (
+        ".*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
+)
+
+# Check that the update was successful
+assert_in_file(os.path.join(cached_yyy_path, "test_file"), "This is a new file")
+
+# 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, which is not dirty).
+os.chdir(cached_yyy_path)
+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 = 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("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")
+
+
+print('SUCCESS')

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

@@ -0,0 +1,3 @@
+driver: python-script
+indexes:
+    compiler_only_index: {}