Skip to content

Commit 8dfd421

Browse files
authored
Merge pull request #4837 from gitbutlerapp/remote-credentials-helper-struct
remove useless credentials helper struct
2 parents 710308b + 3899975 commit 8dfd421

File tree

11 files changed

+78
-143
lines changed

11 files changed

+78
-143
lines changed

crates/gitbutler-branch-actions/src/actions.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use gitbutler_oplog::{
2424
};
2525
use gitbutler_project::{FetchResult, Project};
2626
use gitbutler_reference::{ReferenceName, Refname, RemoteRefname};
27-
use gitbutler_repo::{credentials::Helper, RepoActionsExt, RepositoryExt};
27+
use gitbutler_repo::{RepoActionsExt, RepositoryExt};
2828
use std::path::PathBuf;
2929
use tracing::instrument;
3030

@@ -402,9 +402,8 @@ impl VirtualBranchActions {
402402
name: ReferenceName,
403403
with_force: bool,
404404
) -> Result<()> {
405-
let helper = Helper::default();
406405
let ctx = open_with_verify(project)?;
407-
gitbutler_repo::push_change_reference(&ctx, branch_id, name, with_force, &helper)
406+
gitbutler_repo::push_change_reference(&ctx, branch_id, name, with_force)
408407
}
409408

410409
pub fn update_change_reference(
@@ -484,11 +483,10 @@ impl VirtualBranchActions {
484483
with_force: bool,
485484
askpass: Option<Option<BranchId>>,
486485
) -> Result<()> {
487-
let helper = Helper::default();
488486
let ctx = open_with_verify(project)?;
489487
assure_open_workspace_mode(&ctx)
490488
.context("Pushing a branch requires open workspace mode")?;
491-
branch::push(&ctx, branch_id, with_force, &helper, askpass)
489+
branch::push(&ctx, branch_id, with_force, askpass)
492490
}
493491

494492
pub fn list_local_branches(project: Project) -> Result<Vec<RemoteBranch>> {
@@ -547,12 +545,11 @@ impl VirtualBranchActions {
547545
) -> Result<FetchResult> {
548546
let ctx = CommandContext::open(project)?;
549547

550-
let helper = Helper::default();
551548
let remotes = ctx.repository().remotes_as_string()?;
552549
let fetch_errors: Vec<_> = remotes
553550
.iter()
554551
.filter_map(|remote| {
555-
ctx.fetch(remote, &helper, askpass.clone())
552+
ctx.fetch(remote, askpass.clone())
556553
.err()
557554
.map(|err| err.to_string())
558555
})

crates/gitbutler-branch-actions/src/virtual.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use gitbutler_operating_modes::assure_open_workspace_mode;
2525
use gitbutler_project::access::WorktreeWritePermission;
2626
use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname};
2727
use gitbutler_repo::{
28-
credentials::Helper,
2928
rebase::{cherry_rebase, cherry_rebase_group},
3029
LogUntil, RepoActionsExt, RepositoryExt,
3130
};
@@ -1075,7 +1074,6 @@ pub(crate) fn push(
10751074
ctx: &CommandContext,
10761075
branch_id: BranchId,
10771076
with_force: bool,
1078-
credentials: &Helper,
10791077
askpass: Option<Option<BranchId>>,
10801078
) -> Result<()> {
10811079
let vb_state = ctx.project().virtual_branches();
@@ -1115,25 +1113,14 @@ pub(crate) fn push(
11151113
))
11161114
};
11171115

1118-
ctx.push(
1119-
vbranch.head,
1120-
&remote_branch,
1121-
with_force,
1122-
credentials,
1123-
None,
1124-
askpass,
1125-
)?;
1116+
ctx.push(vbranch.head, &remote_branch, with_force, None, askpass)?;
11261117

11271118
vbranch.upstream = Some(remote_branch.clone());
11281119
vbranch.upstream_head = Some(vbranch.head);
11291120
vb_state
11301121
.set_branch(vbranch.clone())
11311122
.context("failed to write target branch after push")?;
1132-
ctx.fetch(
1133-
remote_branch.remote(),
1134-
credentials,
1135-
askpass.map(|_| "modal".to_string()),
1136-
)?;
1123+
ctx.fetch(remote_branch.remote(), askpass.map(|_| "modal".to_string()))?;
11371124

11381125
Ok(())
11391126
}

crates/gitbutler-repo/src/change_reference.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::str::FromStr;
22

3-
use crate::credentials::Helper;
43
use crate::{LogUntil, RepoActionsExt};
54
use anyhow::Context;
65
use anyhow::{anyhow, Result};
@@ -132,7 +131,6 @@ pub fn push_change_reference(
132131
branch_id: BranchId,
133132
name: ReferenceName,
134133
with_force: bool,
135-
credentials: &Helper,
136134
) -> Result<()> {
137135
let handle = VirtualBranchesHandle::new(ctx.project().gb_dir());
138136
let vbranch = handle.get_branch(branch_id)?;
@@ -149,7 +147,6 @@ pub fn push_change_reference(
149147
commit.id(),
150148
&upstream_refname,
151149
with_force,
152-
credentials,
153150
None,
154151
Some(Some(branch_id)),
155152
)

crates/gitbutler-repo/src/credentials.rs

Lines changed: 55 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ impl From<Credential> for git2::RemoteCallbacks<'_> {
6363
}
6464
}
6565

66-
#[derive(Clone, Default)]
67-
pub struct Helper {}
68-
6966
#[derive(Debug, thiserror::Error)]
7067
pub enum HelpError {
7168
#[error("no url set for remote")]
@@ -78,71 +75,67 @@ pub enum HelpError {
7875
Other(#[from] anyhow::Error),
7976
}
8077

81-
impl Helper {
82-
pub fn help<'a>(
83-
&'a self,
84-
ctx: &'a CommandContext,
85-
remote_name: &str,
86-
) -> Result<Vec<(git2::Remote, Vec<Credential>)>, HelpError> {
87-
let remote = ctx.repository().find_remote(remote_name)?;
88-
let remote_url = Url::from_str(remote.url().ok_or(HelpError::NoUrlSet)?)
89-
.context("failed to parse remote url")?;
78+
pub fn help<'a>(
79+
ctx: &'a CommandContext,
80+
remote_name: &str,
81+
) -> Result<Vec<(git2::Remote<'a>, Vec<Credential>)>, HelpError> {
82+
let remote = ctx.repository().find_remote(remote_name)?;
83+
let remote_url = Url::from_str(remote.url().ok_or(HelpError::NoUrlSet)?)
84+
.context("failed to parse remote url")?;
9085

91-
// if file, no auth needed.
92-
if remote_url.scheme == Scheme::File {
93-
return Ok(vec![(remote, vec![Credential::Noop])]);
94-
}
86+
// if file, no auth needed.
87+
if remote_url.scheme == Scheme::File {
88+
return Ok(vec![(remote, vec![Credential::Noop])]);
89+
}
9590

96-
match &ctx.project().preferred_key {
97-
AuthKey::Local { private_key_path } => {
98-
let ssh_remote = if remote_url.scheme == Scheme::Ssh {
99-
Ok(remote)
100-
} else {
101-
let ssh_url = remote_url.as_ssh()?;
102-
ctx.repository().remote_anonymous(&ssh_url.to_string())
103-
}?;
91+
match &ctx.project().preferred_key {
92+
AuthKey::Local { private_key_path } => {
93+
let ssh_remote = if remote_url.scheme == Scheme::Ssh {
94+
Ok(remote)
95+
} else {
96+
let ssh_url = remote_url.as_ssh()?;
97+
ctx.repository().remote_anonymous(&ssh_url.to_string())
98+
}?;
10499

105-
Ok(vec![(
106-
ssh_remote,
107-
vec![Credential::Ssh(SshCredential::Keyfile {
108-
key_path: private_key_path.clone(),
109-
passphrase: None,
110-
})],
111-
)])
112-
}
113-
AuthKey::GitCredentialsHelper => {
114-
let https_remote = if remote_url.scheme == Scheme::Https {
115-
Ok(remote)
116-
} else {
117-
let url = remote_url.as_https()?;
118-
ctx.repository().remote_anonymous(&url.to_string())
119-
}?;
120-
let flow = Self::https_flow(ctx, &remote_url)?
121-
.into_iter()
122-
.map(Credential::Https)
123-
.collect::<Vec<_>>();
124-
Ok(vec![(https_remote, flow)])
125-
}
126-
AuthKey::SystemExecutable => {
127-
tracing::error!("WARNING: FIXME: this codepath should NEVER be hit. Something is seriously wrong.");
128-
Ok(vec![])
129-
}
100+
Ok(vec![(
101+
ssh_remote,
102+
vec![Credential::Ssh(SshCredential::Keyfile {
103+
key_path: private_key_path.clone(),
104+
passphrase: None,
105+
})],
106+
)])
107+
}
108+
AuthKey::GitCredentialsHelper => {
109+
let https_remote = if remote_url.scheme == Scheme::Https {
110+
Ok(remote)
111+
} else {
112+
let url = remote_url.as_https()?;
113+
ctx.repository().remote_anonymous(&url.to_string())
114+
}?;
115+
let flow = https_flow(ctx, &remote_url)?
116+
.into_iter()
117+
.map(Credential::Https)
118+
.collect::<Vec<_>>();
119+
Ok(vec![(https_remote, flow)])
120+
}
121+
AuthKey::SystemExecutable => {
122+
tracing::error!(
123+
"WARNING: FIXME: this codepath should NEVER be hit. Something is seriously wrong."
124+
);
125+
Ok(vec![])
130126
}
131127
}
128+
}
132129

133-
fn https_flow(
134-
ctx: &CommandContext,
135-
remote_url: &Url,
136-
) -> Result<Vec<HttpsCredential>, HelpError> {
137-
let mut flow = vec![];
138-
139-
let mut helper = git2::CredentialHelper::new(&remote_url.to_string());
140-
let config = ctx.repository().config()?;
141-
helper.config(&config);
142-
if let Some((username, password)) = helper.execute() {
143-
flow.push(HttpsCredential::CredentialHelper { username, password });
144-
}
130+
fn https_flow(ctx: &CommandContext, remote_url: &Url) -> Result<Vec<HttpsCredential>, HelpError> {
131+
let mut flow = vec![];
145132

146-
Ok(flow)
133+
let mut helper = git2::CredentialHelper::new(&remote_url.to_string());
134+
let config = ctx.repository().config()?;
135+
helper.config(&config);
136+
if let Some((username, password)) = helper.execute() {
137+
flow.push(HttpsCredential::CredentialHelper { username, password });
147138
}
139+
140+
Ok(flow)
148141
}

crates/gitbutler-repo/src/repository.rs

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@ use gitbutler_error::error::Code;
88
use gitbutler_project::AuthKey;
99
use gitbutler_reference::{Refname, RemoteRefname};
1010

11-
use crate::{askpass, credentials::Helper, Config, RepositoryExt};
11+
use crate::{askpass, credentials, Config, RepositoryExt};
1212
pub trait RepoActionsExt {
13-
fn fetch(&self, remote_name: &str, credentials: &Helper, askpass: Option<String>)
14-
-> Result<()>;
13+
fn fetch(&self, remote_name: &str, askpass: Option<String>) -> Result<()>;
1514
fn push(
1615
&self,
1716
head: git2::Oid,
1817
branch: &RemoteRefname,
1918
with_force: bool,
20-
credentials: &Helper,
2119
refspec: Option<String>,
2220
askpass_broker: Option<Option<BranchId>>,
2321
) -> Result<()>;
@@ -36,7 +34,6 @@ pub trait RepoActionsExt {
3634
fn add_branch_reference(&self, branch: &Branch) -> Result<()>;
3735
fn git_test_push(
3836
&self,
39-
credentials: &Helper,
4037
remote_name: &str,
4138
branch_name: &str,
4239
askpass: Option<Option<BranchId>>,
@@ -47,7 +44,6 @@ pub trait RepoActionsExt {
4744
impl RepoActionsExt for CommandContext {
4845
fn git_test_push(
4946
&self,
50-
credentials: &Helper,
5147
remote_name: &str,
5248
branch_name: &str,
5349
askpass: Option<Option<BranchId>>,
@@ -67,20 +63,13 @@ impl RepoActionsExt for CommandContext {
6763
let refname =
6864
RemoteRefname::from_str(&format!("refs/remotes/{remote_name}/{branch_name}",))?;
6965

70-
match self.push(commit_id, &refname, false, credentials, None, askpass) {
66+
match self.push(commit_id, &refname, false, None, askpass) {
7167
Ok(()) => Ok(()),
7268
Err(e) => Err(anyhow::anyhow!(e.to_string())),
7369
}?;
7470

7571
let empty_refspec = Some(format!(":refs/heads/{}", branch_name));
76-
match self.push(
77-
commit_id,
78-
&refname,
79-
false,
80-
credentials,
81-
empty_refspec,
82-
askpass,
83-
) {
72+
match self.push(commit_id, &refname, false, empty_refspec, askpass) {
8473
Ok(()) => Ok(()),
8574
Err(e) => Err(anyhow::anyhow!(e.to_string())),
8675
}?;
@@ -257,7 +246,6 @@ impl RepoActionsExt for CommandContext {
257246
head: git2::Oid,
258247
branch: &RemoteRefname,
259248
with_force: bool,
260-
credentials: &Helper,
261249
refspec: Option<String>,
262250
askpass_broker: Option<Option<BranchId>>,
263251
) -> Result<()> {
@@ -295,7 +283,7 @@ impl RepoActionsExt for CommandContext {
295283
.map_err(Into::into);
296284
}
297285

298-
let auth_flows = credentials.help(self, branch.remote())?;
286+
let auth_flows = credentials::help(self, branch.remote())?;
299287
for (mut remote, callbacks) in auth_flows {
300288
let mut update_refs_error: Option<git2::Error> = None;
301289
for callback in callbacks {
@@ -351,12 +339,7 @@ impl RepoActionsExt for CommandContext {
351339
Err(anyhow!("authentication failed").context(Code::ProjectGitAuth))
352340
}
353341

354-
fn fetch(
355-
&self,
356-
remote_name: &str,
357-
credentials: &Helper,
358-
askpass: Option<String>,
359-
) -> Result<()> {
342+
fn fetch(&self, remote_name: &str, askpass: Option<String>) -> Result<()> {
360343
let refspec = format!("+refs/heads/*:refs/remotes/{}/*", remote_name);
361344

362345
// NOTE(qix-): This is a nasty hack, however the codebase isn't structured
@@ -384,7 +367,7 @@ impl RepoActionsExt for CommandContext {
384367
.map_err(Into::into);
385368
}
386369

387-
let auth_flows = credentials.help(self, remote_name)?;
370+
let auth_flows = credentials::help(self, remote_name)?;
388371
for (mut remote, callbacks) in auth_flows {
389372
for callback in callbacks {
390373
let mut fetch_opts = git2::FetchOptions::new();

crates/gitbutler-repo/tests/change_reference.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use gitbutler_branch::VirtualBranchesHandle;
33
use gitbutler_command_context::CommandContext;
44
use gitbutler_commit::commit_ext::CommitExt;
55
use gitbutler_repo::{
6-
create_change_reference, credentials::Helper, list_branch_references, push_change_reference,
6+
create_change_reference, list_branch_references, push_change_reference,
77
update_change_reference, LogUntil, RepoActionsExt,
88
};
99
use tempfile::TempDir;
@@ -217,13 +217,7 @@ fn push_success() -> Result<()> {
217217
"refs/remotes/origin/first".into(),
218218
test_ctx.commits.first().unwrap().change_id().unwrap(),
219219
)?;
220-
let result = push_change_reference(
221-
&ctx,
222-
reference.branch_id,
223-
reference.name,
224-
false,
225-
&Helper::default(),
226-
);
220+
let result = push_change_reference(&ctx, reference.branch_id, reference.name, false);
227221
assert!(result.is_ok());
228222
Ok(())
229223
}

crates/gitbutler-repo/tests/credentials.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::{path::PathBuf, str};
22

33
use gitbutler_command_context::CommandContext;
44
use gitbutler_project as projects;
5-
use gitbutler_repo::credentials::{Credential, Helper, SshCredential};
5+
use gitbutler_repo::credentials::{help, Credential, SshCredential};
66
use gitbutler_testsupport::{temp_dir, test_repository};
77
use gitbutler_user as users;
88

@@ -27,8 +27,6 @@ impl TestCase<'_> {
2727
.expect("valid v1 sample user");
2828
users.set_user(&user).unwrap();
2929

30-
let helper = Helper::default();
31-
3230
let (repo, _tmp) = test_repository();
3331
repo.remote("origin", self.remote_url).unwrap();
3432
let project = projects::Project {
@@ -38,7 +36,7 @@ impl TestCase<'_> {
3836
};
3937
let ctx = CommandContext::open(&project).unwrap();
4038

41-
let flow = helper.help(&ctx, "origin").unwrap();
39+
let flow = help(&ctx, "origin").unwrap();
4240
flow.into_iter()
4341
.map(|(remote, credentials)| (remote.url().as_ref().unwrap().to_string(), credentials))
4442
.collect::<Vec<_>>()

0 commit comments

Comments
 (0)