Rewrite module declarations that use the `#[path="…"]` annotation
to the more idomatic `pub mod …` + `pub use …` forms.
For the "self" module, that is not possible because `self` is an
identifier in Rust, and also `r#self` is not admissable as a raw
identifier, so stick to `rad_self`.
Files in submodules were patched as appropriate.
To get to a point of separating the users' identity from the node, then the `rad
self` command should not display information related to the node so prominently.
The relation between `rad` and `radicle-node` is really similar to that between
`rad` and SSH Agent. They communicate via socket. So, when this connection is
successful, it is printed, but not more.
There may be some re-learning for users here, but it is worth the improvement.
Note that the information being removed here is available via `rad node status`
(see previous commit).
If we ever want to disentangle the users' identity from the node, then
the `rad node` commands must learn to print the information related to
the node, so here we go.
This hook should make people think before committing domain names
like "radicle.xyz" and "radicle.zulipchat.com" to our Rust code.
Note that these checks can be easily skipped as follows:
SKIP=grep-radicle.xyz,grep-radicle.zulipchat.com git commit …
See also <https://pre-commit.com/#temporarily-disabling-hooks>
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.
This is dead code that imports `anyhow`. On the way to removing the
dependency on `anyhow` from `radicle-term`, it just seems to be a good
idea to remove the cruft.
`anyhow` is only used to capture a handful of errors. It also turns out that
usage of `lexopt` was wrong, since it features `parse_with`, which neatly
integrates with `FromStr`. The types, luckily, implement this trait already –
making for cleaner parsing.
For now, the execution errors are all "transparent", which shouldn't be
much of a regression, if at all.
Since we're eyeing at a Windows build, we should also have some CI
system build on Windows, so that developers that do not run it can
access build logs.
Most of the builds currently fail, but that's okay, the intention is
that they become more green over time.
If you are ever spammed by notifications about these on GitHub, disable
these at https://github.com/settings/notifications (scroll to "System"
and the row "Actions").
This way the Iterator::size_hint() function can be used by the
Extend::extend() implementation, which might safe reallocations for
the underlying buffer.
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
In many tests we assert that encoding and decoding gives the same
object. Let's call that a "roundrip", and also provide a proc macro to
generate such tests.
It turns out that failing to read configuration resulted in no output,
because the logger would only be set after configuration had been read.
Fix this, by setting the logger early, with a log level that is then
corrected as soon as configuration is available.
While at it, also clean this up to allow logging errors about failure to
set up the logger.
Include some trivial sandboxing options in the provided service files
as an example and lead users to `systemd-analyze security`.
While being a trivial change and far from a secure service it is an
improvement and may push downstream packagers and/or users to add even
a bit of sandboxing.
Signed-off-by: srestegosaurio <lcdt@disroot.org>
1. Introduce a new hook to run `cargo doc` in the "pre-commit"
phase. We have to resort to wrapping the command in a shell
script, since we cannot configure environment variables via
`.pre-commit.yaml` but need `RUSTDOCFLAGS` so that `cargo doc`
actually exists non-zero when there are warnings.
2. Move `cargo check` from the "pre-push" to the "pre-commit"
phase, as it is relatively quick, and redundant to run before
`clippy`.
3. Let `cargo check` and `cargo doc` run after `rustfmt`.
4. Let a failure of `rustfmt` fail the whole check.
5. Wire through `packageOverrides` to `rustfmt` so we use the
same toolchain as the other tools.
6. Additionally enable `cargo clippy` to consider all features.
Adds a test to ensure that canonical reference updates are received on the node
events stream.
Added a helper to `NodeHandle` for perfoming a commit to a reference and signing
the nodes references, to help with this use case.
After a fetch is performed, `set_head` is called, which essentially updates the
default branch based on the canonical reference rule for that branch.
Later, `set_canonical_refs` is called, which originally could be the empty set
of rules. This would mean that the event for the default branch would never be
emitted.
The approach taken here is to always use the default branch rule if there are no
rules set in the identity document. Unfortunately, this ends up in computing the
default branch twice, due to `set_head`.
Another approach is to use the result of `set_head` to update the set of
`UpdatedCanonicalRefs`, to prevent the need for defaulting to the default branch
rule. However, this does not prevent the double counting as soon as the rules
are added to the identity document, since the default branch rule will then be
synthesised into the rule set.
The logging version was verbose, and not required. It also could end up double
processing canonical reference updates since multiple remotes may update the
same reference.
Instead, use `filter_map` to convert all the reference names, stripping
namespaces, and collect them into a `BTreeSet` to ensure uniqueness.
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.
Since the commit is now always pushed to the Radicle storage, there is no need
to check for fast-forward. If heads diverge, it will be reported by the quorum
error.
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.
Our implementation for the control socket is based on AF_UNIX, and as
the note (which is removed) suggests, we should actually check that the
received socket really is of that domain.
This check was not implemented because it is not exposed via `std`, and
a bit cumbersome to do via `libc`.
I did not realize that `socket2` which neatly sits between `std` and
`libc` in terms of its abstractions and is cross-platform allows us to
do this, and we even already depend on it!
So, add the suggested check.
While at it, refactor the function to return early in cases. Now the
progression from a file descriptor to a socket to a listener is nicely
captured in the types and not obstructed too much by indentation.
Also log at the error level.
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.
Remove `fn deserialize` and instead provide a trait `fn decode_exact`
for testing.
Refactor errors, so that we can now cleanly distinguish between
`Error::UnexpectedEnd` which means that decoding might work given
more data, and `Error::Invalid`, which means that no amount of
additional data will decode.
While at it, cleanup the variants of `Error::Invalid`, and provide `ControlType`.
What made the issue fixed in the parent commit harder to spot
for me was the multitude of concepts and terms involved in
encoding: What's *serializing* vs. *encoding*? Why is there
this odd `Frame::to_bytes`?
So I decided to clean up a bit. I removed `fn serialize` so that the
logic stays close to the data, and instead of it provide
`fn encode_vec` in `trait Encode` so that we don't add up with
one-offs like `Frame::to_bytes` again.
Also we make use of `encode_vec` in the tests.
In 3c5668edd2 I mistakenly moved the check
the length of an encoded `wire::Message` from its `impl wire::Encode` to
the `fn serialize`, so that it would not apply only to messages (as
originally intended), but to *any* argument of `serialize`.
This is not a problem for gossip messages, but catastrophic for Git
streams, which tend to send a lot of data.
The fix is simple: Move the check back into `impl Encode for Message`.
As `std::os::unix` is obviously not available on Windows, resort to the
`winpipe` crate, which implements a very similar API for named pipes.
Apart from simple changes to imports, we need to jump through a few
hoops because the `std::io::{Read, Write}` impls are on `&mut` for
`winpipe`, thus yielding different mutability requirements on Unix vs.
Windows.
We resort to cloning, but as this fallible, swallow the added complexity
of a platform-dependent implementation to not risk panics on Unix.
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.
The signals crate does not build on non-Unix-like platforms, such as
Windows. It uses constants for signals that are not exported from
`libc` on Windows, and transitive dependencies such as `sem_safe` fail
to compile.
Resolve this, by guarding most of the crate, certainly all real
features, by `cfg(unix)`.
To require less uses of the `cfg` macro, move all affected parts into a
new module, and re-export it.
The crate now builds on Windows (although it does not do anything
interesting on that platform) and its Unix interface remains unchanged.
Changes are integrated into `radicle-node`, but only affect Windows.
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`!
Matthias Beyer
Lorenz Leutgeb
Fintan Halpenny
srestegosaurio