The first patch was covered in another patch by Alex, so may be dropped during the merge. We originally thought the include file was not being saved, but due to myself not have a rad/self setup, initially, the tracked peer was filtered out. Instead, we keep all remote tracked peers and either output `<name>@<peer id>` or `<peer id>` depending on the existence of a rad/self for that tracked peer. Published-as: https://github.com/FintanH/radicle-link/tree/patches/include-remote-improvements%2Fv1 Fintan Halpenny (2): gitd: cargo fmt lnk-identities: make name optional for include remotes cli/gitd-lib/src/args.rs | 5 ++++- cli/lnk-identities/src/git/include.rs | 9 ++++++--- librad/src/git/include.rs | 26 +++++++++++++++----------- librad/t/src/tests/git/include.rs | 5 ++--- link-identities/src/relations.rs | 14 ++++++++++++++ 5 files changed, 41 insertions(+), 18 deletions(-) -- 2.31.1
radicle-link/patches/nixos-latest.yml: FAILED in 55s [Make <name>@ optional for include remotes][0] from [Fintan Halpenny][1] [0]: https://lists.sr.ht/~radicle-link/dev/patches/33349 [1]: mailto:fintan.halpenny@gmail.com ✗ #789736 FAILED radicle-link/patches/nixos-latest.yml https://builds.sr.ht/~radicle-link/job/789736
LGTM
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~radicle-link/dev/patches/33349/mbox | git am -3Learn more about email & git
Formatting of the args module in gitd slipped by formatting check. Format args.rs to allow for the check to pass. Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com> --- cli/gitd-lib/src/args.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cli/gitd-lib/src/args.rs b/cli/gitd-lib/src/args.rs index 2ec4d966..1404f73d 100644 --- a/cli/gitd-lib/src/args.rs +++ b/cli/gitd-lib/src/args.rs @@ -61,7 +61,10 @@ impl Args { self, spawner: Arc<link_async::Spawner>, ) -> Result<Config<BoxedSigner>, Error> { - let home = self.lnk_home.map(LnkHome::Root).unwrap_or(LnkHome::ProjectDirs); + let home = self + .lnk_home + .map(LnkHome::Root) + .unwrap_or(LnkHome::ProjectDirs); let profile = Profile::from_home(&home, None)?; let signer = spawner .blocking({ -- 2.31.1
Formatting of the args module in gitd slipped by formatting check. Format args.rs to allow for the check to pass. Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com> --- cli/gitd-lib/src/args.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cli/gitd-lib/src/args.rs b/cli/gitd-lib/src/args.rs index 2ec4d966..1404f73d 100644 --- a/cli/gitd-lib/src/args.rs +++ b/cli/gitd-lib/src/args.rs @@ -61,7 +61,10 @@ impl Args { self, spawner: Arc<link_async::Spawner>, ) -> Result<Config<BoxedSigner>, Error> { - let home = self.lnk_home.map(LnkHome::Root).unwrap_or(LnkHome::ProjectDirs); + let home = self + .lnk_home + .map(LnkHome::Root) + .unwrap_or(LnkHome::ProjectDirs); let profile = Profile::from_home(&home, None)?; let signer = spawner .blocking({ -- 2.31.1
If a remote peer does not include a `rad/self` then the include generation would not include them as a git remote. The use of `rad/self` is to simply supply the `name` so that remotes can be more easily remembered and are human-friendly. This does not mean that they are necessary. Make `<name>@` optional for remotes added to include files, falling back to `<peer id>` only as the remote name. Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com> --- cli/lnk-identities/src/git/include.rs | 9 ++++++--- librad/src/git/include.rs | 26 +++++++++++++++----------- librad/t/src/tests/git/include.rs | 5 ++--- link-identities/src/relations.rs | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/cli/lnk-identities/src/git/include.rs b/cli/lnk-identities/src/git/include.rs index a4e64cc9..c56858ea 100644 --- a/cli/lnk-identities/src/git/include.rs +++ b/cli/lnk-identities/src/git/include.rs @@ -54,9 +54,12 @@ where tracked .into_iter() .filter_map(|peer| { - relations::Peer::replicated_remote(peer).map(|(p, u)| { - git_ext::RefLike::try_from(u.person().subject().name.to_string()) - .map(|r| (r, p)) + peer.remote().map(|(peer, status)| match status { + relations::Status::NotReplicated => Ok((None, peer)), + relations::Status::Replicated(relations::Replicated { user }) => { + git_ext::RefLike::try_from(user.person().subject().name.to_string()) + .map(|handle| (Some(handle), peer)) + }, }) }) .collect::<Result<Vec<_>, _>>()?, diff --git a/librad/src/git/include.rs b/librad/src/git/include.rs index 8384ce6e..1d0b3f5d 100644 --- a/librad/src/git/include.rs +++ b/librad/src/git/include.rs @@ -81,7 +81,10 @@ impl<Path> Include<Path> { } } - pub fn add_remote(&mut self, url: LocalUrl, peer: PeerId, handle: impl Into<ext::RefLike>) { + pub fn add_remote<H>(&mut self, url: LocalUrl, peer: PeerId, handle: H) + where + H: Into<Option<ext::RefLike>>, + { let remote = Self::build_remote(url, peer, handle); self.remotes.push(remote); } @@ -148,12 +151,12 @@ impl<Path> Include<Path> { pub fn from_tracked_persons<R, I>(path: Path, local_url: LocalUrl, tracked: I) -> Self where Path: Debug, - R: Into<ext::RefLike>, + R: Into<Option<ext::RefLike>>, I: IntoIterator<Item = (R, PeerId)>, { let remotes = tracked .into_iter() - .map(|(handle, peer)| Self::build_remote(local_url.clone(), peer, handle.into())) + .map(|(handle, peer)| Self::build_remote(local_url.clone(), peer, handle)) .collect(); tracing::trace!("computed remotes: {:?}", remotes); @@ -164,14 +167,15 @@ impl<Path> Include<Path> { } } - fn build_remote( - url: LocalUrl, - peer: PeerId, - handle: impl Into<ext::RefLike>, - ) -> Remote<LocalUrl> { - let handle = handle.into(); - let name = ext::RefLike::try_from(format!("{}@{}", handle, peer)) - .expect("handle and peer are reflike"); + fn build_remote<H>(url: LocalUrl, peer: PeerId, handle: H) -> Remote<LocalUrl> + where + H: Into<Option<ext::RefLike>>, + { + let name = match handle.into() { + Some(handle) => ext::RefLike::try_from(format!("{}@{}", handle, peer)) + .expect("handle and peer are reflike"), + None => ext::RefLike::from(peer), + }; Remote::new(url, name.clone()).with_fetchspecs(vec![Refspec { src: Reference::heads(Flat, peer), dst: GenericRef::heads(Flat, name), diff --git a/librad/t/src/tests/git/include.rs b/librad/t/src/tests/git/include.rs index 76687990..db26c5f3 100644 --- a/librad/t/src/tests/git/include.rs +++ b/librad/t/src/tests/git/include.rs @@ -35,7 +35,6 @@ const LINGLING_SEED: [u8; 32] = [ lazy_static! { static ref LYLA_HANDLE: ext::RefLike = reflike!("lyla"); static ref ROVER_HANDLE: ext::RefLike = reflike!("rover"); - static ref LINGLING_HANDLE: ext::RefLike = reflike!("lingling"); static ref LOCAL_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(LOCAL_SEED)); static ref LYLA_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(LYLA_SEED)); static ref ROVER_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(ROVER_SEED)); @@ -113,11 +112,11 @@ fn can_create_and_update() -> Result<(), Error> { ); // The tracking graph changed entirely. - let remote_lingling = format!("{}@{}", *LINGLING_HANDLE, *LINGLING_PEER_ID); + let remote_lingling = format!("{}", *LINGLING_PEER_ID); { let mut include = Include::new(tmp_dir.path().to_path_buf(), url.clone()); - include.add_remote(url, *LINGLING_PEER_ID, (*LINGLING_HANDLE).clone()); + include.add_remote(url, *LINGLING_PEER_ID, None); include.save()?; }; diff --git a/link-identities/src/relations.rs b/link-identities/src/relations.rs index ac98f845..d8984f05 100644 --- a/link-identities/src/relations.rs +++ b/link-identities/src/relations.rs @@ -92,6 +92,20 @@ impl<U> Peer<Status<U>> { Self::Local { .. } | Self::Remote { .. } => None, } } + + pub fn local(self) -> Option<(PeerId, Status<U>)> { + match self { + Peer::Local { peer_id, status } => Some((peer_id, status)), + Peer::Remote { .. } => None, + } + } + + pub fn remote(self) -> Option<(PeerId, Status<U>)> { + match self { + Peer::Local { .. } => None, + Peer::Remote { peer_id, status } => Some((peer_id, status)), + } + } } impl<S> Peer<S> { -- 2.31.1
builds.sr.ht <builds@sr.ht>radicle-link/patches/nixos-latest.yml: FAILED in 55s [Make <name>@ optional for include remotes][0] from [Fintan Halpenny][1] [0]: https://lists.sr.ht/~radicle-link/dev/patches/33349 [1]: mailto:fintan.halpenny@gmail.com ✗ #789736 FAILED radicle-link/patches/nixos-latest.yml https://builds.sr.ht/~radicle-link/job/789736
If a remote peer does not include a `rad/self` then the include generation would not include them as a git remote. The use of `rad/self` is to simply supply the `name` so that remotes can be more easily remembered and are human-friendly. This does not mean that they are necessary. Make `<name>@` optional for remotes added to include files, falling back to `<peer id>` only as the remote name. Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com> --- cli/lnk-identities/src/git/include.rs | 9 ++++++--- librad/src/git/include.rs | 26 +++++++++++++++----------- librad/t/src/tests/git/include.rs | 5 ++--- link-identities/src/relations.rs | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/cli/lnk-identities/src/git/include.rs b/cli/lnk-identities/src/git/include.rs index a4e64cc9..c56858ea 100644 --- a/cli/lnk-identities/src/git/include.rs +++ b/cli/lnk-identities/src/git/include.rs @@ -54,9 +54,12 @@ where tracked .into_iter() .filter_map(|peer| { - relations::Peer::replicated_remote(peer).map(|(p, u)| { - git_ext::RefLike::try_from(u.person().subject().name.to_string()) - .map(|r| (r, p)) + peer.remote().map(|(peer, status)| match status { + relations::Status::NotReplicated => Ok((None, peer)), + relations::Status::Replicated(relations::Replicated { user }) => { + git_ext::RefLike::try_from(user.person().subject().name.to_string()) + .map(|handle| (Some(handle), peer)) + }, }) }) .collect::<Result<Vec<_>, _>>()?, diff --git a/librad/src/git/include.rs b/librad/src/git/include.rs index 8384ce6e..1d0b3f5d 100644 --- a/librad/src/git/include.rs +++ b/librad/src/git/include.rs @@ -81,7 +81,10 @@ impl<Path> Include<Path> { } } - pub fn add_remote(&mut self, url: LocalUrl, peer: PeerId, handle: impl Into<ext::RefLike>) { + pub fn add_remote<H>(&mut self, url: LocalUrl, peer: PeerId, handle: H) + where + H: Into<Option<ext::RefLike>>, + { let remote = Self::build_remote(url, peer, handle); self.remotes.push(remote); } @@ -148,12 +151,12 @@ impl<Path> Include<Path> { pub fn from_tracked_persons<R, I>(path: Path, local_url: LocalUrl, tracked: I) -> Self where Path: Debug, - R: Into<ext::RefLike>, + R: Into<Option<ext::RefLike>>, I: IntoIterator<Item = (R, PeerId)>, { let remotes = tracked .into_iter() - .map(|(handle, peer)| Self::build_remote(local_url.clone(), peer, handle.into())) + .map(|(handle, peer)| Self::build_remote(local_url.clone(), peer, handle)) .collect(); tracing::trace!("computed remotes: {:?}", remotes); @@ -164,14 +167,15 @@ impl<Path> Include<Path> { } } - fn build_remote( - url: LocalUrl, - peer: PeerId, - handle: impl Into<ext::RefLike>, - ) -> Remote<LocalUrl> { - let handle = handle.into(); - let name = ext::RefLike::try_from(format!("{}@{}", handle, peer)) - .expect("handle and peer are reflike"); + fn build_remote<H>(url: LocalUrl, peer: PeerId, handle: H) -> Remote<LocalUrl> + where + H: Into<Option<ext::RefLike>>, + { + let name = match handle.into() { + Some(handle) => ext::RefLike::try_from(format!("{}@{}", handle, peer)) + .expect("handle and peer are reflike"), + None => ext::RefLike::from(peer), + }; Remote::new(url, name.clone()).with_fetchspecs(vec![Refspec { src: Reference::heads(Flat, peer), dst: GenericRef::heads(Flat, name), diff --git a/librad/t/src/tests/git/include.rs b/librad/t/src/tests/git/include.rs index 76687990..db26c5f3 100644 --- a/librad/t/src/tests/git/include.rs +++ b/librad/t/src/tests/git/include.rs @@ -35,7 +35,6 @@ const LINGLING_SEED: [u8; 32] = [ lazy_static! { static ref LYLA_HANDLE: ext::RefLike = reflike!("lyla"); static ref ROVER_HANDLE: ext::RefLike = reflike!("rover"); - static ref LINGLING_HANDLE: ext::RefLike = reflike!("lingling"); static ref LOCAL_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(LOCAL_SEED)); static ref LYLA_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(LYLA_SEED)); static ref ROVER_PEER_ID: PeerId = PeerId::from(SecretKey::from_seed(ROVER_SEED)); @@ -113,11 +112,11 @@ fn can_create_and_update() -> Result<(), Error> { ); // The tracking graph changed entirely. - let remote_lingling = format!("{}@{}", *LINGLING_HANDLE, *LINGLING_PEER_ID); + let remote_lingling = format!("{}", *LINGLING_PEER_ID); { let mut include = Include::new(tmp_dir.path().to_path_buf(), url.clone()); - include.add_remote(url, *LINGLING_PEER_ID, (*LINGLING_HANDLE).clone()); + include.add_remote(url, *LINGLING_PEER_ID, None); include.save()?; }; diff --git a/link-identities/src/relations.rs b/link-identities/src/relations.rs index ac98f845..d8984f05 100644 --- a/link-identities/src/relations.rs +++ b/link-identities/src/relations.rs @@ -92,6 +92,20 @@ impl<U> Peer<Status<U>> { Self::Local { .. } | Self::Remote { .. } => None, } } + + pub fn local(self) -> Option<(PeerId, Status<U>)> { + match self { + Peer::Local { peer_id, status } => Some((peer_id, status)), + Peer::Remote { .. } => None, + } + } + + pub fn remote(self) -> Option<(PeerId, Status<U>)> { + match self { + Peer::Local { .. } => None, + Peer::Remote { peer_id, status } => Some((peer_id, status)), + } + } } impl<S> Peer<S> { -- 2.31.1