From 13b6b2fdc6c594cfc2963794ccf0e28977514a25 Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Sun, 16 Jan 2022 22:56:39 +0100 Subject: [PATCH] support deleting tag on remote (#1079) --- CHANGELOG.md | 1 + asyncgit/src/lib.rs | 1 + asyncgit/src/push.rs | 11 +++- asyncgit/src/sync/branch/merge_commit.rs | 8 +-- asyncgit/src/sync/branch/merge_ff.rs | 6 +- asyncgit/src/sync/branch/merge_rebase.rs | 14 ++--- asyncgit/src/sync/branch/mod.rs | 22 +++---- asyncgit/src/sync/remotes/mod.rs | 6 ++ asyncgit/src/sync/remotes/push.rs | 68 +++++++++++++++++---- asyncgit/src/sync/remotes/tags.rs | 78 +++++++++++++++++++++--- src/app.rs | 33 ++++++++-- src/components/push.rs | 9 ++- src/components/reset.rs | 4 ++ src/queue.rs | 8 ++- src/strings.rs | 6 ++ src/tabs/status.rs | 7 ++- 16 files changed, 221 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df6a926..4de1ef9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - allow reverting a commit from the commit log ([#927](https://github.com/extrawurst/gitui/issues/927)) - disable pull cmd on local-only branches ([#1047](https://github.com/extrawurst/gitui/issues/1047)) - support adding annotations to tags ([#747](https://github.com/extrawurst/gitui/issues/747)) +- support deleting tag on remote ([#1074](https://github.com/extrawurst/gitui/issues/1074)) ### Fixed - Keep commit message when pre-commit hook fails ([#1035](https://github.com/extrawurst/gitui/issues/1035)) diff --git a/asyncgit/src/lib.rs b/asyncgit/src/lib.rs index 061a784a..98c3fe89 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -55,6 +55,7 @@ pub use crate::{ status::{AsyncStatus, StatusParams}, sync::{ diff::{DiffLine, DiffLineType, FileDiff}, + remotes::push::PushType, status::{StatusItem, StatusItemType}, }, tags::AsyncTags, diff --git a/asyncgit/src/push.rs b/asyncgit/src/push.rs index 7165c30e..593e9c2e 100644 --- a/asyncgit/src/push.rs +++ b/asyncgit/src/push.rs @@ -1,8 +1,10 @@ use crate::{ error::{Error, Result}, sync::{ - cred::BasicAuthCredential, remotes::push::push, - remotes::push::ProgressNotification, RepoPath, + cred::BasicAuthCredential, + remotes::push::push_raw, + remotes::push::{ProgressNotification, PushType}, + RepoPath, }, AsyncGitNotification, RemoteProgress, }; @@ -20,6 +22,8 @@ pub struct PushRequest { /// pub branch: String, /// + pub push_type: PushType, + /// pub force: bool, /// pub delete: bool, @@ -100,10 +104,11 @@ impl AsyncPush { arc_progress, ); - let res = push( + let res = push_raw( &repo, params.remote.as_str(), params.branch.as_str(), + params.push_type, params.force, params.delete, params.basic_credential.clone(), diff --git a/asyncgit/src/sync/branch/merge_commit.rs b/asyncgit/src/sync/branch/merge_commit.rs index e7987544..50d7dc39 100644 --- a/asyncgit/src/sync/branch/merge_commit.rs +++ b/asyncgit/src/sync/branch/merge_commit.rs @@ -99,7 +99,7 @@ mod test { use super::*; use crate::sync::{ branch_compare_upstream, - remotes::{fetch, push::push}, + remotes::{fetch, push::push_branch}, tests::{ debug_cmd_print, get_commit_ids, repo_clone, repo_init_bare, write_commit_file, write_commit_file_at, @@ -129,7 +129,7 @@ mod test { Time::new(1, 0), ); - push( + push_branch( &clone1_dir.path().to_str().unwrap().into(), "origin", "master", @@ -151,7 +151,7 @@ mod test { ); //push should fail since origin diverged - assert!(push( + assert!(push_branch( &clone2_dir.into(), "origin", "master", @@ -228,7 +228,7 @@ mod test { "git status", ); - push( + push_branch( &clone1_dir.path().to_str().unwrap().into(), "origin", "master", diff --git a/asyncgit/src/sync/branch/merge_ff.rs b/asyncgit/src/sync/branch/merge_ff.rs index dbe3be40..5fe02769 100644 --- a/asyncgit/src/sync/branch/merge_ff.rs +++ b/asyncgit/src/sync/branch/merge_ff.rs @@ -53,7 +53,7 @@ pub fn branch_merge_upstream_fastforward( pub mod test { use super::*; use crate::sync::{ - remotes::{fetch, push::push}, + remotes::{fetch, push::push_branch}, tests::{ debug_cmd_print, get_commit_ids, repo_clone, repo_init_bare, write_commit_file, @@ -75,7 +75,7 @@ pub mod test { let commit1 = write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.path().to_str().unwrap().into(), "origin", "master", @@ -99,7 +99,7 @@ pub mod test { "commit2", ); - push( + push_branch( &clone2_dir.path().to_str().unwrap().into(), "origin", "master", diff --git a/asyncgit/src/sync/branch/merge_rebase.rs b/asyncgit/src/sync/branch/merge_rebase.rs index dce5ab1b..e1c9c9fb 100644 --- a/asyncgit/src/sync/branch/merge_rebase.rs +++ b/asyncgit/src/sync/branch/merge_rebase.rs @@ -38,7 +38,7 @@ mod test { use super::*; use crate::sync::{ branch_compare_upstream, get_commits_info, - remotes::{fetch, push::push}, + remotes::{fetch, push::push_branch}, tests::{ debug_cmd_print, get_commit_ids, repo_clone, repo_init_bare, write_commit_file, write_commit_file_at, @@ -81,7 +81,7 @@ mod test { assert_eq!(clone1.head_detached().unwrap(), false); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -111,7 +111,7 @@ mod test { assert_eq!(clone2.head_detached().unwrap(), false); - push( + push_branch( &clone2_dir.into(), "origin", "master", @@ -193,7 +193,7 @@ mod test { Time::new(0, 0), ); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -219,7 +219,7 @@ mod test { Time::new(1, 0), ); - push( + push_branch( &clone2_dir.into(), "origin", "master", @@ -287,7 +287,7 @@ mod test { let _commit1 = write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -312,7 +312,7 @@ mod test { "commit2", ); - push( + push_branch( &clone2_dir.into(), "origin", "master", diff --git a/asyncgit/src/sync/branch/mod.rs b/asyncgit/src/sync/branch/mod.rs index 601df341..327a3de1 100644 --- a/asyncgit/src/sync/branch/mod.rs +++ b/asyncgit/src/sync/branch/mod.rs @@ -458,7 +458,7 @@ mod tests_branch_compare { mod tests_branches { use super::*; use crate::sync::{ - remotes::{get_remotes, push::push}, + remotes::{get_remotes, push::push_branch}, rename_branch, tests::{ debug_cmd_print, repo_clone, repo_init, repo_init_bare, @@ -509,7 +509,7 @@ mod tests_branches { write_commit_file(&repo, "f1.txt", "foo", "c1"); rename_branch(&dir.into(), "refs/heads/master", branch_name) .unwrap(); - push( + push_branch( &dir.into(), "origin", branch_name, @@ -715,7 +715,7 @@ mod test_delete_branch { #[cfg(test)] mod test_remote_branches { use super::*; - use crate::sync::remotes::push::push; + use crate::sync::remotes::push::push_branch; use crate::sync::tests::{ repo_clone, repo_init_bare, write_commit_file, }; @@ -744,7 +744,7 @@ mod test_remote_branches { write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -759,7 +759,7 @@ mod test_remote_branches { write_commit_file(&clone1, "test.txt", "test2", "commit2"); - push( + push_branch( &clone1_dir.into(), "origin", "foo", @@ -801,7 +801,7 @@ mod test_remote_branches { // clone1 write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -813,7 +813,7 @@ mod test_remote_branches { .unwrap(); create_branch(&clone1_dir.into(), "foo").unwrap(); write_commit_file(&clone1, "test.txt", "test2", "commit2"); - push( + push_branch( &clone1_dir.into(), "origin", "foo", @@ -869,7 +869,7 @@ mod test_remote_branches { let branch_name = "bar/foo"; write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -881,7 +881,7 @@ mod test_remote_branches { .unwrap(); create_branch(&clone1_dir.into(), branch_name).unwrap(); write_commit_file(&clone1, "test.txt", "test2", "commit2"); - push( + push_branch( &clone1_dir.into(), "origin", branch_name, @@ -921,7 +921,7 @@ mod test_remote_branches { // clone1 write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( &clone1_dir.into(), "origin", "master", @@ -933,7 +933,7 @@ mod test_remote_branches { .unwrap(); create_branch(&clone1_dir.into(), "foo").unwrap(); write_commit_file(&clone1, "test.txt", "test2", "commit2"); - push( + push_branch( &clone1_dir.into(), "origin", "foo", diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index b0c57d26..d79a1da5 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -95,6 +95,12 @@ fn fetch_from_remote( options.download_tags(git2::AutotagOption::All); options.remote_callbacks(callbacks.callbacks()); remote.fetch(&[] as &[&str], Some(&mut options), None)?; + // fetch tags (also removing remotely deleted ones) + remote.fetch( + &["refs/tags/*:refs/tags/*"], + Some(&mut options), + None, + )?; Ok(()) } diff --git a/asyncgit/src/sync/remotes/push.rs b/asyncgit/src/sync/remotes/push.rs index 94589c0c..7559adeb 100644 --- a/asyncgit/src/sync/remotes/push.rs +++ b/asyncgit/src/sync/remotes/push.rs @@ -88,8 +88,23 @@ impl AsyncProgress for ProgressNotification { } } -#[allow(clippy::redundant_pub_crate)] -pub(crate) fn push( +/// +#[derive(Copy, Clone, Debug)] +pub enum PushType { + /// + Branch, + /// + Tag, +} + +impl Default for PushType { + fn default() -> Self { + Self::Branch + } +} + +#[cfg(test)] +pub fn push_branch( repo_path: &RepoPath, remote: &str, branch: &str, @@ -97,6 +112,30 @@ pub(crate) fn push( delete: bool, basic_credential: Option, progress_sender: Option>, +) -> Result<()> { + push_raw( + repo_path, + remote, + branch, + PushType::Branch, + force, + delete, + basic_credential, + progress_sender, + ) +} + +//TODO: clenaup +#[allow(clippy::too_many_arguments)] +pub fn push_raw( + repo_path: &RepoPath, + remote: &str, + branch: &str, + ref_type: PushType, + force: bool, + delete: bool, + basic_credential: Option, + progress_sender: Option>, ) -> Result<()> { scope_time!("push"); @@ -115,8 +154,13 @@ pub(crate) fn push( (true, false) => "+", (false, false) => "", }; + let ref_type = match ref_type { + PushType::Branch => "heads", + PushType::Tag => "tags", + }; + let branch_name = - format!("{}refs/heads/{}", branch_modifier, branch); + format!("{}refs/{}/{}", branch_modifier, ref_type, branch); remote.push(&[branch_name.as_str()], Some(&mut options))?; if let Some((reference, msg)) = @@ -182,7 +226,7 @@ mod tests { ) .unwrap(); - push( + push_branch( &tmp_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -208,7 +252,7 @@ mod tests { // Attempt a normal push, // should fail as branches diverged assert_eq!( - push( + push_branch( &tmp_other_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -224,7 +268,7 @@ mod tests { // Attempt force push, // should work as it forces the push through assert_eq!( - push( + push_branch( &tmp_other_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -294,7 +338,7 @@ mod tests { let commits = get_commit_ids(&repo, 1); assert!(commits.contains(&repo_1_commit)); - push( + push_branch( &tmp_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -337,7 +381,7 @@ mod tests { // Attempt a normal push, // should fail as branches diverged assert_eq!( - push( + push_branch( &tmp_other_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -358,7 +402,7 @@ mod tests { // Attempt force push, // should work as it forces the push through - push( + push_branch( &tmp_other_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -405,7 +449,7 @@ mod tests { let commits = get_commit_ids(&repo, 1); assert!(commits.contains(&commit_1)); - push( + push_branch( &tmp_repo_dir.path().to_str().unwrap().into(), "origin", "master", @@ -424,7 +468,7 @@ mod tests { .unwrap(); // Push the local branch - push( + push_branch( &tmp_repo_dir.path().to_str().unwrap().into(), "origin", "test_branch", @@ -450,7 +494,7 @@ mod tests { // Delete the remote branch assert_eq!( - push( + push_branch( &tmp_repo_dir.path().to_str().unwrap().into(), "origin", "test_branch", diff --git a/asyncgit/src/sync/remotes/tags.rs b/asyncgit/src/sync/remotes/tags.rs index b712bdff..61f89dfd 100644 --- a/asyncgit/src/sync/remotes/tags.rs +++ b/asyncgit/src/sync/remotes/tags.rs @@ -153,10 +153,16 @@ pub fn push_tags( #[cfg(test)] mod tests { use super::*; - use crate::sync::{ - self, - remotes::{fetch, fetch_all, push::push}, - tests::{repo_clone, repo_init_bare}, + use crate::{ + sync::{ + self, delete_tag, + remotes::{ + fetch, fetch_all, + push::{push_branch, push_raw}, + }, + tests::{repo_clone, repo_init_bare}, + }, + PushType, }; use pretty_assertions::assert_eq; use sync::tests::write_commit_file; @@ -183,7 +189,7 @@ mod tests { sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap(); - push( + push_branch( clone1_dir, "origin", "master", false, false, None, None, ) .unwrap(); @@ -231,7 +237,7 @@ mod tests { sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap(); - push( + push_branch( clone1_dir, "origin", "master", false, false, None, None, ) .unwrap(); @@ -265,7 +271,7 @@ mod tests { sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap(); - push( + push_branch( clone1_dir, "origin", "master", false, false, None, None, ) .unwrap(); @@ -294,7 +300,7 @@ mod tests { let commit1 = write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( clone1_dir, "origin", "master", false, false, None, None, ) .unwrap(); @@ -334,7 +340,7 @@ mod tests { let commit1 = write_commit_file(&clone1, "test.txt", "test", "commit1"); - push( + push_branch( clone1_dir, "origin", "master", false, false, None, None, ) .unwrap(); @@ -362,4 +368,58 @@ mod tests { assert_eq!(tags1, tags2); } + + #[test] + fn test_tags_delete_remote() { + let (r1_dir, _repo) = repo_init_bare().unwrap(); + let r1_dir = r1_dir.path().to_str().unwrap(); + + let (clone1_dir, clone1) = repo_clone(r1_dir).unwrap(); + let clone1_dir: &RepoPath = + &clone1_dir.path().to_str().unwrap().into(); + + let commit1 = + write_commit_file(&clone1, "test.txt", "test", "commit1"); + push_branch( + clone1_dir, "origin", "master", false, false, None, None, + ) + .unwrap(); + + let (clone2_dir, _clone2) = repo_clone(r1_dir).unwrap(); + let clone2_dir: &RepoPath = + &clone2_dir.path().to_str().unwrap().into(); + + // clone1 - creates tag + + sync::tag_commit(clone1_dir, &commit1, "tag1", None).unwrap(); + push_tags(clone1_dir, "origin", None, None).unwrap(); + + // clone 2 - pull + + fetch_all(clone2_dir, &None, &None).unwrap(); + assert_eq!(sync::get_tags(clone2_dir).unwrap().len(), 1); + + // delete on clone 1 + + delete_tag(clone1_dir, "tag1").unwrap(); + + push_raw( + clone1_dir, + "origin", + "tag1", + PushType::Tag, + false, + true, + None, + None, + ) + .unwrap(); + + push_tags(clone1_dir, "origin", None, None).unwrap(); + + // clone 2 + + fetch_all(clone2_dir, &None, &None).unwrap(); + assert_eq!(sync::get_tags(clone2_dir).unwrap().len(), 0); + } } diff --git a/src/app.rs b/src/app.rs index e8f06376..78c2f98c 100644 --- a/src/app.rs +++ b/src/app.rs @@ -25,7 +25,7 @@ use crate::{ use anyhow::{bail, Result}; use asyncgit::{ sync::{self, RepoPathRef}, - AsyncGitNotification, + AsyncGitNotification, PushType, }; use crossbeam_channel::Sender; use crossterm::event::{Event, KeyEvent}; @@ -723,8 +723,9 @@ impl App { self.file_to_open = path; flags.insert(NeedsUpdate::COMMANDS); } - InternalEvent::Push(branch, force, delete) => { - self.push_popup.push(branch, force, delete)?; + InternalEvent::Push(branch, push_type, force, delete) => { + self.push_popup + .push(branch, push_type, force, delete)?; flags.insert(NeedsUpdate::ALL); } InternalEvent::Pull(branch) => { @@ -790,6 +791,7 @@ impl App { Ok(flags) } + #[allow(clippy::too_many_lines)] fn process_confirmed_action( &mut self, action: Action, @@ -850,6 +852,7 @@ impl App { |name| { InternalEvent::Push( name.to_string(), + PushType::Branch, false, true, ) @@ -867,13 +870,33 @@ impl App { error.to_string(), )); } else { + let remote = sync::get_default_remote( + &self.repo.borrow(), + )?; + + self.queue.push(InternalEvent::ConfirmAction( + Action::DeleteRemoteTag(tag_name, remote), + )); + flags.insert(NeedsUpdate::ALL); self.tags_popup.update_tags()?; } } + Action::DeleteRemoteTag(tag_name, _remote) => { + self.queue.push(InternalEvent::Push( + tag_name, + PushType::Tag, + false, + true, + )); + } Action::ForcePush(branch, force) => { - self.queue - .push(InternalEvent::Push(branch, force, false)); + self.queue.push(InternalEvent::Push( + branch, + PushType::Branch, + force, + false, + )); } Action::PullMerge { rebase, .. } => { self.pull_popup.try_conflict_free_merge(rebase); diff --git a/src/components/push.rs b/src/components/push.rs index 6123362b..528f44da 100644 --- a/src/components/push.rs +++ b/src/components/push.rs @@ -17,8 +17,8 @@ use asyncgit::{ }, get_branch_remote, get_default_remote, RepoPathRef, }, - AsyncGitNotification, AsyncPush, PushRequest, RemoteProgress, - RemoteProgressState, + AsyncGitNotification, AsyncPush, PushRequest, PushType, + RemoteProgress, RemoteProgressState, }; use crossbeam_channel::Sender; use crossterm::event::Event; @@ -57,6 +57,7 @@ pub struct PushComponent { progress: Option, pending: bool, branch: String, + push_type: PushType, queue: Queue, theme: SharedTheme, key_config: SharedKeyConfig, @@ -79,6 +80,7 @@ impl PushComponent { pending: false, visible: false, branch: String::new(), + push_type: PushType::Branch, git_push: AsyncPush::new(repo.borrow().clone(), sender), progress: None, input_cred: CredComponent::new( @@ -94,10 +96,12 @@ impl PushComponent { pub fn push( &mut self, branch: String, + push_type: PushType, force: bool, delete: bool, ) -> Result<()> { self.branch = branch; + self.push_type = push_type; self.modifier = match (force, delete) { (true, true) => PushComponentModifier::ForceDelete, (false, true) => PushComponentModifier::Delete, @@ -149,6 +153,7 @@ impl PushComponent { self.git_push.request(PushRequest { remote, branch: self.branch.clone(), + push_type: self.push_type, force, delete: self.modifier.delete(), basic_credential: cred, diff --git a/src/components/reset.rs b/src/components/reset.rs index b10071d4..dd48a442 100644 --- a/src/components/reset.rs +++ b/src/components/reset.rs @@ -183,6 +183,10 @@ impl ConfirmComponent { &self.key_config, tag_name, ), + ), + Action::DeleteRemoteTag(_tag_name,remote) => ( + strings::confirm_title_delete_tag_remote(), + strings::confirm_msg_delete_tag_remote(remote), ), Action::ForcePush(branch, _force) => ( strings::confirm_title_force_push( diff --git a/src/queue.rs b/src/queue.rs index 48a1ff64..8f7c06f6 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -1,6 +1,7 @@ use crate::{components::AppOption, tabs::StashingOptions}; -use asyncgit::sync::{ - diff::DiffLinePosition, CommitId, CommitTags, TreeFile, +use asyncgit::{ + sync::{diff::DiffLinePosition, CommitId, CommitTags, TreeFile}, + PushType, }; use bitflags::bitflags; use std::{ @@ -39,6 +40,7 @@ pub enum Action { DeleteLocalBranch(String), DeleteRemoteBranch(String), DeleteTag(String), + DeleteRemoteTag(String, String), ForcePush(String, bool), PullMerge { incoming: usize, rebase: bool }, AbortMerge, @@ -85,7 +87,7 @@ pub enum InternalEvent { /// OpenExternalEditor(Option), /// - Push(String, bool, bool), + Push(String, PushType, bool, bool), /// Pull(String), /// diff --git a/src/strings.rs b/src/strings.rs index a143795c..495528c6 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -243,6 +243,12 @@ pub fn confirm_msg_delete_tag( ) -> String { format!("Confirm deleting Tag: '{}' ?", tag_name) } +pub fn confirm_title_delete_tag_remote() -> String { + "Delete Tag (remote)".to_string() +} +pub fn confirm_msg_delete_tag_remote(remote_name: &str) -> String { + format!("Confirm deleting tag on remote '{}'?", remote_name) +} pub fn confirm_title_force_push( _key_config: &SharedKeyConfig, ) -> String { diff --git a/src/tabs/status.rs b/src/tabs/status.rs index f2c941a3..52b91358 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -20,7 +20,7 @@ use asyncgit::{ }, sync::{BranchCompare, CommitId}, AsyncDiff, AsyncGitNotification, AsyncStatus, DiffParams, - DiffType, StatusParams, + DiffType, PushType, StatusParams, }; use crossbeam_channel::Sender; use crossterm::event::Event; @@ -558,7 +558,10 @@ impl Status { )); } else { self.queue.push(InternalEvent::Push( - branch, force, false, + branch, + PushType::Branch, + force, + false, )); } }