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.
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.
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.
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.
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 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.
The `fn io_other` disguises all sorts of errors as I/O errors, making
errors in the transport component of `radicle-fetch` harder to
understand.
Instead, expose a new error type that allows to discriminate.
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>
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.
The upgrade of the `radicle-crypto` crate was missed when upgrading
semver versions.
Unfortunately, since it is at the base of the dependency tree, all
crates that depend on it require an additional upgrade.
If one attempted to compile the test target for `radicle-node` by itself, it
would result in a compiler error – due to missing trait implementations that are
provided by `radicle-protocol/test`.
It would compile fine when `--all` is used due to cargo unifying feature sets,
so it went unnoticed until now.
This fixes the issue by adding `radicle-protocol` to the `dev-dependencies` and
enabling the `test` feature flag.
The logic for computing canonical references conflated the semantics of
annotated and lighweight tags, yielding confusing/wrong results. The
main culprit was the call to `peel_to_commit` in
`impl ReadRepository for Repository`, peeling tag objects away.
To resolve this, we separate out quroum computation per object type: one
implementation for commits, and one for tags.
Also move move canonical error types and methods into their own
module to have a cleaner file structure for the main logic.
Capture the `BTreeMap<Oid, u8>` type into a `Votes` struct, with its own API.
We were getting the objects from the repository twice – once in `Canonical::new`
and once again in `ensure_commit_or_tag` – so the latter was removed.
Use an `enum CanonicalObject` to specify which object types the canonical
process supports.
During `converges`, separate commits and tags, and ensure that we are only
looking at objects of one type. Note, that Fintan think this is actually the
*wrong* place to do it because we already filter the objects – so we should
keep track of that information higher up the callstack.
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Mostly changes of `use`s and moving components to `radicle-protocol`,
with the goal of just getting `radicle-node` to work on top of the new
`radicle-protocol` crate.
Ensure that the default branch is only ever synthesized from the `Doc`'s
`threshold` and `delegates` value. If the rule is ever written into a `RawDoc`,
then the update of the that `RawDoc` must go through the `update::veryify`
function, which checks that the rule is not present.
Refactor the `connect` method to return a `ConnectError` so that it can be
reported via logging at call sites, and returned as an error from the node
handle.
Prepare the crates for releasing to crates.io:
- radicle-remote-helper: 0.10 -> 0.11
- radicle-term: 0.12 -> 0.13
- radicle-cli: 0.13 -> 0.14
- radicle-node: 0.11 -> 0.12
- radicle-fetch: 0.11 -> 0.12
- radicle: 0.15 -> 0.16
It was also necessary to specify the `version` in the workspace Cargo.toml file,
since this is required for publishing to crates.io.
This change better separates the different errors that can occur during an
upload. It does this by:
- Separating the authorization error into its own enum and having a variant
simply for that
- The `PacketLine` error was never constructed, so the error returned from the
`upload_pack::pktline::git_request` is now wrapped in this variant.
- The error from `upload_pack::upload_pack` is now wrapped in an `UploadPack`
variant.
- The `Io` variant is removed.
- Other redundant variants are also removed.
Lorenz Leutgeb
Fintan Halpenny
Sebastian Martinez
Matthias Beyer
Erik Kundt
Lorenz Leutgeb