From a2079051a82d37649dc95f690abb92da63548e6f Mon Sep 17 00:00:00 2001 From: Seb M'Caw Date: Fri, 13 Sep 2024 08:05:20 +0000 Subject: [PATCH 1/4] Add support for git repos with multiple remotes configured --- src/alire/alire-publish.adb | 210 +++++++-- src/alire/alire-publish.ads | 8 +- src/alire/alire-vcss-git.adb | 157 +++++-- src/alire/alire-vcss-git.ads | 66 ++- .../tests/publish/multiple-remotes/test.py | 403 ++++++++++++++++++ .../tests/publish/multiple-remotes/test.yaml | 1 + 6 files changed, 742 insertions(+), 103 deletions(-) create mode 100644 testsuite/tests/publish/multiple-remotes/test.py create mode 100644 testsuite/tests/publish/multiple-remotes/test.yaml diff --git a/src/alire/alire-publish.adb b/src/alire/alire-publish.adb index b62352360..6443b7e51 100644 --- a/src/alire/alire-publish.adb +++ b/src/alire/alire-publish.adb @@ -196,29 +196,38 @@ package body Alire.Publish is end if; end Git_Error; + -------------------------- + -- Require_Confirmation -- + -------------------------- + -- Prompt the user to confirm they wish to proceed. + -- + -- Raises a Checked_Error with message "Abandoned by user" if they don't. + procedure Require_Confirmation (Prompt : String; + Default_To_Yes : Boolean := True) + is + use CLIC.User_Input; + begin + Ada.Text_IO.New_Line; + if Query (Prompt, + Valid => (Yes | No => True, others => False), + Default => (if Default_To_Yes then Yes else No)) + /= Yes + then + Raise_Checked_Error ("Abandoned by user"); + end if; + end Require_Confirmation; + --------------------- -- Check_Git_Clean -- --------------------- - -- Check that the repo is clean. If we need it only for generating an - -- archive, that is enough; otherwise, check that we are in sync with - -- the remote to which the origin will point to. - procedure Check_Git_Clean (Path : Any_Path; For_Archiving : Boolean) is + -- Check that the repo is clean (no uncommited changes to tracked files). + procedure Check_Git_Clean (Path : Any_Path) is use all type VCSs.Git.States; Git : constant VCSs.Git.VCS := VCSs.Git.Handler; begin case Git.Status (Path) is - when No_Remote => - if For_Archiving then - Put_Success ("Local repository is clean (without remote)."); - else - Git_Error ("No remote configured", Path); - end if; when Clean => Put_Success ("Local repository is clean."); - when Ahead => - Git_Error ("Your branch is ahead of remote" & ASCII.LF & - "Please push local commits to the remote branch.", - Path); when Dirty => Git_Error (TTY.Emph ("git status") & " You have unstaged changes. " & @@ -233,8 +242,6 @@ package body Alire.Publish is -- Checks the presence of recommended/mandatory fields in the release procedure Check_Release (Release : Releases.Release; Context : in out Data) is - use CLIC.User_Input; - Recommend : AAA.Strings.Vector; -- Optional Missing : AAA.Strings.Vector; -- Mandatory @@ -352,19 +359,11 @@ package body Alire.Publish is -- Final confirmation. We default to Yes if no recommended missing or -- Force. - Ada.Text_IO.New_Line; - if Query + Require_Confirmation ("Do you want to proceed with this information?", - Valid => (Yes | No => True, others => False), - Default => (if Force or else - (Recommend.Is_Empty - and then not Caret_Pre_1 - and then not Dev_Version) - then Yes - else No)) /= Yes - then - Raise_Checked_Error ("Abandoned by user"); - end if; + Default_To_Yes => Force or else (Recommend.Is_Empty + and then not Caret_Pre_1 + and then not Dev_Version)); end Check_Release; ----------------- @@ -755,7 +754,7 @@ package body Alire.Publish is begin if Is_Repo then - Check_Git_Clean (Base_Path (Context), For_Archiving => True); + Check_Git_Clean (Base_Path (Context)); else Trace.Warning ("Not in a git repository, assuming plain sources."); end if; @@ -1109,6 +1108,8 @@ package body Alire.Publish is Root : Roots.Optional.Root := Roots.Optional.Search_Root (Path); Git : constant VCSs.Git.VCS := VCSs.Git.Handler; + Use_Head : constant Boolean := Revision = "" or else Revision = "HEAD"; + Subdir : Unbounded_Relative_Path; -- In case we are publishing a nested crate (monorepo), its relative -- path in regard to the git worktree will be stored here by @@ -1149,6 +1150,128 @@ package body Alire.Publish is end if; end Check_Nested_Crate; + ------------------ + -- Infer_Remote -- + ------------------ + + function Infer_Remote (Root_Path : Absolute_Path; + Commit : String) + return String; + -- Return the name of the remote to use as the published origin. + -- + -- Raises Checked_Error if the commit is not on any remote, or is on + -- multiple remotes and the correct choice is not discernable (i.e. if + -- Revision was not specified in the form of a branch which is + -- configured to track a remote). + -- + -- User confirmation is required if not one of 2 simple cases: + -- - Revision refers to a branch which tracks (and is synchronised + -- with) a remote branch. + -- - The repo has exactly one remote configured, and Revision refers + -- to a commit (via a tag, hash or detached HEAD) which is present + -- thereon. + + function Infer_Remote (Root_Path : Absolute_Path; + Commit : String) + return String + is + use all type VCSs.Git.Branch_States; + + Branch : constant String := + (if Use_Head then Git.Branch (Root_Path) else Revision); + Branch_Status : VCSs.Git.Branch_States; + Branch_Remote : constant String := + Git.Branch_Remote (Root_Path, Branch, Branch_Status); + -- We are also using this to detect if Revision refers to something + -- other than a branch (i.e. a commit or tag), in which case + -- Branch_Status will be No_Branch. + + Repo_Remote : constant String := + Git.Repo_Remote (Root_Path, Checked => False); + Commit_Remotes : constant AAA.Strings.Set := + Git.Commit_Remotes (Root_Path, Commit); + begin + -- A branch which doesn't track a remote branch is treated as + -- equivalent to its current head commit, subject to user + -- confirmation. + if Branch_Status in No_Remote then + Require_Confirmation + ("The specified branch is not configured to track a remote." + & New_Line + & "Do you want to attempt to publish its head commit anyway?"); + end if; + + case Branch_Status is + when No_Branch | No_Remote => + -- Revision refers to a commit, not a branch. + if Commit_Remotes.Length in 0 then + -- Commit is not present on any remote, so can't be + -- published. + Ada.Text_IO.New_Line; + Git_Error + ("The specified commit is not present on any configured " + & "remote.", + Root_Path); + raise Program_Error with "unreachable code"; + elsif Repo_Remote /= "" then + -- Commit is present on the repo's only remote. + return Repo_Remote; + elsif Commit_Remotes.Length in 1 then + -- Commit is only present on one remote, but others are + -- configured, so check that this is the one the user + -- intended to publish. + Require_Confirmation + ("The repository has multiple remotes configured, but the " + & "specified commit is only present on the remote '" + & Commit_Remotes.First_Element + & "'." + & New_Line + & "The published manifest will therefore use the origin '" + & Git.Remote_URL (Root_Path, Commit_Remotes.First_Element) + & "'." + & New_Line + & "Is this correct?"); + return Commit_Remotes.First_Element; + else + -- Commit is present on multiple remotes, so the user will + -- need to specify explicitly. + Ada.Text_IO.New_Line; + Git_Error + ("The specified commit is present on multiple remotes" + & New_Line + & "Please use 'alr publish ' to " + & "resolve this ambiguity.", + Root_Path); + raise Program_Error with "unreachable code"; + end if; + + when Ahead => + -- 'git push' is required to publish the current head + Ada.Text_IO.New_Line; + Git_Error + ("Your branch is ahead of remote" + & New_Line + & "Please push local commits to the remote branch.", + Root_Path); + raise Program_Error with "unreachable code"; + + when Behind => + -- The local branch is behind the remote, so check the user + -- isn't mistakenly publishing an outdated version + Require_Confirmation + ("There are commits on the remote branch which could be " + & "retrieved with 'git pull'." + & New_Line + & "Are you sure you want to publish without them?"); + return Branch_Remote; + + when Synced => + -- The local branch is synchronised with a remote branch, so + -- no user confirmation is required. + return Branch_Remote; + end case; + end Infer_Remote; + begin -- Early report and exit if there's any trouble with the supplied path @@ -1164,20 +1287,27 @@ package body Alire.Publish is Git_Error ("no git repository found", Root_Path); end if; + -- Do not continue if no remote is configured + + if VCSs.Git.Remotes (Root_Path).Length in 0 then + Git_Error ("No remote configured.", Root_Path); + end if; + -- Do not continue if the local repo is dirty - Check_Git_Clean (Root_Path, For_Archiving => False); + Check_Git_Clean (Root_Path); -- If the Git root does not match the path to the manifest, we are -- seeing a crate in a subdir, so we go to monorepo mode. Check_Nested_Crate (Root_Path); - -- If not given a revision, check the local manifest contents - -- already. No matter what, it will be checked again on the - -- deployed sources step. + -- If the relevant commit is already checked out, check the local + -- manifest contents now to catch any issues as early as possible. + -- No matter what, it will be checked again on the deployed sources + -- step. - if Revision = "" or else Revision = "HEAD" then + if Use_Head then declare Tmp_Context : Data := (Options => Options, others => <>); begin @@ -1190,9 +1320,9 @@ package body Alire.Publish is declare Commit : constant String := Git.Revision_Commit (Root_Path, - (if Revision /= "" - then Revision - else "HEAD")); + (if Use_Head + then "HEAD" + else Revision)); begin if Commit /= "" then Put_Success ("Revision exists in local repository (" @@ -1203,10 +1333,10 @@ package body Alire.Publish is end if; declare + Remote : constant String := Infer_Remote (Root_Path, Commit); + Raw_URL : constant String := - Git.Fetch_URL - (Root_Path, - Origin => Git.Remote (Root_Path)); + Git.Fetch_URL (Root_Path, Remote); -- The one reported by the repo, in its public form Fetch_URL : constant String := diff --git a/src/alire/alire-publish.ads b/src/alire/alire-publish.ads index 9c892a4c9..35e749a76 100644 --- a/src/alire/alire-publish.ads +++ b/src/alire/alire-publish.ads @@ -26,9 +26,11 @@ package Alire.Publish is Revision : String := "HEAD"; Options : All_Options := New_Options) with Pre => URI.Scheme (Path) in URI.File_Schemes; - -- Check that given Path is an up-to-date git repo. If so, proceed with - -- remote repo verification. If no revision given use the HEAD commit, - -- otherwise use the revision (tag, branch, commit) commit. + -- Check that given Path is a git repo with a remote configured. If so, + -- check that Revision (tag, branch, commit) is suitable for publishing, + -- then proceed using Remote_Origin. + -- + -- If Revision is "" or "HEAD", use the repo's current HEAD. procedure Remote_Origin (URL : Alire.URL; Commit : String := ""; diff --git a/src/alire/alire-vcss-git.adb b/src/alire/alire-vcss-git.adb index f819ac365..40bf320b4 100644 --- a/src/alire/alire-vcss-git.adb +++ b/src/alire/alire-vcss-git.adb @@ -1,4 +1,5 @@ with Ada.Directories; +with Ada.Containers; with Alire.Directories; with Alire.OS_Lib.Subprocess; @@ -120,6 +121,71 @@ package body Alire.VCSs.Git is return Output; end Branches; + function Branch_Remote (This : VCS; + Path : Directory_Path; + Branch : String; + Status : out Branch_States) + return String + is + use Ada.Containers; + Guard : Directories.Guard (Directories.Enter (Path)) with Unreferenced; + Ref : constant String := "refs/heads/" & Branch; + Output : constant AAA.Strings.Vector := + Run_Git_And_Capture + (Empty_Vector + & "for-each-ref" + -- We need the name of the remote and the name of the remote branch + -- thereon which the local branch tracks (normally the same as the + -- local branch, but not always). + -- We use ":" as a separator, as it is not permitted in refs or + -- remote names. + & "--format=%(upstream:remotename):%(upstream)" + & Ref); + begin + if Output.Length = 0 then + Status := No_Branch; + return ""; + elsif Output.Length > 1 then + -- Git should prevent branch "a/b" coexisting with "a", so this + -- case (where there are multiple matches of the form + -- 'refs/heads//') should not be possible if + -- Branch exists. + Status := No_Branch; + return ""; + elsif Output.First_Element = ":" then + -- The branch exists, but doesn't track a remote (both + -- "%(upstream:remotename)" and "%(upstream)" are the empty + -- string, so only the separator is returned) + Status := No_Remote; + return ""; + elsif not Contains (Output.First_Element, ":") then + -- The separator is missing for some reason + raise Program_Error + with "Git gave output inconsistent with specified format"; + else + declare + Remote_Branch_Ref : constant String := + Tail (Output.First_Element, ":"); + Commits_Ahead : constant AAA.Strings.Vector := + Run_Git_And_Capture + (Empty_Vector + & "rev-list" + & String'(Remote_Branch_Ref & ".." & Branch)); + Commits_Behind : constant AAA.Strings.Vector := + Run_Git_And_Capture + (Empty_Vector + & "rev-list" + & String'(Branch & ".." & Remote_Branch_Ref)); + begin + Status := + (if Commits_Ahead.Length /= 0 then Ahead + elsif Commits_Behind.Length /= 0 then Behind + else Synced); + return Head (Output.First_Element, ":"); + end; + end if; + end Branch_Remote; + ----------- -- Clone -- ----------- @@ -257,8 +323,7 @@ package body Alire.VCSs.Git is else -- Create a temporary remote with our credentials and use it to push declare - Old : constant URL := - Handler.Remote_URL (Repo, Handler.Remote (Repo)); + Old : constant URL := Handler.Remote_URL (Repo, Remote); Writurl : constant URL := Replace (Old, "//", "//" & User_Info.User_GitHub_Login @@ -393,29 +458,57 @@ package body Alire.VCSs.Git is Err_To_Out => True) = 0; end Is_Repository; - ------------ - -- Remote -- - ------------ + -------------------- + -- Commit_Remotes -- + -------------------- + + function Commit_Remotes (This : VCS; + Path : Directory_Path; + Commit : Git_Commit) + return AAA.Strings.Set is + pragma Unreferenced (This); + Guard : Directories.Guard (Directories.Enter (Path)) with Unreferenced; + Output : constant AAA.Strings.Vector := Run_Git_And_Capture + (Empty_Vector + & "for-each-ref" + & "--format=%(refname:rstrip=-3)" + & String'("--contains=" & Commit) + & "refs/remotes/"); + Result : AAA.Strings.Set := Empty_Set; + begin + for Line of Output loop + -- We want to return just the remote name, without "refs/remotes/". + Assert (Has_Prefix (Line, "refs/remotes/"), "Unexpected Git output"); + Result.Include (Tail (Line, "refs/remotes/")); + end loop; + return Result; + end Commit_Remotes; - function Remote (This : VCS; - Path : Directory_Path; - Checked : Boolean := True) - return String is + ----------------- + -- Repo_Remote -- + ----------------- + + function Repo_Remote (This : VCS; + Path : Directory_Path; + Checked : Boolean := True) + return String is pragma Unreferenced (This); Guard : Directories.Guard (Directories.Enter (Path)) with Unreferenced; Output : constant AAA.Strings.Vector := Run_Git_And_Capture (Empty_Vector & "remote"); begin - if Output.Is_Empty then - if Checked then + if Output.Length in 1 then + return Output.First_Element; + else + if Checked and then Output.Is_Empty then Raise_Checked_Error ("No remote is configured"); + elsif Checked then + Raise_Checked_Error ("Multiple remotes are configured"); else return ""; end if; - else - return Output.First_Element; end if; - end Remote; + end Repo_Remote; ---------------- -- Remote_URL -- @@ -525,19 +618,14 @@ package body Alire.VCSs.Git is is Guard : Directories.Guard (Directories.Enter (Repo)) with Unreferenced; - -- Out_1 should be portable. Out_2 is used as last resort; I believe - -- git is not localized so it should always work but since it relies on - -- human output it might break at any time I guess. Worst case, we would - -- report an 'Ahead' as 'Dirty'. - - Out_1 : constant AAA.Strings.Vector := + Output : constant AAA.Strings.Vector := Run_Git_And_Capture (Empty_Vector & "status" & "--porcelain"); Untracked_File : Natural := 0; Tracked_File : Natural := 0; begin - for Line of Out_1 loop + for Line of Output 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 @@ -554,30 +642,7 @@ package body Alire.VCSs.Git is -- There are added/modified tracked files return Dirty; else - -- Retrieve revisions from remote branch tip up to our local HEAD. If - -- not empty, we are locally ahead. - declare - Branch : constant String := This.Branch (Repo); - Remote : constant String := This.Remote (Repo, Checked => False); - begin - if Remote = "" then - return No_Remote; - elsif (for all B of Branches (Repo, Local => False) => - B /= Remote & "/" & Branch) - then -- The branch doesn't even exist remotely - return Ahead; - elsif Run_Git_And_Capture - (Empty_Vector - & "rev-list" - & String'(Remote & "/" & Branch - & "..HEAD")).Is_Empty - then - return Clean; - else - -- At least one local commit not pushed to the remote - return Ahead; - end if; - end; + return Clean; end if; end Status; @@ -646,7 +711,7 @@ package body Alire.VCSs.Git is Run_Git (Empty_Vector & "checkout" - & String'(This.Remote (Repo) & "/" & Branch) + & String'(This.Repo_Remote (Repo) & "/" & Branch) & "-B" & Branch & Extra diff --git a/src/alire/alire-vcss-git.ads b/src/alire/alire-vcss-git.ads index da4d5d526..cfb06a435 100644 --- a/src/alire/alire-vcss-git.ads +++ b/src/alire/alire-vcss-git.ads @@ -37,7 +37,9 @@ package Alire.VCSs.Git is function Branch (This : VCS; Path : Directory_Path) return String; - -- Returns the branch name of the repo checked out at Path. + -- Return the name of the branch currently checked out in the repo at Path. + -- + -- This may have the form "(HEAD detached at )". function Branches (Repo : Directory_Path; Local : Boolean := True; @@ -45,6 +47,25 @@ package Alire.VCSs.Git is return AAA.Strings.Vector; -- List all known branches (without going on-line) + type Branch_States is + (No_Branch, -- The specified branch doesn't exist + No_Remote, -- Local branch is not configured to track any remote + Synced, -- Local and remote branches are synchronished + Ahead, -- Local branch has commit(s) not present on remote branch + Behind); -- Not Ahead, and remote branch has commit(s) not on local + + not overriding + function Branch_Remote (This : VCS; + Path : Directory_Path; + Branch : String; + Status : out Branch_States) + return String; + -- Retrieve the name of the remote tracked by a specified branch (usually + -- "origin"), and determine whether the local and remote branches are + -- synchronised. + -- + -- Returns "" when Status is No_Branch or No_Remote. + overriding function Clone (This : VCS; From : URL; @@ -90,8 +111,9 @@ package Alire.VCSs.Git is Token : String := "") return Outcome; -- Push to the remote. If Create, use "-u ". If an -- Auth Token is given, a temporary remote that includes the token will be - -- created and removed for the push; the local branch will be set to track - -- the original remote afterwards. + -- created and removed for the push; in this case, there will be no change + -- to the local branch unless Create is True, in which case any configured + -- upstream will be unset. not overriding function Remote_Commit (This : VCS; @@ -105,8 +127,10 @@ package Alire.VCSs.Git is Repo : Directory_Path; Rev : String) return String; - -- Returns the commit for a revision, if the repository and revision - -- (commit, tag, branch...) exist. Should be a no-op for a commit. + -- Returns the commit for a revision (commit, tag, branch...), if the + -- repository and revision exist. Returns "" otherwise. + -- + -- Should be a no-op for a commit. not overriding function Is_Detached (This : VCS; @@ -119,12 +143,23 @@ package Alire.VCSs.Git is -- Check if a repo exists at Path not overriding - function Remote (This : VCS; - Path : Directory_Path; - Checked : Boolean := True) - return String; - -- Retrieve current remote name (usually "origin"). If checked, raise - -- Checked_Error when no remote configured. Otherwise, return ""; + function Commit_Remotes (This : VCS; + Path : Directory_Path; + Commit : Git_Commit) + return AAA.Strings.Set; + -- Retrieve the names of all remotes on which Commit can be found. + -- + -- Only searches the current remote-tracking branches, so this information + -- may be out of date without a preceeding 'git fetch'. + + not overriding + function Repo_Remote (This : VCS; + Path : Directory_Path; + Checked : Boolean := True) + return String; + -- Retrieve the name of the only remote configured for the repository + -- (usually "origin"). If Checked, raise Checked_Error when zero or + -- multiple remotes are configured. Otherwise, return "". not overriding function Remote_URL (This : VCS; @@ -148,17 +183,20 @@ package Alire.VCSs.Git is Branch : String) return Outcome; -- Update and track Branch, if given. + -- + -- Raises Checked_Error when the repo has multiple remotes configured and + -- Branch is not the same as the current HEAD. type States is (Dirty, -- Uncommitted local changes - No_Remote, -- Clean, no remote configured - Clean, -- Clean, up to date with remote - Ahead); -- Clean, ahead of remote (needs pull) + Clean); -- Clean -- States we are interested in for publishing not overriding function Status (This : VCS; Repo : Directory_Path) return States; + -- Return whether the repo contains uncommitted changes to tracked files. + -- -- Note that untracked files don't cause a Dirty result! not overriding diff --git a/testsuite/tests/publish/multiple-remotes/test.py b/testsuite/tests/publish/multiple-remotes/test.py new file mode 100644 index 000000000..b9ea5f040 --- /dev/null +++ b/testsuite/tests/publish/multiple-remotes/test.py @@ -0,0 +1,403 @@ +""" +Check publishing from a local repo with multiple remotes configured. +""" + + +import os +import re +import subprocess + +from drivers.alr import run_alr, init_local_crate +from drivers.asserts import assert_match +from drivers.helpers import git_branch, git_head, init_git_repo + + +TEST_ROOT_DIR = os.getcwd() + +COMMIT_CONFIRM_PROMPT = ( + r".*The specified branch is not configured to track a remote\.(\r\n|\r|\n)" + r"Do you want to attempt to publish its head commit anyway\?" +) +COMMIT_NOT_PRESENT_MESSAGE = ( + r".*The specified commit is not present on any configured remote\." +) +URL_CONFIRM_PROMPT_TEMPLATE = ( + r".*The repository has multiple remotes configured, but the specified " + r"commit is only present on the remote 'remote{n}'\.(\r\n|\r|\n)" + r"The published manifest will therefore use the origin " + rf"'{re.escape(TEST_ROOT_DIR + os.path.sep)}remote{{n}}'\.(\r\n|\r|\n)" + r"Is this correct\?" +) +AMBIGUOUS_REMOTE_MESSAGE = ( + r".*The specified commit is present on multiple remotes.*Please use " + r"'alr publish ' to resolve this ambiguity\." +) +UNSTAGED_CHANGES_MESSAGE = ( + r".*You have unstaged changes\. Please commit or stash them\." +) +AHEAD_OF_REMOTE_MESSAGE = ( + r".*Your branch is ahead of remote.*Please push local commits to the " + r"remote branch\." +) +BEHIND_REMOTE_CONFIRM_PROMPT = ( + r".*There are commits on the remote branch which could be retrieved with " + r"'git pull'\.(\r\n|\r|\n)Are you sure you want to publish without them\?" +) + + +def run(*args, **kwargs): + subprocess.run(*args, **kwargs).check_returncode() + +def commit_file(commit_name: str, path: str, content: str): + """ + Create a new file with the specified content and `git commit` it. + + Also returns the commit's hash and attaches the tag `f"tag_{commit_name}"` + thereto. + """ + with open(path, "x") as f: + f.write(content) + run(["git", "add", path]) + run(["git", "commit", "-m", f"Commit {commit_name}"]) + run(["git", "tag", f"tag_{commit_name}"]) + return git_head() + +def test_publishing( + extra_args: list[str], + remote: int, + commit_id: str, + output_pattern: str | None = None, +): + """ + Run `alr --force publish --skip-submit` with the specified additional args, + and assert that it succeeds (optionally asserting the output matches a regex + pattern) and that the resulting manifest points to the specified remote + and commit. + """ + p = run_alr("--force", "publish", "--skip-submit", *extra_args, quiet=False) + manifest_path = os.path.join( + TEST_ROOT_DIR, "xxx", "alire", "releases", "xxx-0.1.0-dev.toml" + ) + assert_match( + ( + ".*Your index manifest file has been generated at " + + re.escape(manifest_path) + ), + p.out + ) + if output_pattern is not None: + assert_match(output_pattern, p.out) + with open(manifest_path) as f: + manifest = f.read() + assert_match(rf'.*url = "git\+file:.*remote{remote}"', manifest) + assert_match(rf'.*commit = "{commit_id}"', manifest) + +def check_publishing_fails(extra_args: list[str], error_pattern: str | None): + """ + Run `alr --force publish --skip-submit` with the specified additional args, + and assert that it fails (optionally asserting the output matches a regex + pattern). + """ + p = run_alr( + "--force", "publish", "--skip-submit", *extra_args, + quiet=False, complain_on_error=False + ) + if error_pattern is not None: + assert_match(error_pattern, p.out) + + +# Create two empty 'remote' repositories. +os.mkdir("remote1") +run(["git", "init", "--bare", "remote1"]) +remote1_path = os.path.join(TEST_ROOT_DIR, "remote1") +os.mkdir("remote2") +run(["git", "init", "--bare", "remote2"]) +remote2_path = os.path.join(TEST_ROOT_DIR, "remote2") +# Create a 'local' repo containing a crate. +init_local_crate(with_maintainer_login=True) +initial_commit = init_git_repo(".") +default_branch = git_branch() +# Create a new branch. +run(["git", "checkout", "-b", "other_branch"]) +# Add various commits (and tags) to both branches, including a merge commit. +# +# The repo history will be: +# A - B - C - D <--- default branch +# \ / +# E - F <--- other_branch +commit_A = initial_commit +run(["git", "tag", "tag_A"]) +commit_E = commit_file( + commit_name="E", + path="test_file_0", + content="This file will be merged so that it is on both branches.\n" +) +run(["git", "checkout", default_branch]) +commit_B = commit_file( + commit_name="B", + path="test_file_1", + content="This file is only on the default branch.\n" +) +run(["git", "merge", "other_branch"]) +commit_C = git_head() +run(["git", "tag", "tag_C"]) +commit_D = commit_file( + commit_name="D", + path="test_file_2", + content="This file is also only on the default branch.\n" +) +run(["git", "checkout", "other_branch"]) +commit_F = commit_file( + commit_name="F", + path="test_file_3", + content="This file is only on other_branch.\n" +) +# Add branches at the same locations but which are not configured to track +# either remote. +run(["git", "branch", "other_branch_local"]) +run(["git", "checkout", default_branch]) +run(["git", "branch", "default_branch_local"]) + + +# Check `alr publish` gives an appropriate error with no remotes configured. +check_publishing_fails(extra_args=[], error_pattern=".*No remote configured") + + +# Set the default branch to track remote1. +run(["git", "remote", "add", "remote1", remote1_path]) +run(["git", "push", "--set-upstream", "remote1", default_branch]) + +# Verify `alr publish` still fails with other_branch checked out but succeeds +# with the default branch. +run(["git", "checkout", "other_branch"]) +check_publishing_fails( + extra_args=[], + error_pattern=COMMIT_CONFIRM_PROMPT + COMMIT_NOT_PRESENT_MESSAGE +) +run(["git", "checkout", default_branch]) +test_publishing(extra_args=[], remote=1, commit_id=commit_D) +# Verify the same results with the form `alr publish . `. +check_publishing_fails( + extra_args=[".", "other_branch"], + error_pattern=COMMIT_CONFIRM_PROMPT + COMMIT_NOT_PRESENT_MESSAGE +) +test_publishing(extra_args=[".", default_branch], remote=1, commit_id=commit_D) +# Verify publishing default_branch_local also succeeds, subject to user +# confirmation. +test_publishing( + extra_args=[".", "default_branch_local"], + remote=1, + commit_id=commit_D, + output_pattern=COMMIT_CONFIRM_PROMPT +) +run(["git", "checkout", "default_branch_local"]) +test_publishing( + extra_args=[], + remote=1, + commit_id=commit_D, + output_pattern=COMMIT_CONFIRM_PROMPT +) + +# Verify `alr publish . ` succeeds for all commits present on the remote +# and fails for commit_F (which has not been pushed). +commits = [commit_A, commit_B, commit_C, commit_D, commit_E] +tags = ["tag_A", "tag_B", "tag_C", "tag_D", "tag_E" ] +for commit in commits: + test_publishing(extra_args=[".", commit], remote=1, commit_id=commit) +check_publishing_fails( + extra_args=[".", commit_F], error_pattern=COMMIT_NOT_PRESENT_MESSAGE +) +# Verify the same results for tags. +for commit, tag in zip(commits, tags): + test_publishing(extra_args=[".", tag], remote=1, commit_id=commit) +check_publishing_fails( + extra_args=[".", "tag_F"], error_pattern=COMMIT_NOT_PRESENT_MESSAGE +) + + +# Set other_branch to track remote2, so the repo now has two remotes +run(["git", "remote", "add", "remote2", remote2_path]) +run(["git", "push", "--set-upstream", "remote2", "other_branch"]) + +# Verify publishing with other_branch checked out now yields a manifest pointing +# to remote2_path. +run(["git", "checkout", "other_branch"]) +test_publishing(extra_args=[], remote=2, commit_id=commit_F) +# Check out the default branch, and verify publishing still yields a manifest +# pointing to remote1_path. +run(["git", "checkout", default_branch]) +test_publishing(extra_args=[], remote=1, commit_id=commit_D) +# Verify the same results with the form `alr publish . ` +test_publishing(extra_args=[".", "other_branch"], remote=2, commit_id=commit_F) +test_publishing(extra_args=[".", default_branch], remote=1, commit_id=commit_D) + +# Verify publishing a commit (or tag) which is on only one remote succeeds, +# subject to user confirmation. +commits = [commit_B, commit_C, commit_D, commit_F] +tags = ["tag_B", "tag_C", "tag_D", "tag_F" ] +remotes = [1, 1, 1, 2 ] +for commit, tag, remote in zip(commits, tags, remotes): + for ref in [commit, tag]: + test_publishing( + extra_args=[".", ref], + remote=remote, + commit_id=commit, + output_pattern=URL_CONFIRM_PROMPT_TEMPLATE.format(n=remote) + ) +# Verify publishing a commit (or tag) which is on multiple remotes fails, with +# instructions on resolving the ambiguity. Also verify that those instructions +# are correct (i.e. that `alr publish ` succeeds). +for ref in [commit_E, "tag_E"]: + check_publishing_fails( + extra_args=[".", ref], error_pattern=AMBIGUOUS_REMOTE_MESSAGE, + ) + test_publishing([f"git+file:{remote1_path}", commit_E], 1, commit_E) + test_publishing([f"git+file:{remote2_path}", commit_E], 2, commit_E) + +# Verify publishing the branches which are not pushed to the remote succeeds, +# subject to user confirmation. +cases = [ + ("default_branch_local", commit_D, 1), ("other_branch_local", commit_F, 2) +] +for branch, commit, remote in cases: + test_publishing( + extra_args=[".", branch], + remote=remote, + commit_id=commit, + output_pattern=( + COMMIT_CONFIRM_PROMPT + URL_CONFIRM_PROMPT_TEMPLATE.format(n=remote) + ) + ) +# Verify `alr publish` with the branch checked out yields the same result. +for branch, commit, remote in cases: + run(["git", "checkout", branch]) + test_publishing( + extra_args=[], + remote=remote, + commit_id=commit, + output_pattern=( + COMMIT_CONFIRM_PROMPT + URL_CONFIRM_PROMPT_TEMPLATE.format(n=remote) + ) + ) + +# Add a new branch which points to commit E (present on both remotes), and check +# publishing fails with both a prompt that the branch isn't tracking a remote +# and instructions on resolving the ambiguity. +run(["git", "branch", "ambiguous_branch", commit_E]) +check_publishing_fails( + extra_args=[".", "ambiguous_branch"], + error_pattern=COMMIT_CONFIRM_PROMPT + AMBIGUOUS_REMOTE_MESSAGE, +) +run(["git", "checkout", "ambiguous_branch"]) +check_publishing_fails( + extra_args=[], + error_pattern=COMMIT_CONFIRM_PROMPT + AMBIGUOUS_REMOTE_MESSAGE, +) + +# Configure ambiguous_branch to track one of the remotes, and verify publishing +# now succeeds and uses this remote as the origin. +run(["git", "push", "--set-upstream", "remote2", "ambiguous_branch"]) +test_publishing(extra_args=[], remote=2, commit_id=commit_E) +run(["git", "checkout", default_branch]) +test_publishing( + extra_args=[".", "ambiguous_branch"], remote=2, commit_id=commit_E +) + +# Verify that user confirmation is required to publish a local branch which is +# behind the remote branch (the remote branch points to commit_D, but the local +# branch points to commit_C). +run(["git", "checkout", "-b", "behind_branch", commit_D]) +run(["git", "push", "--set-upstream", "remote1", "behind_branch"]) +run(["git", "reset", "--hard", commit_C]) +test_publishing( + extra_args=[], + remote=1, + commit_id=commit_C, + output_pattern=BEHIND_REMOTE_CONFIRM_PROMPT +) +# Verify the same result specifying the branch explicitly +run(["git", "checkout", "other_branch"]) +test_publishing( + extra_args=[".", "behind_branch"], + remote=1, + commit_id=commit_C, + output_pattern=BEHIND_REMOTE_CONFIRM_PROMPT +) + +# Edit one of the files and check that `alr publish` fails due to the unclean +# repo. +run(["git", "checkout", default_branch]) +with open("test_file_1", "w") as f: + f.write("This file is only on the default branch, and has been edited.\n") +check_publishing_fails(extra_args=[], error_pattern=UNSTAGED_CHANGES_MESSAGE) +# Verify the same failure for explicitly specifying any branch, commit or tag +refs = [ + default_branch, "other_branch", "default_branch_local", + commit_D, commit_E, commit_F, "tag_D", "tag_E", "tag_F", +] +for ref in refs: + check_publishing_fails( + extra_args=[".", ref], error_pattern=UNSTAGED_CHANGES_MESSAGE + ) + +# Commit the edited file and check that `alr publish` still fails because the +# remote is not synchronised. +run(["git", "add", "test_file_1"]) +run(["git", "commit", "-m", "This commit is on default branch but not remote1"]) +commit_G = git_head() +check_publishing_fails(extra_args=[], error_pattern=AHEAD_OF_REMOTE_MESSAGE) +# Explicitly publishing the unsynchronised branch or commit should also fail +# (regardless of which branch is currently checked out). +check_publishing_fails( + extra_args=[".", default_branch], error_pattern=AHEAD_OF_REMOTE_MESSAGE +) +check_publishing_fails( + extra_args=[".", commit_G], error_pattern=COMMIT_NOT_PRESENT_MESSAGE +) +run(["git", "checkout", "other_branch"]) +check_publishing_fails( + extra_args=[".", default_branch], error_pattern=AHEAD_OF_REMOTE_MESSAGE +) +check_publishing_fails( + extra_args=[".", commit_G], error_pattern=COMMIT_NOT_PRESENT_MESSAGE +) +# However, publishing other commits/tags/branches should now work again, since +# they are present on the relevant remote. +test_publishing(extra_args=[], remote=2, commit_id=commit_F) +cases = [ + (commit_B, commit_B, 1), (commit_D, commit_D, 1), (commit_F, commit_F, 2), + ("tag_B", commit_B, 1), ("tag_D", commit_D, 1), ("tag_F", commit_F, 2), + ("other_branch", commit_F, 2), ("ambiguous_branch", commit_E, 2) +] +for ref, commit, remote in cases: + test_publishing(extra_args=[".", ref], remote=remote, commit_id=commit) + +# Disable the default branch's tracking of remote1, and verify that publishing +# fails with appropriate messages. +run(["git", "checkout", default_branch]) +run(["git", "branch", "--unset-upstream"]) +check_publishing_fails( + extra_args=[], + error_pattern=COMMIT_CONFIRM_PROMPT + COMMIT_NOT_PRESENT_MESSAGE +) +check_publishing_fails( + extra_args=[".", default_branch], + error_pattern=COMMIT_CONFIRM_PROMPT + COMMIT_NOT_PRESENT_MESSAGE +) + +# Verify that a detached head works, and behaves the same as specifying the +# commit explicitly +run(["git", "checkout", commit_B]) +test_publishing( + extra_args=[], + remote=1, + commit_id=commit_B, + output_pattern=URL_CONFIRM_PROMPT_TEMPLATE.format(n=1) +) +run(["git", "checkout", commit_E]) +check_publishing_fails(extra_args=[], error_pattern=AMBIGUOUS_REMOTE_MESSAGE) +run(["git", "checkout", commit_G]) +check_publishing_fails(extra_args=[], error_pattern=COMMIT_NOT_PRESENT_MESSAGE) + + +print('SUCCESS') diff --git a/testsuite/tests/publish/multiple-remotes/test.yaml b/testsuite/tests/publish/multiple-remotes/test.yaml new file mode 100644 index 000000000..32c747b3f --- /dev/null +++ b/testsuite/tests/publish/multiple-remotes/test.yaml @@ -0,0 +1 @@ +driver: python-script From 6a29f1c0fe46e290fb35d198e104368db403a34d Mon Sep 17 00:00:00 2001 From: Seb M'Caw Date: Mon, 14 Oct 2024 13:56:42 +0000 Subject: [PATCH 2/4] Support Python 3.9 --- testsuite/tests/publish/multiple-remotes/test.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/testsuite/tests/publish/multiple-remotes/test.py b/testsuite/tests/publish/multiple-remotes/test.py index b9ea5f040..809d14ec9 100644 --- a/testsuite/tests/publish/multiple-remotes/test.py +++ b/testsuite/tests/publish/multiple-remotes/test.py @@ -6,6 +6,7 @@ import os import re import subprocess +from typing import Optional from drivers.alr import run_alr, init_local_crate from drivers.asserts import assert_match @@ -66,7 +67,7 @@ def test_publishing( extra_args: list[str], remote: int, commit_id: str, - output_pattern: str | None = None, + output_pattern: Optional[str] = None, ): """ Run `alr --force publish --skip-submit` with the specified additional args, @@ -92,7 +93,10 @@ def test_publishing( assert_match(rf'.*url = "git\+file:.*remote{remote}"', manifest) assert_match(rf'.*commit = "{commit_id}"', manifest) -def check_publishing_fails(extra_args: list[str], error_pattern: str | None): +def check_publishing_fails( + extra_args: list[str], + error_pattern: Optional[str] = None, +): """ Run `alr --force publish --skip-submit` with the specified additional args, and assert that it fails (optionally asserting the output matches a regex From b3528dd1ddc31b6b4b7fd343bc349acc795aea7a Mon Sep 17 00:00:00 2001 From: Seb M'Caw Date: Thu, 24 Oct 2024 15:51:52 +0000 Subject: [PATCH 3/4] Move commit_file() to helpers --- testsuite/drivers/helpers.py | 17 +++++++++++++ .../tests/publish/multiple-remotes/test.py | 24 ++++--------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index b13877498..6585bc9ca 100644 --- a/testsuite/drivers/helpers.py +++ b/testsuite/drivers/helpers.py @@ -223,6 +223,23 @@ def commit_all(path): return head_commit.decode() +def git_commit_file( + commit_name: str, path: str, content: str, mode: str = "x" +) -> str: + """ + Write to a file with the specified content and `git commit` it. + + Also returns the commit's hash and attaches the tag `f"tag_{commit_name}"` + thereto. + """ + with open(path, mode) as f: + f.write(content) + run(["git", "add", path]).check_returncode() + run(["git", "commit", "-m", f"Commit {commit_name}"]).check_returncode() + run(["git", "tag", f"tag_{commit_name}"]).check_returncode() + return git_head() + + def zip_dir(path, filename): """ Zip contents of path into filename. Relative paths are preserved. diff --git a/testsuite/tests/publish/multiple-remotes/test.py b/testsuite/tests/publish/multiple-remotes/test.py index 809d14ec9..90261bcf7 100644 --- a/testsuite/tests/publish/multiple-remotes/test.py +++ b/testsuite/tests/publish/multiple-remotes/test.py @@ -10,7 +10,7 @@ from drivers.alr import run_alr, init_local_crate from drivers.asserts import assert_match -from drivers.helpers import git_branch, git_head, init_git_repo +from drivers.helpers import git_branch, git_commit_file, git_head, init_git_repo TEST_ROOT_DIR = os.getcwd() @@ -49,20 +49,6 @@ def run(*args, **kwargs): subprocess.run(*args, **kwargs).check_returncode() -def commit_file(commit_name: str, path: str, content: str): - """ - Create a new file with the specified content and `git commit` it. - - Also returns the commit's hash and attaches the tag `f"tag_{commit_name}"` - thereto. - """ - with open(path, "x") as f: - f.write(content) - run(["git", "add", path]) - run(["git", "commit", "-m", f"Commit {commit_name}"]) - run(["git", "tag", f"tag_{commit_name}"]) - return git_head() - def test_publishing( extra_args: list[str], remote: int, @@ -131,13 +117,13 @@ def check_publishing_fails( # E - F <--- other_branch commit_A = initial_commit run(["git", "tag", "tag_A"]) -commit_E = commit_file( +commit_E = git_commit_file( commit_name="E", path="test_file_0", content="This file will be merged so that it is on both branches.\n" ) run(["git", "checkout", default_branch]) -commit_B = commit_file( +commit_B = git_commit_file( commit_name="B", path="test_file_1", content="This file is only on the default branch.\n" @@ -145,13 +131,13 @@ def check_publishing_fails( run(["git", "merge", "other_branch"]) commit_C = git_head() run(["git", "tag", "tag_C"]) -commit_D = commit_file( +commit_D = git_commit_file( commit_name="D", path="test_file_2", content="This file is also only on the default branch.\n" ) run(["git", "checkout", "other_branch"]) -commit_F = commit_file( +commit_F = git_commit_file( commit_name="F", path="test_file_3", content="This file is only on other_branch.\n" From d998f142e996c57f3635fde97ddd85429bfd300a Mon Sep 17 00:00:00 2001 From: Seb M'Caw Date: Thu, 24 Oct 2024 15:54:18 +0000 Subject: [PATCH 4/4] Use git_commit_file() elsewhere --- testsuite/tests/pin/branch/test.py | 7 ++----- testsuite/tests/pin/conflicting-remote/test.py | 8 ++------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/testsuite/tests/pin/branch/test.py b/testsuite/tests/pin/branch/test.py index ad580c1f6..8a3c30ae8 100644 --- a/testsuite/tests/pin/branch/test.py +++ b/testsuite/tests/pin/branch/test.py @@ -4,8 +4,7 @@ from drivers.alr import run_alr, alr_pin, alr_unpin, init_local_crate from drivers.asserts import assert_match -from drivers.helpers import git_branch, init_git_repo -from e3.os.fs import touch +from drivers.helpers import git_branch, git_commit_file, init_git_repo import re import os @@ -21,9 +20,7 @@ # Create a second branch and commit for testing subprocess.run(["git", "checkout", "-b", "devel"]).check_returncode() -touch("telltale") -subprocess.run(["git", "add", "telltale"]).check_returncode() -subprocess.run(["git", "commit", "-m", "branching"]).check_returncode() +git_commit_file(commit_name="branching", path="telltale", content="") os.chdir("..") # Now pin to the branch, and verify the telltale file exists in the checkout diff --git a/testsuite/tests/pin/conflicting-remote/test.py b/testsuite/tests/pin/conflicting-remote/test.py index 0c60e14a8..01ea29af4 100644 --- a/testsuite/tests/pin/conflicting-remote/test.py +++ b/testsuite/tests/pin/conflicting-remote/test.py @@ -4,8 +4,7 @@ from drivers.alr import run_alr, alr_pin, alr_unpin, init_local_crate from drivers.asserts import assert_eq, assert_match -from drivers.helpers import git_blast, git_head, init_git_repo -from e3.os.fs import touch +from drivers.helpers import git_blast, git_commit_file, git_head, init_git_repo from re import escape import re @@ -23,10 +22,7 @@ # Create a second branch and commit for testing subprocess.run(["git", "checkout", "-b", "devel"]).check_returncode() -touch("x") -subprocess.run(["git", "add", "x"]).check_returncode() -subprocess.run(["git", "commit", "-m", "branching"]).check_returncode() -head2 = git_head() +head2 = git_commit_file(commit_name="branching", path="x", content="") os.chdir("..")