This thread contains a patchset. You're looking at the original emails,
but you may wish to use the patch review UI.
Review patch
12
3
[PATCH radicle-link] lnk-identities: optionally use initial branch on git clone
Better follow git's command line behavior by setting one local branch, the
projects default head, on clone. It is assumed to be the project
identities 'default_branch' field.
Implement the new clone argument as optional to keep backward
compatibility.
---
cli/lnk-identities/src/git.rs | 7 +++++
cli/lnk-identities/src/git/checkout.rs | 30 ++++++++++++++ -----
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
index c9ed5e90..f2dac322 100644
--- a/cli/lnk-identities/src/git.rs
+++ b/cli/lnk-identities/src/git.rs
@@ -188,6 +188,7 @@ pub fn clone<F>(
path: &Path,
storage: F,
mut remote: Remote<LocalUrl>,
+ init_branch: Option<&str>,
) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
where
F: CanOpenStorage + 'static,
@@ -201,6 +202,12 @@ where
let branch: git_ext::RefLike = OneLevel::from(reference).into();
let branch = branch.strip_prefix(remote.name.clone())?;
let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
+ if let Some(init_branch) = init_branch {
+ if branch.as_str() != init_branch {
+ continue;
+ }
+ }
+
let _remote_branch = repo.reference(
reflike!("refs/remotes")
.join(remote.name.clone())
diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
index b85163dd..bcbbfa2c 100644
--- a/cli/lnk-identities/src/git/checkout.rs
+++ b/cli/lnk-identities/src/git/checkout.rs
@@ -139,17 +139,21 @@ where
pub struct Local {
url: LocalUrl,
path: PathBuf,
+ default_branch: OneLevel,
}
impl Local {
- pub fn new<I>(identity: &I, path: PathBuf) -> Self
+ pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
where
- I: HasName + HasUrn,
+ I: HasBranch + HasName + HasUrn,
{
- Self {
- url: LocalUrl::from(identity.urn()),
+ let urn = identity.urn();
+ let default_branch = identity.branch_or_die(urn.clone())?;
+ Ok(Self {
+ url: LocalUrl::from(urn),
path,
- }
+ default_branch,
+ })
}
fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
@@ -164,7 +168,12 @@ impl Local {
force: Force::True,
},
);
- Ok(git::clone(&self.path, open_storage, rad)?)
+ Ok(git::clone(
+ &self.path,
+ open_storage,
+ rad,
+ Some(&self.default_branch),
+ )?)
}
}
@@ -209,7 +218,12 @@ impl Peer {
force: Force::True,
}]);
- let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
+ let (repo, _) = git::clone(
+ &self.path,
+ open_storage.clone(),
+ remote,
+ Some(&self.default_branch),
+ )?;
// Create a rad remote and push the default branch so we can set it as the
// upstream.
@@ -248,7 +262,7 @@ where
{
let path = path.resolve(identity.name());
Ok(match remote {
- None => Either::Left(Local::new(identity, path)),
+ None => Either::Left(Local::new(identity, path)?),
Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
})
}
diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
index 5177eb38..1ca7d8fb 100644
--- a/cli/lnk-identities/t/src/tests/git/checkout.rs
+++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
@@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
signer: signer.into(),
};
- let local = Local::new(&proj.project, temp.path().to_path_buf());
+ let local = Local::new(&proj.project, temp.path().to_path_buf())?;
let repo = checkout(
&paths,
settings,
--
2.35.3
[radicle-link/patches/nixos-latest.yml] build failed
On Thu Aug 25, 2022 at 6:30 PM IST, Slack Coder wrote:
> Better follow git's command line behavior by setting one local branch, the
> projects default head, on clone. It is assumed to be the project
> identities 'default_branch' field.
>
> Implement the new clone argument as optional to keep backward
> compatibility.
> ---
> cli/lnk-identities/src/git.rs | 7 +++++
> cli/lnk-identities/src/git/checkout.rs | 30 ++++++++++++++-----
> .../t/src/tests/git/checkout.rs | 2 +-
> 3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
> index c9ed5e90..f2dac322 100644
> --- a/cli/lnk-identities/src/git.rs
> +++ b/cli/lnk-identities/src/git.rs
> @@ -188,6 +188,7 @@ pub fn clone<F>(
> path: &Path,
> storage: F,
> mut remote: Remote<LocalUrl>,
> + init_branch: Option<&str>,
> ) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
> where
> F: CanOpenStorage + 'static,
> @@ -201,6 +202,12 @@ where
> let branch: git_ext::RefLike = OneLevel::from(reference).into();
> let branch = branch.strip_prefix(remote.name.clone())?;
> let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
> + if let Some(init_branch) = init_branch {
> + if branch.as_str() != init_branch {
> + continue;
> + }
> + }
> +
I'm wondering if instead of matching all the time, we do one fetch and
one match, following some pseudo code like:
match init_branch {
// same as old logic
None => loop refs -> create ref
// we don't use a for loop and instead just search through for
// the one we're looking for
Some(init) => refs.find(init) -> create ref
}
> let _remote_branch = repo.reference(
> reflike!("refs/remotes")
> .join(remote.name.clone())
> diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
> index b85163dd..bcbbfa2c 100644
> --- a/cli/lnk-identities/src/git/checkout.rs
> +++ b/cli/lnk-identities/src/git/checkout.rs
> @@ -139,17 +139,21 @@ where
> pub struct Local {
> url: LocalUrl,
> path: PathBuf,
> + default_branch: OneLevel,
I'm wondering, since the `init_branch` is optional could this made
`Option<OneLevel>`? That we don't need to use `branch_or_die` and the
code stays compatible with a `Project` that doesn't define a
`default_branch` and also a `Person`.
> }
>
> impl Local {
> - pub fn new<I>(identity: &I, path: PathBuf) -> Self
> + pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
> where
> - I: HasName + HasUrn,
> + I: HasBranch + HasName + HasUrn,
> {
> - Self {
> - url: LocalUrl::from(identity.urn()),
> + let urn = identity.urn();
> + let default_branch = identity.branch_or_die(urn.clone())?;
> + Ok(Self {
> + url: LocalUrl::from(urn),
> path,
> - }
> + default_branch,
> + })
> }
>
> fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
> @@ -164,7 +168,12 @@ impl Local {
> force: Force::True,
> },
> );
> - Ok(git::clone(&self.path, open_storage, rad)?)
> + Ok(git::clone(
> + &self.path,
> + open_storage,
> + rad,
> + Some(&self.default_branch),
> + )?)
> }
> }
>
> @@ -209,7 +218,12 @@ impl Peer {
> force: Force::True,
> }]);
>
> - let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
> + let (repo, _) = git::clone(
> + &self.path,
> + open_storage.clone(),
> + remote,
> + Some(&self.default_branch),
> + )?;
>
> // Create a rad remote and push the default branch so we can set it as the
> // upstream.
> @@ -248,7 +262,7 @@ where
> {
> let path = path.resolve(identity.name());
> Ok(match remote {
> - None => Either::Left(Local::new(identity, path)),
> + None => Either::Left(Local::new(identity, path)?),
> Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
> })
> }
> diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
> index 5177eb38..1ca7d8fb 100644
> --- a/cli/lnk-identities/t/src/tests/git/checkout.rs
> +++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
> @@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
> signer: signer.into(),
> };
>
> - let local = Local::new(&proj.project, temp.path().to_path_buf());
> + let local = Local::new(&proj.project, temp.path().to_path_buf())?;
> let repo = checkout(
> &paths,
> settings,
> --
> 2.35.3
On 8/26/22 06:00, Fintan Halpenny wrote:
>> Better follow git's command line behavior by setting one local branch, the
>> projects default head, on clone. It is assumed to be the project
>> identities 'default_branch' field.
>>
>> Implement the new clone argument as optional to keep backward
>> compatibility.
>> ---
>> cli/lnk-identities/src/git.rs | 7 +++++
>> cli/lnk-identities/src/git/checkout.rs | 30 ++++++++++++++-----
>> .../t/src/tests/git/checkout.rs | 2 +-
>> 3 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
>> index c9ed5e90..f2dac322 100644
>> --- a/cli/lnk-identities/src/git.rs
>> +++ b/cli/lnk-identities/src/git.rs
>> @@ -188,6 +188,7 @@ pub fn clone<F>(
>> path: &Path,
>> storage: F,
>> mut remote: Remote<LocalUrl>,
>> + init_branch: Option<&str>,
>> ) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
>> where
>> F: CanOpenStorage + 'static,
>> @@ -201,6 +202,12 @@ where
>> let branch: git_ext::RefLike = OneLevel::from(reference).into();
>> let branch = branch.strip_prefix(remote.name.clone())?;
>> let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
>> + if let Some(init_branch) = init_branch {
>> + if branch.as_str() != init_branch {
>> + continue;
>> + }
>> + }
>> +
> I'm wondering if instead of matching all the time, we do one fetch and
> one match, following some pseudo code like:
>
> match init_branch {
> // same as old logic
> None => loop refs -> create ref
> // we don't use a for loop and instead just search through for
> // the one we're looking for
> Some(init) => refs.find(init) -> create ref
> }
>
It would involve moving each the branch ref parsing and the reference
creation to their own function to keep the code dry. I'm happy either
way, good to go ahead with this?
[PATCH radicle-link v2 0/1] Reduce local branches from git::clone()
Version two for reducing local branch creation for git clone.
Slack Coder (1):
lnk-identities: optionally use initial branch on git clone
cli/lnk-identities/src/git.rs | 74 +++++++++++++------
cli/lnk-identities/src/git/checkout.rs | 30 ++++++--
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 75 insertions(+), 31 deletions(-)
--
2.35.3
[PATCH radicle-link v2 1/1] lnk-identities: optionally use initial branch on git clone
Better follow git's command line behavior by setting one local branch, the
projects default head, on clone. It is assumed to be the project
identities 'default_branch' field.
Implement the new clone argument as optional to keep backward
compatibility.
---
cli/lnk-identities/src/git.rs | 74 +++++++++++++ ------
cli/lnk-identities/src/git/checkout.rs | 30 ++++++ --
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 75 insertions(+), 31 deletions(-)
diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
index c9ed5e90..48e167b8 100644
--- a/cli/lnk-identities/src/git.rs
+++ b/cli/lnk-identities/src/git.rs
@@ -16,14 +16,11 @@ use librad::{
},
types::{
remote::{LocalFetchspec, LocalPushspec, Remote},
- Fetchspec,
- Force,
- Refspec,
+ Fetchspec, Force, Refspec,
},
},
git_ext::{self, OneLevel, Qualified},
- reflike,
- refspec_pattern,
+ reflike, refspec_pattern,
std_ext::result::ResultExt as _,
};
@@ -49,6 +46,9 @@ pub enum Error {
#[error(transparent)]
Git(#[from] git2::Error),
+
+ #[error("branch does not exist")]
+ BranchNotExist,
}
/// Equips a repository with a rad remote for the given id. If the directory at
@@ -178,6 +178,26 @@ pub fn set_upstream<Url>(
Ok(())
}
+ fn create_local_ref(
+ repo: &git2::Repository,
+ branch: git_ext::RefLike,
+ oid: git2::Oid,
+ remote: &Remote<LocalUrl>,
+ msg: &str,
+ ) -> Result<(), Error> {
+ let _remote_branch = repo.reference(
+ reflike!("refs/remotes")
+ .join(remote.name.clone())
+ .join(branch.clone())
+ .as_str(),
+ oid,
+ true,
+ msg,
+ )?;
+ let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, msg);
+ Ok(())
+ }
+
/// Clone a git repository to the `path` location, based off of the `remote`
/// provided.
///
@@ -188,29 +208,39 @@ pub fn clone<F>(
path: &Path,
storage: F,
mut remote: Remote<LocalUrl>,
+ init_branch: Option<&str>,
) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
where
F: CanOpenStorage + 'static,
{
let repo = git2::Repository::init(path)?;
remote.save(&repo)?;
- for (reference, oid) in remote.fetch(storage, &repo, LocalFetchspec::Configured)? {
- let msg = format!("Fetched `{}->{}`", reference, oid);
- tracing::debug!("{}", msg);
-
- let branch: git_ext::RefLike = OneLevel::from(reference).into();
- let branch = branch.strip_prefix(remote.name.clone())?;
- let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
- let _remote_branch = repo.reference(
- reflike!("refs/remotes")
- .join(remote.name.clone())
- .join(branch.clone())
- .as_str(),
- oid,
- true,
- &msg,
- )?;
- let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, &msg);
+
+ let _branches: Vec<_> = remote
+ .fetch(storage, &repo, LocalFetchspec::Configured)?
+ .map(|(_ref, oid)| {
+ let msg = format!("Fetched `{}->{}`", _ref, oid);
+ tracing::debug!("{}", msg);
+
+ let branch: git_ext::RefLike = OneLevel::from(_ref).into();
+ let branch = branch.strip_prefix(remote.name.clone()).unwrap();
+ let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
+ (branch, oid, msg)
+ })
+ .collect();
+ match init_branch {
+ Some(init_branch) => {
+ let (branch, oid, msg) = _branches
+ .iter()
+ .find(|(branch, _, _)| branch.as_str() == init_branch)
+ .ok_or(Error::BranchNotExist)?;
+ create_local_ref(&repo, branch.clone(), *oid, &remote, msg)?;
+ },
+ None => {
+ for (branch, oid, msg) in _branches {
+ create_local_ref(&repo, branch, oid, &remote, &msg)?
+ }
+ },
}
Ok((repo, remote))
diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
index b85163dd..bcbbfa2c 100644
--- a/cli/lnk-identities/src/git/checkout.rs
+++ b/cli/lnk-identities/src/git/checkout.rs
@@ -139,17 +139,21 @@ where
pub struct Local {
url: LocalUrl,
path: PathBuf,
+ default_branch: OneLevel,
}
impl Local {
- pub fn new<I>(identity: &I, path: PathBuf) -> Self
+ pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
where
- I: HasName + HasUrn,
+ I: HasBranch + HasName + HasUrn,
{
- Self {
- url: LocalUrl::from(identity.urn()),
+ let urn = identity.urn();
+ let default_branch = identity.branch_or_die(urn.clone())?;
+ Ok(Self {
+ url: LocalUrl::from(urn),
path,
- }
+ default_branch,
+ })
}
fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
@@ -164,7 +168,12 @@ impl Local {
force: Force::True,
},
);
- Ok(git::clone(&self.path, open_storage, rad)?)
+ Ok(git::clone(
+ &self.path,
+ open_storage,
+ rad,
+ Some(&self.default_branch),
+ )?)
}
}
@@ -209,7 +218,12 @@ impl Peer {
force: Force::True,
}]);
- let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
+ let (repo, _) = git::clone(
+ &self.path,
+ open_storage.clone(),
+ remote,
+ Some(&self.default_branch),
+ )?;
// Create a rad remote and push the default branch so we can set it as the
// upstream.
@@ -248,7 +262,7 @@ where
{
let path = path.resolve(identity.name());
Ok(match remote {
- None => Either::Left(Local::new(identity, path)),
+ None => Either::Left(Local::new(identity, path)?),
Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
})
}
diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
index 5177eb38..1ca7d8fb 100644
--- a/cli/lnk-identities/t/src/tests/git/checkout.rs
+++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
@@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
signer: signer.into(),
};
- let local = Local::new(&proj.project, temp.path().to_path_buf());
+ let local = Local::new(&proj.project, temp.path().to_path_buf())?;
let repo = checkout(
&paths,
settings,
--
2.35.3
Re: [PATCH radicle-link v2 1/1] lnk-identities: optionally use initial branch on git clone
You need to run `cargo fmt` over `git.rs` :)
On Thu Sep 1, 2022 at 5:46 PM IST, Slack Coder wrote:
> Better follow git's command line behavior by setting one local branch, the
> projects default head, on clone. It is assumed to be the project
> identities 'default_branch' field.
>
> Implement the new clone argument as optional to keep backward
> compatibility.
> ---
> cli/lnk-identities/src/git.rs | 74 +++++++++++++------
> cli/lnk-identities/src/git/checkout.rs | 30 ++++++--
> .../t/src/tests/git/checkout.rs | 2 +-
> 3 files changed, 75 insertions(+), 31 deletions(-)
>
> diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
> index c9ed5e90..48e167b8 100644
> --- a/cli/lnk-identities/src/git.rs
> +++ b/cli/lnk-identities/src/git.rs
> @@ -16,14 +16,11 @@ use librad::{
> },
> types::{
> remote::{LocalFetchspec, LocalPushspec, Remote},
> - Fetchspec,
> - Force,
> - Refspec,
> + Fetchspec, Force, Refspec,
> },
> },
> git_ext::{self, OneLevel, Qualified},
> - reflike,
> - refspec_pattern,
> + reflike, refspec_pattern,
> std_ext::result::ResultExt as _,
> };
>
> @@ -49,6 +46,9 @@ pub enum Error {
>
> #[error(transparent)]
> Git(#[from] git2::Error),
> +
> + #[error("branch does not exist")]
> + BranchNotExist,
> }
>
> /// Equips a repository with a rad remote for the given id. If the directory at
> @@ -178,6 +178,26 @@ pub fn set_upstream<Url>(
> Ok(())
> }
>
> +fn create_local_ref(
> + repo: &git2::Repository,
> + branch: git_ext::RefLike,
> + oid: git2::Oid,
> + remote: &Remote<LocalUrl>,
> + msg: &str,
> +) -> Result<(), Error> {
> + let _remote_branch = repo.reference(
> + reflike!("refs/remotes")
> + .join(remote.name.clone())
> + .join(branch.clone())
> + .as_str(),
> + oid,
> + true,
> + msg,
> + )?;
> + let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, msg);
> + Ok(())
> +}
> +
> /// Clone a git repository to the `path` location, based off of the `remote`
> /// provided.
> ///
> @@ -188,29 +208,39 @@ pub fn clone<F>(
> path: &Path,
> storage: F,
> mut remote: Remote<LocalUrl>,
> + init_branch: Option<&str>,
Can you change this to be `git_ext::RefLike` instead, please?
> ) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
> where
> F: CanOpenStorage + 'static,
> {
> let repo = git2::Repository::init(path)?;
> remote.save(&repo)?;
> - for (reference, oid) in remote.fetch(storage, &repo, LocalFetchspec::Configured)? {
> - let msg = format!("Fetched `{}->{}`", reference, oid);
> - tracing::debug!("{}", msg);
> -
> - let branch: git_ext::RefLike = OneLevel::from(reference).into();
> - let branch = branch.strip_prefix(remote.name.clone())?;
> - let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
> - let _remote_branch = repo.reference(
> - reflike!("refs/remotes")
> - .join(remote.name.clone())
> - .join(branch.clone())
> - .as_str(),
> - oid,
> - true,
> - &msg,
> - )?;
> - let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, &msg);
> +
> + let _branches: Vec<_> = remote
The underscore is misleading here. It generally means that a variable
isn't used, but in fact it is used below.
> + .fetch(storage, &repo, LocalFetchspec::Configured)?
> + .map(|(_ref, oid)| {
> + let msg = format!("Fetched `{}->{}`", _ref, oid);
> + tracing::debug!("{}", msg);
> +
> + let branch: git_ext::RefLike = OneLevel::from(_ref).into();
> + let branch = branch.strip_prefix(remote.name.clone()).unwrap();
> + let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
> + (branch, oid, msg)
> + })
> + .collect();
> + match init_branch {
> + Some(init_branch) => {
> + let (branch, oid, msg) = _branches
> + .iter()
> + .find(|(branch, _, _)| branch.as_str() == init_branch)
> + .ok_or(Error::BranchNotExist)?;
This error might cause some confusion for users. Could we provide:
a) The context of the missing branch, ie. we were trying to clone with
a speicific initial branch.
b) The actual branch we were looking for.
> + create_local_ref(&repo, branch.clone(), *oid, &remote, msg)?;
> + },
> + None => {
> + for (branch, oid, msg) in _branches {
> + create_local_ref(&repo, branch, oid, &remote, &msg)?
> + }
> + },
> }
>
> Ok((repo, remote))
> diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
> index b85163dd..bcbbfa2c 100644
> --- a/cli/lnk-identities/src/git/checkout.rs
> +++ b/cli/lnk-identities/src/git/checkout.rs
> @@ -139,17 +139,21 @@ where
> pub struct Local {
> url: LocalUrl,
> path: PathBuf,
> + default_branch: OneLevel,
I believe I asked for this to be optional in the last patch?
> }
>
> impl Local {
> - pub fn new<I>(identity: &I, path: PathBuf) -> Self
> + pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
> where
> - I: HasName + HasUrn,
> + I: HasBranch + HasName + HasUrn,
> {
> - Self {
> - url: LocalUrl::from(identity.urn()),
> + let urn = identity.urn();
> + let default_branch = identity.branch_or_die(urn.clone())?;
The above would make this not need to use `branch_or_die`.
> + Ok(Self {
> + url: LocalUrl::from(urn),
> path,
> - }
> + default_branch,
> + })
> }
>
> fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
> @@ -164,7 +168,12 @@ impl Local {
> force: Force::True,
> },
> );
> - Ok(git::clone(&self.path, open_storage, rad)?)
> + Ok(git::clone(
> + &self.path,
> + open_storage,
> + rad,
> + Some(&self.default_branch),
> + )?)
> }
> }
>
> @@ -209,7 +218,12 @@ impl Peer {
> force: Force::True,
> }]);
>
> - let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
> + let (repo, _) = git::clone(
> + &self.path,
> + open_storage.clone(),
> + remote,
> + Some(&self.default_branch),
> + )?;
>
> // Create a rad remote and push the default branch so we can set it as the
> // upstream.
> @@ -248,7 +262,7 @@ where
> {
> let path = path.resolve(identity.name());
> Ok(match remote {
> - None => Either::Left(Local::new(identity, path)),
> + None => Either::Left(Local::new(identity, path)?),
> Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
> })
> }
> diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
> index 5177eb38..1ca7d8fb 100644
> --- a/cli/lnk-identities/t/src/tests/git/checkout.rs
> +++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
> @@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
> signer: signer.into(),
> };
>
> - let local = Local::new(&proj.project, temp.path().to_path_buf());
> + let local = Local::new(&proj.project, temp.path().to_path_buf())?;
> let repo = checkout(
> &paths,
> settings,
> --
> 2.35.3
[PATCH radicle-link v3 0/1] lnk-identities: optionally use initial branch on git clone
The latest patch set addresses all issues from the feedback.
- remove bad leading underscores '_' in some variable names
- Use OneLevel and RefLike
- make Local.default_branch optional
- correct source code format
Slack Coder (1):
lnk-identities: allow initial branch on git clone
cli/lnk-identities/src/git.rs | 67 ++++++++++++++-----
cli/lnk-identities/src/git/checkout.rs | 30 ++++++---
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 73 insertions(+), 26 deletions(-)
--
2.35.3
[PATCH radicle-link v3 1/1] lnk-identities: allow initial branch on git clone
Better follow git's command line behavior by setting one local branch, the
projects default head, on clone. It is assumed to be the project
identities 'default_branch' field.
Implement the new clone argument as optional to keep backward
compatibility.
---
cli/lnk-identities/src/git.rs | 67 ++++++++++++++ -----
cli/lnk-identities/src/git/checkout.rs | 30 ++++++ ---
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 73 insertions(+), 26 deletions(-)
diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
index c9ed5e90..c31fefc3 100644
--- a/cli/lnk-identities/src/git.rs
+++ b/cli/lnk-identities/src/git.rs
@@ -49,6 +49,9 @@ pub enum Error {
#[error(transparent)]
Git(#[from] git2::Error),
+
+ #[error("failed to clone project with initial branch `{0}`, the branch is missing")]
+ MissingInitialBranch(git_ext::RefLike),
}
/// Equips a repository with a rad remote for the given id. If the directory at
@@ -178,6 +181,26 @@ pub fn set_upstream<Url>(
Ok(())
}
+ fn create_local_ref(
+ repo: &git2::Repository,
+ branch: git_ext::RefLike,
+ oid: git2::Oid,
+ remote: &Remote<LocalUrl>,
+ msg: &str,
+ ) -> Result<(), Error> {
+ let _remote_branch = repo.reference(
+ reflike!("refs/remotes")
+ .join(remote.name.clone())
+ .join(branch.clone())
+ .as_str(),
+ oid,
+ true,
+ msg,
+ )?;
+ let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, msg);
+ Ok(())
+ }
+
/// Clone a git repository to the `path` location, based off of the `remote`
/// provided.
///
@@ -188,29 +211,39 @@ pub fn clone<F>(
path: &Path,
storage: F,
mut remote: Remote<LocalUrl>,
+ init_branch: Option<&git_ext::RefLike>,
) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
where
F: CanOpenStorage + 'static,
{
let repo = git2::Repository::init(path)?;
remote.save(&repo)?;
- for (reference, oid) in remote.fetch(storage, &repo, LocalFetchspec::Configured)? {
- let msg = format!("Fetched `{}->{}`", reference, oid);
- tracing::debug!("{}", msg);
-
- let branch: git_ext::RefLike = OneLevel::from(reference).into();
- let branch = branch.strip_prefix(remote.name.clone())?;
- let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
- let _remote_branch = repo.reference(
- reflike!("refs/remotes")
- .join(remote.name.clone())
- .join(branch.clone())
- .as_str(),
- oid,
- true,
- &msg,
- )?;
- let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, &msg);
+
+ let branches: Vec<_> = remote
+ .fetch(storage, &repo, LocalFetchspec::Configured)?
+ .map(|(reference, oid)| {
+ let msg = format!("Fetched `{}->{}`", reference, oid);
+ tracing::debug!("{}", msg);
+
+ let branch: git_ext::RefLike = OneLevel::from(reference).into();
+ let branch = branch.strip_prefix(remote.name.clone()).unwrap();
+ let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
+ (branch, oid, msg)
+ })
+ .collect();
+ match init_branch {
+ Some(init_branch) => {
+ let (branch, oid, msg) = branches
+ .iter()
+ .find(|(branch, _, _)| branch == init_branch)
+ .ok_or(Error::MissingInitialBranch(init_branch.clone()))?;
+ create_local_ref(&repo, branch.clone(), *oid, &remote, msg)?;
+ },
+ None => {
+ for (branch, oid, msg) in branches {
+ create_local_ref(&repo, branch, oid, &remote, &msg)?
+ }
+ },
}
Ok((repo, remote))
diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
index b85163dd..5f9e86e2 100644
--- a/cli/lnk-identities/src/git/checkout.rs
+++ b/cli/lnk-identities/src/git/checkout.rs
@@ -139,17 +139,21 @@ where
pub struct Local {
url: LocalUrl,
path: PathBuf,
+ default_branch: Option<OneLevel>,
}
impl Local {
- pub fn new<I>(identity: &I, path: PathBuf) -> Self
+ pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
where
- I: HasName + HasUrn,
+ I: HasBranch + HasName + HasUrn,
{
- Self {
- url: LocalUrl::from(identity.urn()),
+ let urn = identity.urn();
+ let default_branch = identity.branch();
+ Ok(Self {
+ url: LocalUrl::from(urn),
path,
- }
+ default_branch,
+ })
}
fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
@@ -164,7 +168,12 @@ impl Local {
force: Force::True,
},
);
- Ok(git::clone(&self.path, open_storage, rad)?)
+ Ok(git::clone(
+ &self.path,
+ open_storage,
+ rad,
+ self.default_branch.map(|v| RefLike::from(v)).as_ref(),
+ )?)
}
}
@@ -209,7 +218,12 @@ impl Peer {
force: Force::True,
}]);
- let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
+ let (repo, _) = git::clone(
+ &self.path,
+ open_storage.clone(),
+ remote,
+ Some(&RefLike::from(self.default_branch.clone())),
+ )?;
// Create a rad remote and push the default branch so we can set it as the
// upstream.
@@ -248,7 +262,7 @@ where
{
let path = path.resolve(identity.name());
Ok(match remote {
- None => Either::Left(Local::new(identity, path)),
+ None => Either::Left(Local::new(identity, path)?),
Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
})
}
diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
index 5177eb38..1ca7d8fb 100644
--- a/cli/lnk-identities/t/src/tests/git/checkout.rs
+++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
@@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
signer: signer.into(),
};
- let local = Local::new(&proj.project, temp.path().to_path_buf());
+ let local = Local::new(&proj.project, temp.path().to_path_buf())?;
let repo = checkout(
&paths,
settings,
--
2.35.3
Re: [PATCH radicle-link v3 0/1] lnk-identities: optionally use initial branch on git clone
[PATCH radicle-link v4 0/1] lnk-identities: optionally use initial branch on git clone
Amend commits with sign-off
Slack Coder (1):
lnk-identities: allow initial branch on git clone
cli/lnk-identities/src/git.rs | 67 ++++++++++++++-----
cli/lnk-identities/src/git/checkout.rs | 30 ++++++---
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 73 insertions(+), 26 deletions(-)
--
2.35.3
[PATCH radicle-link v4 1/1] lnk-identities: allow initial branch on git clone
Better follow git's command line behavior by setting one local branch, the
projects default head, on clone. It is assumed to be the project
identities 'default_branch' field.
Implement the new clone argument as optional to keep backward
compatibility.
Signed-off-by: Slack Coder <slackcoder@server.ky>
---
cli/lnk-identities/src/git.rs | 67 ++++++++++++++ -----
cli/lnk-identities/src/git/checkout.rs | 30 ++++++ ---
.../t/src/tests/git/checkout.rs | 2 +-
3 files changed, 73 insertions(+), 26 deletions(-)
diff --git a/cli/lnk-identities/src/git.rs b/cli/lnk-identities/src/git.rs
index c9ed5e90..c31fefc3 100644
--- a/cli/lnk-identities/src/git.rs
+++ b/cli/lnk-identities/src/git.rs
@@ -49,6 +49,9 @@ pub enum Error {
#[error(transparent)]
Git(#[from] git2::Error),
+
+ #[error("failed to clone project with initial branch `{0}`, the branch is missing")]
+ MissingInitialBranch(git_ext::RefLike),
}
/// Equips a repository with a rad remote for the given id. If the directory at
@@ -178,6 +181,26 @@ pub fn set_upstream<Url>(
Ok(())
}
+ fn create_local_ref(
+ repo: &git2::Repository,
+ branch: git_ext::RefLike,
+ oid: git2::Oid,
+ remote: &Remote<LocalUrl>,
+ msg: &str,
+ ) -> Result<(), Error> {
+ let _remote_branch = repo.reference(
+ reflike!("refs/remotes")
+ .join(remote.name.clone())
+ .join(branch.clone())
+ .as_str(),
+ oid,
+ true,
+ msg,
+ )?;
+ let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, msg);
+ Ok(())
+ }
+
/// Clone a git repository to the `path` location, based off of the `remote`
/// provided.
///
@@ -188,29 +211,39 @@ pub fn clone<F>(
path: &Path,
storage: F,
mut remote: Remote<LocalUrl>,
+ init_branch: Option<&git_ext::RefLike>,
) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
where
F: CanOpenStorage + 'static,
{
let repo = git2::Repository::init(path)?;
remote.save(&repo)?;
- for (reference, oid) in remote.fetch(storage, &repo, LocalFetchspec::Configured)? {
- let msg = format!("Fetched `{}->{}`", reference, oid);
- tracing::debug!("{}", msg);
-
- let branch: git_ext::RefLike = OneLevel::from(reference).into();
- let branch = branch.strip_prefix(remote.name.clone())?;
- let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
- let _remote_branch = repo.reference(
- reflike!("refs/remotes")
- .join(remote.name.clone())
- .join(branch.clone())
- .as_str(),
- oid,
- true,
- &msg,
- )?;
- let _local_branch = repo.reference(Qualified::from(branch).as_str(), oid, true, &msg);
+
+ let branches: Vec<_> = remote
+ .fetch(storage, &repo, LocalFetchspec::Configured)?
+ .map(|(reference, oid)| {
+ let msg = format!("Fetched `{}->{}`", reference, oid);
+ tracing::debug!("{}", msg);
+
+ let branch: git_ext::RefLike = OneLevel::from(reference).into();
+ let branch = branch.strip_prefix(remote.name.clone()).unwrap();
+ let branch = branch.strip_prefix(reflike!("heads")).unwrap_or(branch);
+ (branch, oid, msg)
+ })
+ .collect();
+ match init_branch {
+ Some(init_branch) => {
+ let (branch, oid, msg) = branches
+ .iter()
+ .find(|(branch, _, _)| branch == init_branch)
+ .ok_or(Error::MissingInitialBranch(init_branch.clone()))?;
+ create_local_ref(&repo, branch.clone(), *oid, &remote, msg)?;
+ },
+ None => {
+ for (branch, oid, msg) in branches {
+ create_local_ref(&repo, branch, oid, &remote, &msg)?
+ }
+ },
}
Ok((repo, remote))
diff --git a/cli/lnk-identities/src/git/checkout.rs b/cli/lnk-identities/src/git/checkout.rs
index b85163dd..5f9e86e2 100644
--- a/cli/lnk-identities/src/git/checkout.rs
+++ b/cli/lnk-identities/src/git/checkout.rs
@@ -139,17 +139,21 @@ where
pub struct Local {
url: LocalUrl,
path: PathBuf,
+ default_branch: Option<OneLevel>,
}
impl Local {
- pub fn new<I>(identity: &I, path: PathBuf) -> Self
+ pub fn new<I>(identity: &I, path: PathBuf) -> Result<Self, Error>
where
- I: HasName + HasUrn,
+ I: HasBranch + HasName + HasUrn,
{
- Self {
- url: LocalUrl::from(identity.urn()),
+ let urn = identity.urn();
+ let default_branch = identity.branch();
+ Ok(Self {
+ url: LocalUrl::from(urn),
path,
- }
+ default_branch,
+ })
}
fn checkout<F>(self, open_storage: F) -> Result<(git2::Repository, Remote<LocalUrl>), Error>
@@ -164,7 +168,12 @@ impl Local {
force: Force::True,
},
);
- Ok(git::clone(&self.path, open_storage, rad)?)
+ Ok(git::clone(
+ &self.path,
+ open_storage,
+ rad,
+ self.default_branch.map(|v| RefLike::from(v)).as_ref(),
+ )?)
}
}
@@ -209,7 +218,12 @@ impl Peer {
force: Force::True,
}]);
- let (repo, _) = git::clone(&self.path, open_storage.clone(), remote)?;
+ let (repo, _) = git::clone(
+ &self.path,
+ open_storage.clone(),
+ remote,
+ Some(&RefLike::from(self.default_branch.clone())),
+ )?;
// Create a rad remote and push the default branch so we can set it as the
// upstream.
@@ -248,7 +262,7 @@ where
{
let path = path.resolve(identity.name());
Ok(match remote {
- None => Either::Left(Local::new(identity, path)),
+ None => Either::Left(Local::new(identity, path)?),
Some(remote) => Either::Right(Peer::new(identity, remote, path)?),
})
}
diff --git a/cli/lnk-identities/t/src/tests/git/checkout.rs b/cli/lnk-identities/t/src/tests/git/checkout.rs
index 5177eb38..1ca7d8fb 100644
--- a/cli/lnk-identities/t/src/tests/git/checkout.rs
+++ b/cli/lnk-identities/t/src/tests/git/checkout.rs
@@ -47,7 +47,7 @@ fn local_checkout() -> anyhow::Result<()> {
signer: signer.into(),
};
- let local = Local::new(&proj.project, temp.path().to_path_buf());
+ let local = Local::new(&proj.project, temp.path().to_path_buf())?;
let repo = checkout(
&paths,
settings,
--
2.35.3
Re: [PATCH radicle-link v4 0/1] lnk-identities: optionally use initial branch on git clone
Thanks! Merged :)
Master-at: 904250f533a6482ebfa4bfb7e3660a4982f07ed8