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.
Microsoft Windows has Universal Naming Convention (UNC) paths, see
<https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats#unc-paths>
The Rust standard library normalizes paths to this form when calling
`std::fs::canonicalize`, see
<https://doc.rust-lang.org/std/fs/fn.canonicalize.html>
However, some programs, do not parse these paths correctly, including
our ally `git`:
$ git clone \\?\C:\Users\lorenz\tmp
Cloning into 'tmp'...
ssh: Could not resolve hostname \\\\?\\c: No such host is known.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
The `dunce` crate solves the problem of having to deal with
normalization and avoiding weird UNC paths for us. Note that on
non-Windows platforms, it *behaves exactly like*
`std::fs::canonicalize`!
The stream tests did not take into account that the tips of previous COB entries
need to be inserted so that a Git history can be formed.
This was surfaced by the added regression test.
If the tests are run without a global Git config, then the tests will error with
no name or email set for the commits.
Instead, use the test fixture setup to ensure there is a name and email for
running the tests.
This patch adds the `cob::common::Title` struct, this allows instead of
using `std::str::String` or generics and traits for it, to define more
precisely what a title should be.
It trims the provided string and makes sure it contains no carriage
return or new line characters.
Where a `std::str::String` makes more sense so it's easier to mutate it,
we keep the code as is.
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Add type aliases to specialise the `Stream` type for each of `Patch`, `Issue`,
and `Identity`.
An `init` constructor is also added for each to make it easier to construct each
stream.
Introduce a new `stream` module to `radicle::cob`. The purpose of this module is
to provide an API for iterating over a COB's operations, given a range of
commits.
The `CobStream` trait provides the generic API for implementing and using, while
`Stream` provides a concrete implementation using the `git2` crate.
The approach is to use a `git2::Revwalk` for walking over each commit in the
range, then loading the `Op` for that commit – skipping any commits that are
unrelated to that COB type, in the case of associated COBs.
The `Stream` is tested using some properties of the API.
The next steps are to introduce specialised types for `Patch` and `Issue` to
ensure that it works for those.
Adds an `Op::load` helper method for loading an `Entry`, given its `Oid` and the
`store` it is stored in.
This can be useful for downstream consumers to inspect operations on COBs given
a single `Oid`, without having to load the entire object and/or graph.
It can also be useful when looking at implementing COB stream primitives.
Add a helper method `Op::manifest_of`, to allow the caller to inspect the
`Manifest` of the entry. This can be used to avoid attempting to call `Op:load`
with a manifest that would not match the expected `Action`s of the `Op`.
This patch adds an implementation of the Iterator::size_hint() trait
function so that the compiler can optimize allocations on the collecting
side of the iterator better.
From the trait function docs:
> size_hint() is primarily intended to be used for optimizations such
> as reserving space for the elements of the iterator, but must not be
> trusted to e.g., omit bounds checks in unsafe code. An incorrect
> implementation of size_hint() should not lead to memory safety
> violations.
Source: https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.size_hint
And:
> Vec may use any or none of the following strategies, depending on the
> supplied iterator:
>
> * preallocate based on Iterator::size_hint() and panic if the number
> of items is outside the provided lower/upper bounds
Source: https://doc.rust-lang.org/stable/std/iter/trait.FromIterator.html
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
When initializing the `radicle-node` process, see
`radicle-node/src/runtime.rs`, an attempt is made to read from a node
announcement file.
This file is never written, therefore all of these reads fail.
Even though this caching mechanism might make sense or might have made
sense at some point, it is dead code now, and therefore removed.
Also, the fact that the file name was defined in the `radicle` crate is
ugly.
Ensures that preferred seeds that are not synced are made part of the `to_sync`
set, and the Announcer is constructed without an error.
As well as this, ensure that the `synced` and `to_sync` sets do not overlap by
constructing the `to_sync` and removing any `synced` elements from it.
Making a note of that `Announcer::can_continue` might never be able to hit the
`Break` case. It could be defensive programming, so I'm going to leave it in for
the time being.
Adds a test case to show that the Announcer is allowed to mark an "unknown" node
as synced.
There may be a case where gossip reached a node that the current node was not
aware of. This should be allowed and should count towards the final replication
factor.
Fintan Halpenny
Lorenz Leutgeb
Defelo
Matthias Beyer
Sebastian Martinez
