Command line parsing differs on Windows vs. POSIX compliant operating
systems (just consider the differences in handling `\` for paths vs. as
a marker for escape sequences). Thus, on Windows use `winsplit` to
split command arguments instead of `shlex` for Unix-like OSes.
`winsplit` is a small crate with no other dependencies.
Domain Names have restrictions on their total length, and the length
of individual labels[^0].
Since `TypeName`s are expected to be reverse domain name strings, the
total length of the internal string should not exceed 255 and each
component should not exceed 63.
[^0]: https://www.rfc-editor.org/rfc/rfc1035#section-2.3.4
The test name being used is `valid_typenames`, however it tests for
invalid `TypeName`s as well.
The invalid `TypeName` tests are now separated into another test
called `invalid_typenames`.
Enable the sha1 feature by default for `radicle-oid`.
Without this, the crate cannot be tested in isolation. It results in a
compile error, requiring the feature to be enabled.
Wire up the new `FetcherService`. This reduces the `Service` fetch
methods down to:
- `fetch`
- `fetched`
- `dequeue_fetches`
- `fetch_refs_at`
This simplifies the code by off-loading the logic to the `fetcher`
family of types, rather than handling the intricacies in the `Service`
itself.
The `Service` now just performs the necessary I/O based on the
returned events from the `fetcher`.
This also removes the fetching state from the node sessions. This
follows the single responsibility, and sessions now only care about
their connectivity.
Breaking changes:
- The `Connected` state of a peer no longer contains fetching
information, which was being returned as part of the JSON payload
result.
- The `rad debug` information for ongoing fetches contained the number
listeners awaiting for results, this was removed.
The `FetcherService` wraps the `FetcherState` alongside keeping track
of subscribers for a given fetch.
Typically, this is a channel for which the fetch result should be sent
to.
These subscribers are coalesced by maintaing a map from `(rid, node)`
pairs to the subscriber.
This continues the sans-IO approach where the type of subscribers are
not specified, but rather are kept as a type parameter – to be
specified by the caller.
This patch introduces the `FetcherState` which encapsulates the logic
and state for keeping track of fetches.
It uses a sans-IO approach, where the state of fetches transition
based on the current state and provided input.
Callers can then decide to perform I/O based on the returned event.
The `From<String> for Message` implementation has an interesting
interaction between `clap` and how `Message` can be used from the
command line.
Note that `FromStr` is not implemented for `Message` – which usually
what is to be expected for use with `clap`, but in fact, `clap` also
allows `From<String>`.
This would make `Message` be a required option when using `rad issue comment`.
It is not possible to use `default_value_t` because that required an
implementation of `Display`, which in this case we do not want to
implement. Trying to use `default_value = "Message::Edit"` also would
not work – it uses that as the text.
So, the solution is to mark it as optional, and default to
`Message::Edit` when it is not specified.
Be more defensive by preventing a node, that is not configured for
Tor, to not connect to `.onion` addresses via the `Service::connect`
method.
This means that an `Outbox::connect` should not be produced, and the
address table should not insert an entry.
When maintaining persistent connections, the config was consulted for
a peer address. Multiple addresses for a single `NodeId` can be
listed. This can result in a different address being used for
reconnecting if the `HashSet` returns in a different order.
The original address that was used for connection is in the `Session`,
so this should be used instead.
This command is confusing in name and function. Rather than creating
a hard fork of a repository, with a new identity, it pushes the
default branch to the local user's namespace.
Deprecate it and add some help text.
When a non-delegate attempts to update the identity document, their
action will be rejected.
Provide a better error, along with a hint, to the non-delegate.
Changes the `Error::WithHint::hint` field to use a `String` type instead of
`&'static str`. This allows the caller to provide extra hint
information.
To make it easier to use an `Error::with_hint` constructor is added.
The previous error `UnexpectedState` was opaque, and ended up in a
confusing message when a user would try to edit the identity document
while not being a deleagte themselves.
Instead, provide a more specific error that mentions the `Did`
performing the action, and the action itself.
Note that the `UnexpectedState` variant was matched on during
evaluation, meaning that operations would still appear in the timeline
of the COB. With the change to the other variant, the timeline does
not record the operation anymore.
Two code fences that do not contain are not marked as such, leading to
false negatives when testing `radicle-core`. To remedy, mark these
fences as text.
Previously, the `references_of` implementation restricted the set of
references to certain categories: heads, tags, notes, rad, and cobs.
This is too restrictive to allow peers to share any references they
want. The only exceptions is references `refs/tmp/heads` that are
created for creating patches.
Evaluate the `error!` logging in `radicle-node`.
The majority of cases, the `error!` is downgraded to `warn!`. These
cases are generally useful to know if there's an issue, but the
operator cannon necessarily do anything about it.
In a few cases, `debug!` was chosen. These are generally when an error
means that a result is not part of an accumulation, or the system will
eventually correct itself during another event or restart.
Evaluate the `warn!` logging in `radicle-node`.
If the event is a result of something being suspicious in the running
of the protocol, then it remains a `warn!`.
However, if the system ends up ignoring the event and can continue
without issues, the log level is reduced to `debug!`.
Evaluate the `error!` logging in `radicle-protocol`.
The majority of cases, the `error!` is downgraded to `warn!`. These
cases are generally useful to know if there's an issue, but the
operator cannon necessarily do anything about it.
In a few cases, `debug!` was chosen. These are generally when an error
means that a result is not part of an accumulation, or the system will
eventually correct itself during another event or restart.
If the previous message started with "Error", then the wording is
changed to say "Failed to".
A call to `Database::prune` with no limit could result in a
domain-defined overflow error.
If the limit is not provided, then it uses `i64::MAX`. It is
reasonable to expect that if the `usize` provided is more than
`i64::MAX` then the `i64::MAX` value is acceptable to use for pruning.
Evaluate the `warn!` logging in `radicle-protocol`.
If the event is a result of something being suspicious in the running
of the protocol, then it remains a `warn!`.
However, if the system ends up ignoring the event and can continue
without issues, the log level is reduced to `debug!`.
Evaluate the use of `warn!` and `error!` logging in `radicle-fetch`.
The majority are downgraded to `debug!`, the rest either being
downgraded to `info!` or removed altogether.
Instead of changing `impl From<SystemTime> for LocalTime` to `TryFrom`,
the implementation was removed, as there are not many callsites, and
they are well served by `LocalTime::now`.
radicle-localtime: drop TryFrom SystemTime
radicle-node: switch SystemTime usage with LocalTime
Introduce the `radicle-core` crate for housing data types that are at
the core of the Radicle protocol. The initial data type being added is
`RepoId`. To make the crate as lightweight as possible many of
dependencies are optional and gated by feature flags.
The crate is marked as `no_std` even though it unconditionally depends
on `radicle-crypto`, which is *not* `no_std`. This is to invite
refactoring of `radicle-crypto` to `no_std` at a later time.
The `localtime` crate was defined by cloudhead, and is a minimal
repository with a single `lib.rs`.
Instead of using it as an external dependency, copy the code directly
into a new workspace crate `radicle-localtime`.
The default `serde` implementation goes through the `LocalTime`'s
seconds values rather than milliseconds, since this is the more common
format. This allows the removal of the `serde_ext` functions.
The one place milliseconds was used was for the
`radicle::cob::common::Timestamp` type, where the `Serialize` and
`Deserialize` implementations are manually written.
It also adds a `schemars` feature to remove `schemars_ext` functions
in `radicle` as well.
The message that is returned by `gix-transport` for I/O errors can be
unhelpful, since it does not provide the reason for what happened.
Instead, surface the error so that it provides more detail for logging.
Instead of logging all IO errors as an error, match on the kind of the
error, logging an info message, otherwise an error.
These errors are informational, and cannot necessarily be resolved by
the node operator – since it is a matter of not being able to connect
to another end.
The following block did not need mutable access to the `outbound`
value.
To avoid confusion around mutable access, the call was downgraded to
`get` so that the reader can safely know there is no mutation
happening.
For some COBs, parent commits can be specified for actions, for
example, patches will bundle their base and head commits as parents.
When the COB stream performs the revwalk, these commits will be
included, and will not have the manifest file – resulting in an error.
Instead, this error can be matched against, and the commit is skipped
instead. The other errors should still be resurfaced, since a commit
with a manifest should be expected to load the operation correctly,
and the commit should exist in the repository when attempting to load
it.
One might argue that a valid operation with a missing manifest could
occur, but that would mean that `radicle-cob` has performed an invalid
write. This is undetectable by the stream API, and is ambiguous with
the case of non-Op commits.
The `struct Epoch` introduced in e404f1038f
inadvertently made `radicle-node` suffer from
https://github.com/rust-lang/rust/issues/87906
That is, updating of the "current time" was skewed *way more*
than expected by a slowly ticking monotonic clock. Such slow
ticking can be caused by suspending the system.
Implement `IntoIterator` for the `BoundedVec`.
This allows the use of `into_iter` for returning the owned data,
rather than references via the `Deref` implementation.
All IPv6 addresses would be considered globally routeable, even though
the Rust standard library offers convenience functions to check for
loopback, link-local addresses etc.
Improve checks for IPv6 routeability to catch the most obvious cases of
local or unspecified addresses.
Refactor the check for IPv4 routeability to be more readable and refer
to RFCs, IANA lists, and Rust stabilization tracking issues as
appropriate.
`Address:is_local` would return `false` for all DNS names. This is
incorrect, with one counterexample being the name "localhost", which
generally resolves to a local (usually loopback) address.
The function is changed to catch "localhost", but also the top level
domain ".localhost", which is reserved in RFC 2606, Section 2.
`Address::is_routable` would return `true` for all DNS names. While it
is much harder to decide global routeability based on a domain name, as
these usually have to be resolved to an address before being able to
judge routability, there is one particular class of names, namely local
ones (see above), which are not globally routable.
Due to connection retries in `radicle-node` not choosing new addresses,
mistakenly using an IPv6 address when IPv6 is not supported would result
in failure to bootstrap.
Work around by removing IP addresses again. This means that DNS or Tor
will be required to bootrstrap.
There is an unfortunate lack of single-responsibility with handling the
addresses to connect to.
This is a band-aid fix until the point in time where this can better improved.
Introduce the use of `clap_completion` for generating static completion.
The shells that are supported are the ones that are included in `clap_complete`:
- `bash`,
- `elvish`,
- `zsh`,
- `fish`, and
- `powershell`.
Co-authored-by: Michael Uti <utimichael9@gmail.com>
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
Instead of failing with an `InvalidId` error in
`radicle::storage::ReadStorage::repositories` for repos that don't have
a valid repository ID, skip them and log a warning.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
The previous message is included in the error:
! [remote rejected] master -> refs/patches (patch commits are already included in the base branch)
To make it clearer to the user, also print a warning to tell them that the
commit was in the base branch – including the commit SHA:
warn: attempted to create a patch using the commit f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354, but this commit is already included in the base branch
If a seed for syncing does not have an address, the local node will not be able
to connect to it.
These can be filtered out so that no connection attempts are made for these nodes.
Previously, the remote helper would not support the `--force-with-lease` option.
This change introduces this support and ensures that it works as intended.
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Change `repositories_by_id` to return `impl Iterator<Item = Result<RepositoryInfo,
RepositoryError>>`
instead of `Result<Vec<RepositoryInfo>, RepositoryError>`.
This allows callers to handle failures on a per-repository basis rather
than having the
entire operation fail if a single repository lookup fails.
Previously, the method would stop processing and return an error as soon
as any repository failed to load. Now it processes all repositories and
returns individual results, making the API more resilient and giving
callers
more control over error handling.
Refactors the external command logic into a struct.
This allows for methods to maitain the logic of an external command.
It also allows for a cleaner output for the executable name – avoiding the debug
output of OsString using `""`.
The other variants of `terminal::args::Error` were no longer used.
The only variant required is `Error::WithHint`.
The enum is also made private, since it is only used in this crate.
These helper functions were for parsing arguments passed via lexopt.
Since `clap` is now the parser for these types, these functions are no longer
needed.
The `help` module is no longer needed, since `clap` handles that.
This meant that the re-exports of each command's `ABOUT` could be removed and
the `const`s can now be private.
Now that all commands can be parsed by clap, remove any custom parsing and
directly use `CliArgs::parse`.
The `Diff` command was removed, since only one `external_command` can be
declared for a subcommand. The handling of `rad diff` is now done via the
`External` variant. If the subcommand name is `diff` then this is executed,
otherwise delegate to the possible `rad-{exe}` command.
Also of note, the `rad inspect` command has an alias `rad .`. This changes the
behaviour of `rad . --help`. Before, it would print the help of the `rad`
command. Now, it prints the help of `rad inspect`.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
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>
Moving the sub-comment actions to be beside the top level comment action. This
is mostly to provide a better diff for the next commit – that refactors due to
`clap` changes.
The test 'test_concurrent_fetches' is broken. It asserts over empty
sets nullifying its utility. Correct the test by changing it so that
sets to be asserted against are cloned.
We decided to use the value names to indicate which type the value
is parsed into (instead of just saying that the value is a `STRING`).
This makes the value names consistent with other commands again.
Lorenz Leutgeb
Fintan Halpenny
Defelo
Adrian Duke
Sebastian Martinez
Yaroslav Halchenko
Matthias Beyer
Johannes Kühlewindt
Richard Levitte
Erik Kundt
Lorenz Leutgeb