Due to the idiosyncracies of this command there are a few things to note about
this migration.
The `rad sync` command acts as a default subcommand in itself, in that it has
options that do not apply to its subcommand, `rad sync status`. This means that
the options will print with `rad sync --help`, but they will not print with `rad
sync status`.
The `--inventory` flag is not compatible with any of the other flags and
options, thus they are all marked with `conflicts_with`. This cannot be done for
a positional argument, so `clap` will "helpfully" print out the usage, for
example, as:
error: the argument '--inventory' cannot be used with '--fetch'
Usage: rad sync --inventory [RID]
For more information, try '--help'.
Which is incorrect, since `RID` is ignored. This is explained in the `--help`
documentation for `--inventory`.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
Make `git2` an *optional* dependency of `radicle-cob`, and refactor
`radicle` to depend on crates that in turn do not depend on `git2`
*non-optionally*
The main offending dependency of `radicle-cob` is `radicle-git-ext`
from the `radicle-git` workspace in repository
(rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt) which *non-optionally* depends on
`git2`.
So, to achieve removal of this dependency:
1. The crate is refactored to depend on the new crates
`radicle-git-ref-format` `radicle-git-metadata`, and
`radicle-oid` introduced in the previous commits, instead of
`radicle-git-ext`.
2. Some code from the `radicle-git-ext` crate in the `radicle-git`
workspace in repository (rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt) is
copied. See `crates/radicle-cob/src/backend/git/commit.rs`.
This cascades to `radicle` and its dependents.
Firstly, the there is an
`impl Deref<Target=git2::Oid> for radicle_git_ext::Oid`. This made
it very convenient to just deref to obtain the wrapped `git2::Oid`,
so there are many expressions of the shape `*oid` in `radicle` and
its dependents. However `radicle-oid` does not provide
`impl Deref<Target=git2::Oid> for radicle_oid::Oid`, as notably,
`Target` is an associated type, and not a type parameter, so an
implementation of `Deref` would tie the new `Oid` too tightly to a
particular implementation.
Instead, work with `impl From<radicle_oid::Oid> for git2::Oid`
(which can be enabled using the feature flag `radicle-oid/git2`).
This explains the changes from `*oid` to `oid.into()` at every
transition to "`git2` land".
Secondly, `radicle` and its dependents are refactored to also depend
on `radicle-git-ref-format` and (much less prominently)
`radicle-git-metadata` instead of `radicle-git-ext`
This is to avoid pulling in `git2` via these dependencies.
Thirdly, as the re-exports in `crates/radicle/src/git.rs` change,
they are at the same time also cleaned up. Notably, the types from
`radicle-git-ref-format` are re-exported under `fmt` only, not "twice".
While overall this obviously is very much a breaking change, these
changes should mostly amount to changing from `Deref` to `Into`, i.e.,
`*oid` → `oid.into()` and rewriting imports for `radicle::fmt`.
This is indicated by the mostly mechanical nature of the changes to
`crates/radicle-{cli,node,remote-helper}`.
For further details on the crate see its documentation in
`crates/radicle-oid/src/lib.rs`.
Changes to `crates/radicle{,-crypto}/Cargo.toml` are to preserve the
default features of their dependencies `serde` and `schemars`,
which are disabled to support `radicle-oid` being `no_std`.
In commit 2149770a4b the dependency
`tempfile` was made optional, and added to be enabled by the feature
flag `test`. However, the *tests of this crate itself* need the
dependency *unconditionally* as they want to create temporary
directories.
This was not noticed earlier, since the error does not surface when
the whole workspace is tested (`cargo test --workspace`), but only
when the crate itself is tested (`cargo test --package=radicle`),
because when testing the workspace, dependents of `radicle` will
enable the `test` feature flag.
`impl Display for RepoId` will produce a `rid:` prefix, which
contains a colon, and this trips up various filesystems. Use
`RepoId::canonicial` instead.
The following tests generated an unstable object ID, thus linearizing
history in a different order, causing failures. Fix this by using the
`stable-commits` feature of `radicle-cob`.
thread 'cob::identity::test::test_identity_redact_revision'
panicked at crates\radicle\src\cob\identity.rs:1338:9:
assertion `left == right` failed
left: [Oid(b4307ded046befba374bf8cd9fd787592ceb615c),
Oid(a04165e3d3717c5a1413ec78e852bd8e1bb049d4),
Oid(820d3faf5b507888173b26ccd1a2a81666bd2573),
Oid(30ca6f078a401bf542049594fbb1c8d2371c9819),
Oid(9e9c46971014b123a31cd1195078f95c2e319419)]
right: [Oid(b4307ded046befba374bf8cd9fd787592ceb615c),
Oid(a04165e3d3717c5a1413ec78e852bd8e1bb049d4),
Oid(820d3faf5b507888173b26ccd1a2a81666bd2573),
Oid(9e9c46971014b123a31cd1195078f95c2e319419),
Oid(30ca6f078a401bf542049594fbb1c8d2371c9819)]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'cob::identity::test::test_identity_reject_concurrent'
panicked at crates\radicle\src\cob\identity.rs:1494:9:
assertion `left == right` failed
left: [Oid(b4307ded046befba374bf8cd9fd787592ceb615c),
Oid(bce4233f6a09d6e9c021ba436e3c6923a1a78f31),
Oid(7cacb23be7079c16c8ad1b2458ae5bac95a1295b),
Oid(12b00e2e871b699c84c20427a64e029934c15ec4),
Oid(07431c8d80c11492ef248e07126370e874de3435),
Oid(98b42450159a359c2f482652c66d79b05d0af624)]
right: [Oid(b4307ded046befba374bf8cd9fd787592ceb615c),
Oid(bce4233f6a09d6e9c021ba436e3c6923a1a78f31),
Oid(7cacb23be7079c16c8ad1b2458ae5bac95a1295b),
Oid(98b42450159a359c2f482652c66d79b05d0af624),
Oid(12b00e2e871b699c84c20427a64e029934c15ec4),
Oid(07431c8d80c11492ef248e07126370e874de3435)]
Fixes test `profile::test::canonicalize_home`.
Since `Home::new` canonicalizes its argument (which is what the test
wants to ensure), also canonicalize the expected value, which is
especially important on Windows as the canonicalization is more
complex than on Linux.
For example:
thread 'profile::test::canonicalize_home' panicked at crates\radicle\src\profile.rs:802:9:
assertion `left == right` failed
left: "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\.tmpMnNkr6\\Home\\Radicle"
right: "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmpMnNkr6\\Home\\Radicle"
A refactoring internal to the `radicle` crate, with the goal of
making dependency on `git2` clearer and more controlled.
`radicle::raw` is changed from a complete re-export of `git2`
to a module that selectively re-exports (many) members of `git2`.
required to build the workspace (potentially breaking dependents
outside the workspace, but given just how many types are
re-exported, this seems unlikely).
In the future, no more `use git2::…` statements should be added
outside of `crates/radicle/src/git/raw.rs`. This is in an effort to
decrease dependence on `git2` in the future.
The dependencies `netservices`, `io-reactor` and `popol` served us well,
however they do not support Windows and are not actively maintained.
This change removes the aforementioned dependencies (and `libc` along
with them). It reuses the integration with `cyphernet` from
`netservices` for Noise and SOCKS.
The new module `reactor` is a rewrite of `io-reactor` on top of Mio.
Note that no tests were changed.
The `git-ref-format-core` crate is enough to implement everything
required by `radicle-crypto`, and has a much smaller dependency
footprint than `radicle-git-ext`. Notably, it does not depend on
`git2` and also does not depend on procedural macros.
The "seeds" command does only take a repository ID, and
carries the implicit assumption that the namespace with the same public
key as the Node ID of the receiving `radicle-node` process should be
used as reference for determining whether other nodes are in sync,
but no other namespace.
In the spirit of separating user and node identity, relax this, so that
the command also carries the public keys relative to which the sync
status should be computed. This allows to inspect sync status for
arbitrary namespaces via command.
For now, this feature is not exposed to the CLI, but `rad sync status`
always passes the public key of the active profile.
The "announce references" command does only take a repository ID, and
carries the implicit assumption that the namespace with the same public
key as the Node ID of the receiving `radicle-node` process should be
announced, but no other namespace.
In the spirit of separating user and node identity, relax this, so that
the command also carries the public keys for which the announcement
should be made. This allows to announce arbitrary namespaces out of
storage via command.
For now, this feature is not exposed to the CLI, but rather:
- `rad sync` always announce for the public key of the active profile.
- `git-remote-rad` ditto, as it calls the same codepath.
Some tests actually do not use qualified patterns. The removed lines
*should* not compile because the `qualified_pattern!` proc macro
*should* validate the string literals at compile time, but it contains
a bug and actually does not assert well-formedness of qualified
refspec patterns. Rectify.
On Windows, all attempts to clone repositories failed with
Fetch failed for rad:… from z6Mk…: Access is denied. (os error 5)
The reason is that, other than Unix-like systems, it forbids that
directories that are in use are moved.
To improve the situation, take back control over what is moved and
removed exactly by implementing `cleanup` instead of relying
on `impl Drop for tempfile::TempDir`.
A new type `TempRepository` is introduced to capture a repository that can be
used temporarily, and either cleaned up on failure, or moved on success.
This `TempRepository` can be constructed using `Storage::temporary_repository` –
renamed from `Storage::lock_repository` since it is only creating a temporary
resource and not locking it.
In the future (once Rust 1.89 is a little less cutting edge and more
widely available), we may opt for actual locking via
`std::fs::File::lock`.
Allow the fetch interface to accept anything that implements
`AsRef<Repository>`. This allows flexibility in the types that the `Handle` can
accept.
This change is motivated by wanting to introduce a type that is a temporary
repository that wraps a `Repository`.
With this change, the location of the secret SSH key can be configured
through `${RAD_HOME}/config.json` so that the node key does not have to
be placed under `${RAD_HOME}/keys` anymore.
Further, there is now an option to override `config.json` directly when
executing `radicle-node` via the command line argument
`--secret`.
The primary motivation is more flexible deployments, for example
leveraging external secret management solutions, like
<https://systemd.io/CREDENTIALS/>.
The secret key is fingerprinted by taking the fingerprint of the public
key corresponding to the secret key. This allows to detect when the
secret key changes.
When using Jujutsu and a non-colocated Git repository, the detection
using `git2` directly fails (just as `git` in a shell would fail)
because there is no `.git` directory found by traversing up the
filesystem hierarchy.
Add an invocation of `jj git root` as a fallback.
When run with a secret and public key that do not cryptographically
match, `fn radicle_cli::terminal::io::signer` would prompt the user
for a password to unlock the secret key, and then carry on with a
mismatching key pair.
Fix this by verifying that the keys match in `MemorySigner::load`
and only swallow errors initializing the signer in cases where
prompting for a password makes sense. It does not make sense in the
case of mismatched keys.
Previously it was possible to create an issue with an empty description
(i.e. an empty "root" comment), and also to change it later to a
non-empty string, however it was not possible to clear it again.
`radicle::run` interprets the `Output` returned by the `git`
process. However, depending on the application we have different
requirements for interpreting the output, e.g., how to handle errors.
Make `radicle::run` not interpret `Output`, but move this
logic to the respective binary crates (`radicle-remote-helper` and
`radicle-cli`).
The `verbosity` option is ignored, which makes it more difficult to
obtain debugging information if the "internal" push, i.e., the
`git push` invocation by `git-remote-rad` fails.
Provide two types for verbosity, and wire them up.
Ideally, we would like to use a name that is compliant with RFC 2606,
such as "radicle.example.com", but this would change *lots* of hashes,
which is not worth the hassle.
Instead, make changing this in the future as easy as changing one
constant.
This makes the default Radicle Explorer URL configurable at compile-time
via the environment variable `RADICLE_EXPLORER`.
In order to stay backwards compatible, we still default to
"app.radicle.xyz"
As a library crate, it is bad to return such generic errors.
To get there, errors from `inquire` are downcasted.
This allowed to clean up the public interface of `radicle-term`, so that
it does also not leak the `inquire::InquireError` in its public API.
Whenever the node fetches new updates, it checks if canonical references can be
updated. The node has learned how to return these results and emit them as node
events. This is a breaking change since it adds a new variant the `Event` type,
which is not forwards-compatible.
This change was inspired by the a story as old as time:
𝕃𝕖𝕥 𝕦𝕤 𝕞𝕚𝕩 𝕠𝕦𝕣 𝕓𝕦𝕤𝕚𝕟𝕖𝕤𝕤 𝕝𝕠𝕘𝕚𝕔 𝕨𝕚𝕥𝕙 𝕠𝕦𝕣 𝕀𝕆!
It was motivated by the fact that the canonical quorum logic was spread across
two modules and also two different repositories (in the API sense). The
`radicle-remote-helper` contained special logic for computing the quorum, and
also relied on the logic with `radicle` itself. It would also mix using the
Radicle storage repository and the working copy repository – resulting in issues
where objects could exist in one and not the other.
The change begins by separating away the IO away from any of the business logic
in the `git::canonical` module. To follow along, there are two important submodules:
1. `voting` captures the different type of voting processes for commits and tags
a. Tags are simple, where one `Oid` means one vote
b. Commits are slightly more complicated. They begin with one `Oid` means one
vote, but then it is expected that merge bases are calculated for pairs of
commits. These merge bases are used to increase a vote for an `Oid` if it
is the merge base of another commit.
2. `quorum` builds on top of `voting` and uses the voting processes as well as
the `threshold` to find the quorum for the tag or commit reference.
a. For tags, the first past the threshold wins, but if multiple pass then it
is an error.
b. For commits, the merge base process should be used to increase the votes,
until the caller is ready to find the quorum. At this point, the commits
that pass the `threshold` are then compared to find the commit that is the
child-most commit of all other candidates. If they diverge, then an error
is returned.
There is also a `convergence` module that captures the logic that is required
for the `radicle-remote-helper`. It essentially checks if a candidate object
matches the expected objects, tags or commits, and performs the necessary
convergence logic – checking that the candidate commit is converging with
at least of the other `Did`s.
These two quorum processes, and the convergence process, essentially act as
state machines and can be driven by the use of a Git repository for finding the
merge bases. This is where the `effects` module comes in. The `effects` capture
the necessary traits that are required to drive the state machines of the quorum
processes. It is expected that a Git repository implements these, and in fact, a
`git2::Repository` implementation is provided. The traits are useful, since it
means that a `git2::Repository` can be swapped out and the logic would stay the
same.
This all culminates into the new and improved `Canonical` and
`CanonicalWithConvergence`. Both of which have methods `find_quorum` for
performing the quorum process using a *single* provided repository.
This resulted in the semantic change of requiring that the
`radicle-remote-helper` pushes the candidate commit, irregardless of whether it
will be a fast-forward. For now, this is something that will be accepted while
the UX can be improved in the future by providing detailed warnings of the
divergence and ways to fix it. The benefit is that the tooling will never stop
someone from diverging if that is in fact what they want to do.
I was greeted by `rad patch redact` with
called `Result::unwrap()` on an `Err` value:
Error { code: None, message: Some("failed to convert") }
stack backtrace:
[…]
3: core::result::Result<T,E>::unwrap
at …/rust-1.88.0/lib/rustlib/src/rust/library/core/src/result.rs:1137:23
4: sqlite::cursor::Row::read
at …/index.crates.io-1949cf8c6b5b557f/sqlite-0.32.0/src/cursor.rs:136:9
5: radicle::cob::patch::cache::query::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:624:65
6: <… as radicle::cob::patch::cache::Patches>::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:553:9
7: radicle_cli::commands::rad_patch::redact::run
at ./crates/radicle-cli/src/commands/patch/redact.rs:23:9
8: radicle_cli::commands::rad_patch::run
at ./crates/radicle-cli/src/commands/patch.rs:1026:13
[…]
It turns out that `sqlite::cursor::Row::read` is the panicky version of
`sqlite::cursor::Row::try_read` (which returns a `Result`).
While it is somewhat rare that SQLite reads fail, it is not unheard of.
In `radicle-cli` it might not be critical, but also `radicle-protocol`
and `radicle-fetch` are affected, and they could potentially panic a
`radicle-node` process.
Use `try_read` instead, and propagate down the error handling.
The `winpipe` crate which is our current best bet for support on Windows
requires that named pipes are at the magical location `\\.pipe\`.
Provide a default on Windows that matches this, and use our freshly
assigned IANA service name for it.
This is a breaking change, as we remove a `pub const`, it should be
reasonably easy to fix by dependents, and probably it was a mistake to
make the default `pub const` as dependents should call `Home::socket`
anyway.
Yaroslav Halchenko
Fintan Halpenny
Johannes Kühlewindt
Erik Kundt
Sebastian Martinez
Lorenz Leutgeb
Lorenz Leutgeb
Defelo
Matthias Beyer
