This commit was made by reverting the previous commit, then
re-running the script I used to generate it. In theory there should
be no semantic changes: only changes due to improved formatting from
cargo edit.
I followed the following procedure to make these changes:
* I used maint/changed_crates to find out which crates had changed
since 0.3.0.
* I used grep and maint/list_crates to sort those crates in
topological (dependency) order.
* I looked through semver_status to find which crates were listed as
having semver-relevant changes (new APIs and breaking changes).
* I scanned through the git logs of the crates with no
semver-relevant changes listed to confirm that, indeed, they had
no changes. For those crates, I incremented their patch-level
version _without_ changing the version that other crates depend on.
* I scanned through the git logs of the crates with no
semver-relevant changes listed to confirm that, indeed, they had
no obvious breaking changes.
* I treated all crates that depend on `arti` and/or `arti-client` as
having breaking changes.
* I identified crates that depend on crates that have changed, even
if they have not changed themselves, and identified them as having
a non-breaking change.
* For all of the crates, I used `cargo set-version -p $CRATE --bump
$STATUS` (where `STATUS` is `patch` or `minor`) to update the
versions, and the depended-upon versions.
This gets rid of `#[serde(flatten)]` which prevents serde_ignored (and
other kinds of introspection) from working properly.
The price is now that the toplevel has to deal with two configuration
objects.
The Resolvable trait is overkill right now, but is going to do More
Things in a moment. In particular, we need the impl on tuples, so
that the whole config can be processed in one go.
This change requires a little refactoring of TorClientBuilder: now,
instead of enabling or disabling mistrust, it enables or disables
the decision to _override_ the mistrust in the config.
this change adds unit tests for the public methods of StreamPrefs. although
these are mostly "setter" style functions, the tests confirm the basic
expectations and operation.
I have Plans for this macro. In particular:
* I have a wip branch which tests that the Builder can be
deserialised from an empty config (ie, that config reading
of a config with a blank section for this item works).
* I think we should autogenerate $Config::builder(),
and promote that, rather than $ConfigBuilder::default().
This macro could do that.
This macro is kind of derive-y. Also it has a test in it, and failing
to call it could allow bugs to exist, as well as missing bits of API.
Putting it next to the structs makes it easy to see that it's actually
been called.
Discovered by a test case in my local tree. The test case was
macro-generated by an extension of impl_standard_builder (which
macro istself currently awaiting review, arti!499)
Have also sent an MR to update the upstream docs
https://github.com/jean-airoldie/humantime-serde/pull/8
This new section describes how much variance we accept when it comes
to expired and not-yet-valid directory documents. (Currently, the
only ones where this matters for are consensus documents and
authority certificates.) A document that is invalid by no more than
these tolerances is not _live_, but it can still be used.
These tolerances serve two purposes:
* First, they allow clients to run with a little more clock skew
than they would tolerate otherwise.
* Second, they allow clients to survive the situation where the
authorities are unable to reach a consensus for a day or two.
Compare with Tor's REASONABLY_LIVE_TIME and NETWORKSTATUS_ALLOW_SKEW
constants; also compare with proposal 212.
Closes#412.
I'm doing this per discussion, so that we can have it be part of the
TorConfig later on, and not break stuff as we change the Mistrust
API to have a builder.
This change, unfortunately, results in a little more internal
complexity and duplicated code in arti and arti-client. I've marked
those points with TODOs.
This is derived from the environment, not the configuration file: We
might not want to trust the configuration file until we've decided
whether we like its permissions.
For reference, the git source for this crate (and the others in its
workspace) currently lives in my personal github account (ijackson).
If this fork turns out to be long-lived and gains features and/or
users, it would be good to move it to a gitlab somewhere.
I have granted Nick crate ownership on the crates.io system.
* Builders additionally derive: Debug, Serialize, Deserialize.
* Validated structs no longer derive: Serialize, Deserialize
and all related attributes deleted.
* As a consequence, all the `#[serde(deny_unknown_fields)]`
are gone. That means that right now unknown fields are totally
ignored. This is good for compatibility but poor for useability.
Doing something better here is arti#417, in progress.
* As a consequence, delete tor_dirmgr::retry::default_parallelism.
(The default value was already duplicated into a builder attr.)
And drop the ad-hoc orport() method. This brings FallbackDir's
orports field in line with our list builder API.
The general semver note in "configuation" seems to cover most of this.
The new API is (roughly) as discussed in
https://gitlab.torproject.org/tpo/core/arti/-/issues/451
This is quite a large commit and it is not convenient to split it up.
It contains the following changes:
* Redo the list builder and accessor macros implemnetation,
including docs and tests.
* Change uses of define_list_config_builder. In each case:
- Move the docs about the default value to the containing field.
- Remove the other docs (which were just recapitulations, and
are now not needed since the ListBuilder is no longer public).
- Rewmove or replace `pub` in the define_list_builder_helper call,
so that the builder is no longer public.
- Change the main macro call site to use define_list_builder_helper.
- Add a call to define_list_builder_accessors.
* Make the module `list_builder` pub so that we have somewhere to
put the overview documentation.
* Consequential changes:
- Change `outer.inner().replace(X)` to `outer.set_inner(X)`
- Consequential changes to imports (`use` statements).
Document that this can contain either a string for expansion, or a
literal PathBuf not for expansion.
Rename the `from_path` method to `new_literal`: a very important
difference is whether it gets expanded - less important than the Rust
type. Also, now it takes `Into<PathBuf>`, which avoids a needless
clone.
(We don't change the API in `arti-client` because
`&tempfile::Tempdir()` doesn't implement `Into<PathBuf>`, so
`arti-client` has to have some new `as_ref` calls.)
Provide accessors `as_unexpanded_str` and `as_literal_path`. The
deserialisation already makes this part of the stable API,l so not
pvoding accessors seems just obstructive. They are useful for tests,
too.
Add tests for the new entrypoints, and for deserialisation of both
variants from TOML (via config, or directly) and JSON.
The `[patch]` approach causes the tree not to build when used as a
dependency, unless the `[patch]` is replicated into the depending
project.
Instead, replace our `derive_builer =` dependencies with a reference
to a specific git commit:
perl -i~ -pe 'next unless m/^derive_builder/; s#"(0\.11\.2)"#{ version = "$1", git = "https://github.com/ijackson/rust-derive-builder", rev = "ba0c1a5311bd9f93ddf5f5b8ec2a5f6f03b22fbe" }#' crates/*/Cargo.toml
Note that the commitid has changed. This is because derive_builder is
in fact a workspace of 4 crates. 3 of them are of interest to arti
itself (the 4th exists only for testing). So the same "add git
revision" treatment had to be done to the `derive_builder` and
`derive_builder_macro` crates. Each dependency edge involves a new
commit in the derive_builder workspace, since we can't create a git
commit containing its own commitid. (We want to use commits, rather
than a branch, so that what we are depending on is actually properly
defined, and not subject to the whims of my personal github
namespace.)
There are no actual code changes in derive_builder.
Use sub_builder. We must do something special for defaults.
This involves moving the actual default values for retry_bootstrap and
retry_microdescs into config.rs, since they need to access the fields
of the un-built version of the structure. (An alternative would be to
generate "weak setters" which do not override previous settings, but
derive_builder does not offer to generate them and that seems
overkill.)
Instead, everyone should use DownloadScheduleBuilder.
The new() method would in any case be useless in a moment, since we're
going to embed DownloadScheduleBuilder in the NetworkConfig, not
DownloadSchedule.
The call sites in the tests are all about to change again.
This is an automated change made with a perl one-liner and verified
with grep -L and grep -l.
Some warnings are introduced with this change; they will be removed
in subsequent commits.
See arti#208 for older discussion on this issue.
NetworkConfigBuilder needs to not contain any validated structs, so
that its serde does not expose the validated details.
AuthorityListBuilder is what ought to go here - and it contains
Vec<AuthorityBuilder>, not Vec<Authority>. As a consequence, many
places now deal with AuthorityBuilder, rather than Authority.
Now the network fallbacks configuration wants to Deserialize
a Vec<FallbackDirBuilder>, rather than validated Vec<FallbackDir>.
Methods on FallbackListBuilder are as per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/462#note_2797697
mutatis mutandi for the fact that this struct has only fallbacks in it.
This commitid is the current head of my MR branch
https://github.com/colin-kiegel/rust-derive-builder/pull/253https://github.com/ijackson/rust-derive-builder/tree/field-builder
Using the commitid prevents surprises if that branch is updated.
We will require this newer version of derive_builder. The version
will need to be bumped again later, assuming the upstream MR is merged
and upstream do a release containing the needed changes.
We will need the new version of not only `derive_builder_core` (the
main macro implementation) but also`derive_builder` for a new error
type.
Not all of these strictly need to be bumped to 0.2.0; many could go
to 0.1.1 instead. But since everything at the tor-rtcompat and
higher layers has had breaking API changes, it seems not so useful
to distinguish. (It seems unlikely that anybody at this stage is
depending on e.g. tor-protover but not arti-client.)
The guard manager is responsible for handing out the first hops of
tor circuits, keeping track of their successes and failures, and
remembering their states. Given that, it makes sense to store this
information here. It is not yet used; I'll be fixing that in
upcoming commits.
Arguably, this information no longer belongs in the directory
manager: I've added a todo about moving it.
This commit will break compilation on its own in a couple of places;
subsequent commits will fix it up.
The various background daemon tasks that `arti-client` used to spawn are
now handled inside their respective crates instead, with functions
provided to spawn them that return `TaskHandle`s.
This required introducing a new trait, `NetDirProvider`, which steals
some functionality from the `DirProvider` trait to enable `tor-circmgr`
to depend on it (`tor-circmgr` is a dependency of `tor-dirmgr`, so it
can't depend on `DirProvider` directly).
While we're at it, we also make some of the tasks wait for events from
the `NetDirProvider` instead of sleeping, slightly increasing
efficiency.
There are two reasons why the DynFilter newtype might be needed:
1. To impl Default. But we don't need it to impl Default since we can
have an accessor which does the defaulting.
2. To hide the API. But this is usrely an unstable API.
Just writing Arc<dyn> gets rid of a lot of unnecessary boilerplate and
conversion code.
This is a revised version of !397; it implements a scheduling system for
periodic tasks that can be externally controlled, and then uses the
external control aspect to implement a basic dormant mode (#90).
More technically, the scheduling system consists of a `Stream` that
periodic tasks are expected to embed in a `while` loop or similar, a
way for tasks themselves to choose how long to wait until the stream
next yields a result, and a handle to control this outside of the task.
Instead of requiring a `Box<dyn Isolation>`, it now takes either a
`Box<dyn Isolation>`, or an arbitrary `T` that implements
`Isolation`.
This API still allows the user to pass in a `Box<dyn Isolation>` if
that's what they have, but it doesn't require them to Box the
isolation on their own.
Part of #414.
Now we use NetParams. That implies making its constructor public,
which I think it fine.
This is related to #413 but is far from completing that ticket.
This handwritten conversion function omitted a field. There was
nothing to spot this mistake.
IMO this shows why these particular types ought not to use builders,
but instead, should cause API breaks when things change.
Adding this line here to explicitly fix the bug, although we are about
to abolish this function completely almost right away.
Nick Mathewson
Ian Jackson
Orhun Parmaksız
Ian Jackson
trinity-1686a
michael mccune
eta
Samanta Navarro
solanav