~radicle-link/dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH radicle-link v1] lnk-identities: improve track and untrack

Details
Message ID
<20220623141806.111141-1-fintan.halpenny@gmail.com>
DKIM signature
missing
Download raw message
Patch: +115 -30
The previous version of track and untrack were not updated to take
advantage of the latest `link-tracking` changes. As well as this, the
`inlcude::update` call would fail if the `urn` did not exist, making
it look like the tracking call failed at command line.

To remedy the former, make the `peer` optional to allow for default
tracking. Introduce `config` and `force` to be able to specify the
configuration used for the tracking and if it should forcefully
overwrite the configuration, respectively.

To remedy the later, check for `NotFound` error that can be returned by
the `include::update` call -- logging a warning.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
---

Published-as:
    urn: rad:git:hnrkemwx6gd9r9uw1gqgc9x4urewtnamkwy5o
    seed: hydtac74mgo8xeh34cy7tmzzfejcybmxgfyawhnb4zj8wxxo4qckgh@seed.lnk.network:8799
    remote: hyytgji3qmjmq86ti4b3idgi1wjexowp9jfxmdosreu7qn4t77589e
    tag: patches/lnk-identities/tracking-improvements/v1
Published-as: https://github.com/FintanH/radicle-link/tree/patches/lnk-identities%2Ftracking-improvements%2Fv1

 cli/lnk-identities/src/cli/args.rs          | 21 +++++-
 cli/lnk-identities/src/cli/eval/tracking.rs | 47 +++++++++++--
 cli/lnk-identities/src/tracking.rs          | 77 +++++++++++++++------
 3 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/cli/lnk-identities/src/cli/args.rs b/cli/lnk-identities/src/cli/args.rs
index 1a281256..d790f53e 100644
--- a/cli/lnk-identities/src/cli/args.rs
+++ b/cli/lnk-identities/src/cli/args.rs
@@ -10,7 +10,7 @@ use either::Either;

use librad::{
    crypto::PublicKey,
    git::Urn,
    git::{self, Urn},
    identities::{
        git::Revision,
        payload::{self, KeyOrUrn},
@@ -681,9 +681,19 @@ pub mod tracking {
        #[clap(long)]
        pub urn: Urn,

        /// the peer to track
        /// the peer to track, if not provided the default tracking relationship
        /// will be created
        #[clap(long)]
        pub peer: PeerId,
        pub peer: Option<PeerId>,

        /// the configuration to use when tracking the given peer, if none is
        /// provided the default configuration will be used
        #[clap(long)]
        pub config: Option<git::tracking::Config>,

        /// create the tracking relationship even if one already existed
        #[clap(long, short)]
        pub force: bool,
    }

    /// untrack a peer's gossip for a Radicle URN
@@ -696,6 +706,11 @@ pub mod tracking {
        /// the peer to untrack
        #[clap(long)]
        pub peer: PeerId,

        /// prune any references for the provided `--urn` and `--peer` from the
        /// storage
        #[clap(long)]
        pub prune: bool,
    }
}

diff --git a/cli/lnk-identities/src/cli/eval/tracking.rs b/cli/lnk-identities/src/cli/eval/tracking.rs
index 34919e12..47521154 100644
--- a/cli/lnk-identities/src/cli/eval/tracking.rs
+++ b/cli/lnk-identities/src/cli/eval/tracking.rs
@@ -3,7 +3,10 @@
// This file is part of radicle-link, distributed under the GPLv3 with Radicle
// Linking Exception. For full terms see the included LICENSE file.

use librad::profile::Profile;
use librad::{
    git::tracking::{git::refdb::PrunedRef, policy, UntrackArgs, Untracked},
    profile::Profile,
};
use lnk_clib::{keys::ssh::SshAuthSock, storage::ssh};

use crate::{cli::args::tracking::*, tracking};
@@ -11,21 +14,55 @@ use crate::{cli::args::tracking::*, tracking};
pub fn eval_track(
    profile: &Profile,
    sock: SshAuthSock,
    Track { urn, peer }: Track,
    Track {
        urn,
        peer,
        config,
        force,
    }: Track,
) -> anyhow::Result<()> {
    let (_, storage) = ssh::storage(profile, sock)?;
    let paths = profile.paths();
    tracking::track(&storage, paths, &urn, peer)?;
    let policy = if force {
        policy::Track::Any
    } else {
        policy::Track::MustNotExist
    };
    match tracking::track(&storage, paths, &urn, peer, config, policy)? {
        Ok(r) => println!("created tracking relationship `{}`", r.name),
        Err(err) => eprintln!("could not create tracking relationship: {}", err),
    }
    Ok(())
}

pub fn eval_untrack(
    profile: &Profile,
    sock: SshAuthSock,
    Untrack { urn, peer }: Untrack,
    Untrack { urn, peer, prune }: Untrack,
) -> anyhow::Result<()> {
    let (_, storage) = ssh::storage(profile, sock)?;
    let paths = profile.paths();
    tracking::untrack(&storage, paths, &urn, peer)?;
    let args = UntrackArgs {
        policy: policy::Untrack::Any,
        prune,
    };
    match tracking::untrack(&storage, paths, &urn, peer, args)? {
        Ok(Untracked { pruned, .. }) => {
            println!("untracked `{}` for `{}`", urn, peer);
            if let Some(pruned) = pruned {
                for prune in pruned.0 {
                    match prune {
                        PrunedRef::Direct { name, .. } => println!("pruned reference `{}`", name),
                        PrunedRef::Symbolic { name } => {
                            println!("pruned symbolic reference `{}`", name)
                        },
                    }
                }
            }
        },
        // NOTE: this shouldn't be possible because of the use of `Untrack::Any` above, but keeping
        // it here for posterity.
        Err(err) => eprintln!("could not remove tracking relationship: {}", err),
    }
    Ok(())
}
diff --git a/cli/lnk-identities/src/tracking.rs b/cli/lnk-identities/src/tracking.rs
index ea7c74ba..f504b6af 100644
--- a/cli/lnk-identities/src/tracking.rs
+++ b/cli/lnk-identities/src/tracking.rs
@@ -6,10 +6,11 @@
use thiserror::Error;

use librad::{
    git::{storage::Storage, tracking, Urn},
    git::{identities, storage::Storage, tracking, Urn},
    paths::Paths,
    PeerId,
};
use std_ext::result::ResultExt as _;

use crate::git::include;

@@ -26,27 +27,59 @@ pub enum Error {
    Untrack(#[from] tracking::error::Untrack),
}

// TODO(finto): allow specification of Config
// TODO(finto): perhaps we want a flag to force track?
pub fn track(storage: &Storage, paths: &Paths, urn: &Urn, peer: PeerId) -> Result<(), Error> {
    let _tracked = tracking::track(
        storage,
        urn,
        Some(peer),
        tracking::Config::default(),
        tracking::policy::Track::Any,
    )?;
    include::update(storage, paths, urn)?;
    Ok(())
/// Track the given `urn` and `peer`. This will call
/// [`include::update`] for modifying the include file for the given
/// identity. If the `urn` does not exist in the storage, then no
/// action will be take for the include file.
///
/// See [`tracking::track`] for more on the semantics of tracking.
pub fn track(
    storage: &Storage,
    paths: &Paths,
    urn: &Urn,
    peer: Option<PeerId>,
    config: Option<tracking::Config>,
    policy: tracking::policy::Track,
) -> Result<Result<tracking::Ref, tracking::PreviousError>, Error> {
    let tracked = tracking::track(storage, urn, peer, config.unwrap_or_default(), policy)?;
    include::update(storage, paths, urn)
        .map(|path| tracing::info!(?path, "updated include file"))
        .or_matches::<Error, _, _>(is_not_found, || {
            tracing::warn!(%urn, "could not update include file, the URN did not exist");
            Ok(())
        })?;
    Ok(tracked)
}

pub fn untrack(storage: &Storage, paths: &Paths, urn: &Urn, peer: PeerId) -> Result<(), Error> {
    let _untracked = tracking::untrack(
        storage,
        urn,
        peer,
        tracking::UntrackArgs::new(tracking::policy::Untrack::Any),
    )?;
    include::update(storage, paths, urn)?;
    Ok(())
/// Track the given `urn` and `peer`. This will call
/// [`include::update`] for modifying the include file for the given
/// identity. If the `urn` does not exist in the storage, then no
/// action will be take for the include file.
///
/// See [`tracking::untrack`] for more on the semantics of untracking.
pub fn untrack(
    storage: &Storage,
    paths: &Paths,
    urn: &Urn,
    peer: PeerId,
    args: tracking::UntrackArgs,
) -> Result<Result<tracking::Untracked<String>, tracking::PreviousError>, Error> {
    let untracked = tracking::untrack(storage, urn, peer, args)?;
    include::update(storage, paths, urn)
        .map(|path| tracing::info!(?path, "updated include file"))
        .or_matches::<Error, _, _>(is_not_found, || {
            tracing::warn!(%urn, "could not update include file, the URN did not exist");
            Ok(())
        })?;
    Ok(untracked)
}

fn is_not_found(err: &include::Error) -> bool {
    match err {
        include::Error::Identities(identities::error::Error::NotFound(_))
        | include::Error::Relations(identities::relations::Error::Identities(
            identities::error::Error::NotFound(_),
        )) => true,
        _ => false,
    }
}
-- 
2.31.1

[radicle-link/patches/nixos-latest.yml] build failed

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CKXL8B9XZLR5.3N0051TZ2HE49@cirno>
In-Reply-To
<20220623141806.111141-1-fintan.halpenny@gmail.com> (view parent)
DKIM signature
missing
Download raw message
radicle-link/patches/nixos-latest.yml: FAILED in 18m15s

[lnk-identities: improve track and untrack][0] from [Fintan Halpenny][1]

[0]: https://lists.sr.ht/~radicle-link/dev/patches/33214
[1]: fintan.halpenny@gmail.com

✗ #786303 FAILED radicle-link/patches/nixos-latest.yml https://builds.sr.ht/~radicle-link/job/786303

Re: [radicle-link/patches/nixos-latest.yml] build failed

Details
Message ID
<CAH_DpYSdYjNmz-2qrXncxqzybsph5GdQMx1PHYHnYP9dwLFG-g@mail.gmail.com>
In-Reply-To
<CKXL8B9XZLR5.3N0051TZ2HE49@cirno> (view parent)
DKIM signature
missing
Download raw message
On 23/06/22 02:36pm, builds.sr.ht wrote:
> radicle-link/patches/nixos-latest.yml: FAILED in 18m15s
>
> [lnk-identities: improve track and untrack][0] from [Fintan Halpenny][1]
>
> [0]: https://lists.sr.ht/~radicle-link/dev/patches/33214
> [1]: fintan.halpenny@gmail.com
>
> ✗ #786303 FAILED radicle-link/patches/nixos-latest.yml https://builds.sr.ht/~radicle-link/job/786303

This is actually not a false positive, it's failing due to a clippy
complaint:

    error: match expression looks like `matches!` macro
      --> cli/lnk-identities/src/tracking.rs:78:5
       |
    78 | /     match err {
    79 | |
include::Error::Identities(identities::error::Error::NotFound(_))
    80 | |         |
include::Error::Relations(identities::relations::Error::Identities(
    81 | |             identities::error::Error::NotFound(_),
    82 | |         )) => true,
    83 | |         _ => false,
    84 | |     }
       | |_____^
       |
       = note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
       = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro
    help: try this
       |
    78 ~     matches!(err,
include::Error::Identities(identities::error::Error::NotFound(_))
    79 +         |
include::Error::Relations(identities::relations::Error::Identities(
    80 +             identities::error::Error::NotFound(_),
    81 +         )))
       |

[PATCH radicle-link v2] lnk-identities: improve track and untrack

Details
Message ID
<20220629120019.66961-1-fintan.halpenny@gmail.com>
In-Reply-To
<20220623141806.111141-1-fintan.halpenny@gmail.com> (view parent)
DKIM signature
missing
Download raw message
Patch: +115 -30
The previous version of track and untrack were not updated to take
advantage of the latest `link-tracking` changes. As well as this, the
`inlcude::update` call would fail if the `urn` did not exist, making
it look like the tracking call failed at command line.

To remedy the former, make the `peer` optional to allow for default
tracking. Introduce `config` and `force` to be able to specify the
configuration used for the tracking and if it should forcefully
overwrite the configuration, respectively.

To remedy the later, check for `NotFound` error that can be returned by
the `include::update` call -- logging a warning.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
---

Changes in v2:
* Adhere to clippy's ruling state and use `matches!` instead.

Published-as: https://github.com/FintanH/radicle-link/tree/patches/lnk-identities%2Ftracking-improvements%2Fv2

Range-diff against v1:
1:  1b70642a ! 1:  1209e28e lnk-identities: improve track and untrack
    @@ cli/lnk-identities/src/tracking.rs: pub enum Error {
     +}
     +
     +fn is_not_found(err: &include::Error) -> bool {
    -+    match err {
    ++    matches!(
    ++        err,
     +        include::Error::Identities(identities::error::Error::NotFound(_))
    -+        | include::Error::Relations(identities::relations::Error::Identities(
    -+            identities::error::Error::NotFound(_),
    -+        )) => true,
    -+        _ => false,
    -+    }
    ++            | include::Error::Relations(identities::relations::Error::Identities(
    ++                identities::error::Error::NotFound(_),
    ++            ))
    ++    )
      }

 cli/lnk-identities/src/cli/args.rs          | 21 +++++-
 cli/lnk-identities/src/cli/eval/tracking.rs | 47 +++++++++++--
 cli/lnk-identities/src/tracking.rs          | 77 +++++++++++++++------
 3 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/cli/lnk-identities/src/cli/args.rs b/cli/lnk-identities/src/cli/args.rs
index 1a281256..d790f53e 100644
--- a/cli/lnk-identities/src/cli/args.rs
+++ b/cli/lnk-identities/src/cli/args.rs
@@ -10,7 +10,7 @@ use either::Either;

use librad::{
    crypto::PublicKey,
    git::Urn,
    git::{self, Urn},
    identities::{
        git::Revision,
        payload::{self, KeyOrUrn},
@@ -681,9 +681,19 @@ pub mod tracking {
        #[clap(long)]
        pub urn: Urn,

        /// the peer to track
        /// the peer to track, if not provided the default tracking relationship
        /// will be created
        #[clap(long)]
        pub peer: PeerId,
        pub peer: Option<PeerId>,

        /// the configuration to use when tracking the given peer, if none is
        /// provided the default configuration will be used
        #[clap(long)]
        pub config: Option<git::tracking::Config>,

        /// create the tracking relationship even if one already existed
        #[clap(long, short)]
        pub force: bool,
    }

    /// untrack a peer's gossip for a Radicle URN
@@ -696,6 +706,11 @@ pub mod tracking {
        /// the peer to untrack
        #[clap(long)]
        pub peer: PeerId,

        /// prune any references for the provided `--urn` and `--peer` from the
        /// storage
        #[clap(long)]
        pub prune: bool,
    }
}

diff --git a/cli/lnk-identities/src/cli/eval/tracking.rs b/cli/lnk-identities/src/cli/eval/tracking.rs
index 34919e12..47521154 100644
--- a/cli/lnk-identities/src/cli/eval/tracking.rs
+++ b/cli/lnk-identities/src/cli/eval/tracking.rs
@@ -3,7 +3,10 @@
// This file is part of radicle-link, distributed under the GPLv3 with Radicle
// Linking Exception. For full terms see the included LICENSE file.

use librad::profile::Profile;
use librad::{
    git::tracking::{git::refdb::PrunedRef, policy, UntrackArgs, Untracked},
    profile::Profile,
};
use lnk_clib::{keys::ssh::SshAuthSock, storage::ssh};

use crate::{cli::args::tracking::*, tracking};
@@ -11,21 +14,55 @@ use crate::{cli::args::tracking::*, tracking};
pub fn eval_track(
    profile: &Profile,
    sock: SshAuthSock,
    Track { urn, peer }: Track,
    Track {
        urn,
        peer,
        config,
        force,
    }: Track,
) -> anyhow::Result<()> {
    let (_, storage) = ssh::storage(profile, sock)?;
    let paths = profile.paths();
    tracking::track(&storage, paths, &urn, peer)?;
    let policy = if force {
        policy::Track::Any
    } else {
        policy::Track::MustNotExist
    };
    match tracking::track(&storage, paths, &urn, peer, config, policy)? {
        Ok(r) => println!("created tracking relationship `{}`", r.name),
        Err(err) => eprintln!("could not create tracking relationship: {}", err),
    }
    Ok(())
}

pub fn eval_untrack(
    profile: &Profile,
    sock: SshAuthSock,
    Untrack { urn, peer }: Untrack,
    Untrack { urn, peer, prune }: Untrack,
) -> anyhow::Result<()> {
    let (_, storage) = ssh::storage(profile, sock)?;
    let paths = profile.paths();
    tracking::untrack(&storage, paths, &urn, peer)?;
    let args = UntrackArgs {
        policy: policy::Untrack::Any,
        prune,
    };
    match tracking::untrack(&storage, paths, &urn, peer, args)? {
        Ok(Untracked { pruned, .. }) => {
            println!("untracked `{}` for `{}`", urn, peer);
            if let Some(pruned) = pruned {
                for prune in pruned.0 {
                    match prune {
                        PrunedRef::Direct { name, .. } => println!("pruned reference `{}`", name),
                        PrunedRef::Symbolic { name } => {
                            println!("pruned symbolic reference `{}`", name)
                        },
                    }
                }
            }
        },
        // NOTE: this shouldn't be possible because of the use of `Untrack::Any` above, but keeping
        // it here for posterity.
        Err(err) => eprintln!("could not remove tracking relationship: {}", err),
    }
    Ok(())
}
diff --git a/cli/lnk-identities/src/tracking.rs b/cli/lnk-identities/src/tracking.rs
index ea7c74ba..c5e56355 100644
--- a/cli/lnk-identities/src/tracking.rs
+++ b/cli/lnk-identities/src/tracking.rs
@@ -6,10 +6,11 @@
use thiserror::Error;

use librad::{
    git::{storage::Storage, tracking, Urn},
    git::{identities, storage::Storage, tracking, Urn},
    paths::Paths,
    PeerId,
};
use std_ext::result::ResultExt as _;

use crate::git::include;

@@ -26,27 +27,59 @@ pub enum Error {
    Untrack(#[from] tracking::error::Untrack),
}

// TODO(finto): allow specification of Config
// TODO(finto): perhaps we want a flag to force track?
pub fn track(storage: &Storage, paths: &Paths, urn: &Urn, peer: PeerId) -> Result<(), Error> {
    let _tracked = tracking::track(
        storage,
        urn,
        Some(peer),
        tracking::Config::default(),
        tracking::policy::Track::Any,
    )?;
    include::update(storage, paths, urn)?;
    Ok(())
/// Track the given `urn` and `peer`. This will call
/// [`include::update`] for modifying the include file for the given
/// identity. If the `urn` does not exist in the storage, then no
/// action will be take for the include file.
///
/// See [`tracking::track`] for more on the semantics of tracking.
pub fn track(
    storage: &Storage,
    paths: &Paths,
    urn: &Urn,
    peer: Option<PeerId>,
    config: Option<tracking::Config>,
    policy: tracking::policy::Track,
) -> Result<Result<tracking::Ref, tracking::PreviousError>, Error> {
    let tracked = tracking::track(storage, urn, peer, config.unwrap_or_default(), policy)?;
    include::update(storage, paths, urn)
        .map(|path| tracing::info!(?path, "updated include file"))
        .or_matches::<Error, _, _>(is_not_found, || {
            tracing::warn!(%urn, "could not update include file, the URN did not exist");
            Ok(())
        })?;
    Ok(tracked)
}

pub fn untrack(storage: &Storage, paths: &Paths, urn: &Urn, peer: PeerId) -> Result<(), Error> {
    let _untracked = tracking::untrack(
        storage,
        urn,
        peer,
        tracking::UntrackArgs::new(tracking::policy::Untrack::Any),
    )?;
    include::update(storage, paths, urn)?;
    Ok(())
/// Track the given `urn` and `peer`. This will call
/// [`include::update`] for modifying the include file for the given
/// identity. If the `urn` does not exist in the storage, then no
/// action will be take for the include file.
///
/// See [`tracking::untrack`] for more on the semantics of untracking.
pub fn untrack(
    storage: &Storage,
    paths: &Paths,
    urn: &Urn,
    peer: PeerId,
    args: tracking::UntrackArgs,
) -> Result<Result<tracking::Untracked<String>, tracking::PreviousError>, Error> {
    let untracked = tracking::untrack(storage, urn, peer, args)?;
    include::update(storage, paths, urn)
        .map(|path| tracing::info!(?path, "updated include file"))
        .or_matches::<Error, _, _>(is_not_found, || {
            tracing::warn!(%urn, "could not update include file, the URN did not exist");
            Ok(())
        })?;
    Ok(untracked)
}

fn is_not_found(err: &include::Error) -> bool {
    matches!(
        err,
        include::Error::Identities(identities::error::Error::NotFound(_))
            | include::Error::Relations(identities::relations::Error::Identities(
                identities::error::Error::NotFound(_),
            ))
    )
}
-- 
2.31.1

Re: [PATCH radicle-link v2] lnk-identities: improve track and untrack

Details
Message ID
<CAH_DpYQ0YRhS1A6Src0CGkAtUs0Kn9-7UYi9Bj-vYKMMj-_Y6Q@mail.gmail.com>
In-Reply-To
<20220629120019.66961-1-fintan.halpenny@gmail.com> (view parent)
DKIM signature
missing
Download raw message
LGTM
Reply to thread Export thread (mbox)