diff --git a/src/alire/alire-user_pins.adb b/src/alire/alire-user_pins.adb index 5ae911180..fb3992d26 100644 --- a/src/alire/alire-user_pins.adb +++ b/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 diff --git a/src/alire/alire-vcss-git.adb b/src/alire/alire-vcss-git.adb index b9eea3709..c2fd159b9 100644 --- a/src/alire/alire-vcss-git.adb +++ b/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 ", 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; diff --git a/src/alire/alire-vcss-git.ads b/src/alire/alire-vcss-git.ads index d716211fc..3a0ef639f 100644 --- a/src/alire/alire-vcss-git.ads +++ b/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; diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index 6585bc9ca..b4679d6bf 100644 --- a/testsuite/drivers/helpers.py +++ b/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(".") diff --git a/testsuite/tests/pin/branch-update-dirty/test.py b/testsuite/tests/pin/branch-update-dirty/test.py index b29951c68..407ad4562 100644 --- a/testsuite/tests/pin/branch-update-dirty/test.py +++ b/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") diff --git a/testsuite/tests/publish/private-indexes/test.py b/testsuite/tests/publish/private-indexes/test.py index 0079076a3..05788774a 100644 --- a/testsuite/tests/publish/private-indexes/test.py +++ b/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(