aboutsummaryrefslogtreecommitdiff
path: root/crates/arti-config
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'clippy-allow-arc-clone' into 'main'Nick Mathewson2022-03-011-1/+0
|\ | | | | | | | | Disable clippy::clone_on_ref_ptr See merge request tpo/core/arti!352
| * Disable clippy::clone_on_ref_ptrIan Jackson2022-02-241-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This lint is IMO inherently ill-conceived. I have looked for the reasons why this might be thought to be a good idea and there were basically two (and they are sort of contradictory): I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact that only the pointer is being cloned, not the underlying data." This is the wording from https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr It is a bit terse; we are left to infer why it is a bad idea to obscure this fact. It seems to me that if it is bad to obscure some fact, that must be because the fact is a hazard. But why would it be a hazard to not copy the underlying data ? In other languages, faliing to copy the underlying data is a serious correctness hazard. There is a whose class of bugs where things were not copied, and then mutated and/or reused in multiple places in ways that were not what the programmer intended. In my experience, this is a very common bug when writing Python and Javascript. I'm told it's common in golang too. But in Rust this bug is much much harder to write. The data inside an Arc is immutable. To have this bug you'd have use interior mutability - ie mess around with Mutex or RefCell. That provides a good barrier to these kind of accidents. II. "The reason for writing Rc::clone and Arc::clone [is] to make it clear that only the pointer is being cloned, as opposed to the underlying data. The former is always fast, while the latter can be very expensive depending on what is being cloned." This is the reasoning found here https://github.com/rust-lang/rust-clippy/issues/2048 This is saying that *not* using Arc::clone is hazardous. Specifically, that a deep clone is a performance hazard. But for this argument, the lint is precisely backwards. It's linting the "good" case and asking for it to be written in a more explicit way; while the supposedly bad case can be written conveniently. Also, many objects (in our codebase, and in all the libraries we use) that are Clone are in fact simply handles. They contain Arc(s) (or similar) and are cheap to clone. Indeed, that is the usual case. It does not make sense to distinguish in the syntax we use to clone such a handle, whether the handle is a transparent Arc, or an opaque struct containing one or more other handles. Forcing Arc::clone to be written as such makes for code churn when a type is changed from Arc<Something> to Something: Clone, or vice versa.
* | Bump all crates to 0.1.0arti-v0.1.0Nick Mathewson2022-03-011-4/+4
| |
* | Update README.md files from rustdoc.Nick Mathewson2022-03-011-0/+8
| |
* | Add warnings about configuration stability.Nick Mathewson2022-02-281-0/+8
| |
* | Upgrade to newer version of config crate.Nick Mathewson2022-02-253-32/+56
|/
* Change deny(clippy::all) to warn(clippy::all).Nick Mathewson2022-02-141-1/+1
| | | | Closes #338.
* Fix a doc link.Nick Mathewson2022-02-041-1/+2
|
* Merge branch 'ticket270' into 'main'Nick Mathewson2022-02-033-4/+76
|\ | | | | | | | | | | | | Watch configuration files and reload them when they change Closes #270 See merge request tpo/core/arti!280
| * Document that `notify` behavior is strange with symlinksNick Mathewson2022-02-032-0/+10
| | | | | | | | | | | | | | | | | | | | (More specifically, `notify` behaves differently on different platforms. On some, it can watch specific directory objects on the filesystem, and so it only notices when _those_ directories change. If you change a symlink so that the canonical configuration file location is now in some other directory, `notify` won't notice. But on other platforms, notify just does "stat()" in a loop. On those, it _will_ notice if the configuration file changes.)
| * arti-config: add blank lines between functions.Nick Mathewson2022-02-021-0/+6
| |
| * arti-config: Small type and comment refactoring from review.Nick Mathewson2022-02-021-2/+0
| |
| * Make configuration-watching configurable and off-by-default.Nick Mathewson2022-02-013-2/+57
| | | | | | | | | | | | | | | | I'm slightly concerned about whether this is behavior people would expect to have on-by-default, so let's make this off-by-default for now. Maybe the `application` and `system` sections should merge?
| * Reload configuration when our configuration files change.Nick Mathewson2022-02-011-1/+4
| | | | | | | | Closes #270
* | Fix typosDimitris Apostolou2022-02-021-1/+1
|/
* arti_config: Refactor configuration sources into a structNick Mathewson2022-02-011-26/+73
| | | | | | | | | | | | | | | | | | | This is by no means our final API, but should represent an improvement. Here instead of having to specify a list of files and their is-this-optional status, along with a list of command-line options, we have a single structure that encapsulates all of that information. Two advantages here: - Callers no longer have to remember what the boolean means. - We can "reload" more easily, by keeping the source object around. This change also implements the correct behavior for our default configuration file in `arti::main`: if the file is absent and the user doesn't list a config file, that's no problem. But if the user lists _that very same config file, we should insist that it be present.
* Bump the patch version of every crate that changed since 0.0.3Nick Mathewson2022-01-311-4/+4
|
* Make max_file_limit configurableNeel Chauhan2022-01-282-1/+24
|
* Bump all crate versions to 0.0.3.Nick Mathewson2022-01-111-4/+4
|
* Tracing configuration for logfiles, per-target filtersNick Mathewson2022-01-103-23/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | Previously we could only configure one global tracing filter that applied to stdout and journald. There was no support for log files, either. This patch fixes both issues, by substantially revising the configuration format: There are now separate filters for each log file, for journald, and for the console log. Because we want to allow multiple logfiles, they have to go into an array in the configuration. The configuration logic has grown a bit complicated in its types, since the tracing_subscriber crate would prefer to have the complete structure of tracing Layers known statically. That's fine when you know how many you have, and which kinds there will be, but for the runtime-configuration case we need to mess around with `Box<dyn Layer ...>`. I also had to switch from tracing_subscriber's EnvFilter to its Targets filter. It seems "EnvFilter" can only be applied as a Layer in itself, and won't work as a Filter on an individual Layer. Closes #166. Closes #170.
* Expose and rename stream timeout config.Nick Mathewson2022-01-101-6/+6
| | | | | | | | | | | Previously we kept this in an ambiguously named type, `ClientTimeoutConfig`. But everything we do right now is client related! So `StreamTimeoutConfig` is a better name. Also, we'd previously neglected to expose the builder for this type from `TorClientConfigBuilder`. Now we do. Closes #281.
* Minimize the required version for each dependency.Nick Mathewson2022-01-071-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I found these versions empirically, by using the following process: First, I used `cargo tree --depth 1 --kind all` to get a list of every immediate dependency we had. Then, I used `cargo upgrade --workspace package@version` to change each dependency to the earliest version with which (in theory) the current version is semver-compatible. IOW, if the current version was 3.2.3, I picked "3". If the current version was 0.12.8, I picked "0.12". Then, I used `cargo +nightly upgrade -Z minimal-versions` to downgrade Cargo.lock to the minimal listed version for each dependency. (I had to override a few packages; see .gitlab-ci.yml for details). Finally, I repeatedly increased the version of each of our dependencies until our code compiled and the tests passed. Here's what I found that we need: anyhow >= 1.0.5: Earlier versions break our hyper example. async-broadcast >= 0.3.2: Earlier versions fail our tests. async-compression 0.3.5: Earlier versions handled futures and tokio differently. async-trait >= 0.1.2: Earlier versions are too buggy to compile our code. clap 2.33.0: For Arg::default_value_os(). coarsetime >= 0.1.20: exposed as_ticks() function. curve25519-dalek >= 3.2: For is_identity(). generic-array 0.14.3: Earlier versions don't implement From<&[T; 32]> httparse >= 1.2: Earlier versions didn't implement Error. itertools at 0.10.1: For at_most_once. rusqlite >= 0.26.3: for backward compatibility with older rustc. serde 1.0.103: Older versions break our code. serde_json >= 1.0.50: Since we need its Value type to implement Eq. shellexpand >= 2.1: To avoid a broken dirs crate version. tokio >= 1.4: For Handle::block_on(). tracing >= 0.1.18: Previously, tracing_core and tracing had separate LevelFilter types. typenum >= 1.12: Compatibility with rust-crypto crates x25519-dalek >= 1.2.0: For was_contributory(). Closes #275.
* extend lints to include 'clippy::all'Daniel Eades2021-12-281-0/+1
|
* Rename timeout_rules to stream_timeouts.Nick Mathewson2021-12-072-14/+18
| | | | | | | (There are other timeout rules, after all.) Also, rename stream_timeout to connect_timeout, to make it more clear when it applies.
* Merge branch 'revised_preemptive_config' into 'main'eta2021-12-072-28/+41
|\ | | | | | | | | Usability: renaming and documentation in preemptive circuit config See merge request tpo/core/arti!176
| * Clarify names and docs for predictive circuits.Nick Mathewson2021-12-072-13/+26
| | | | | | | | | | Also, use humantime_serde, rather than a number of seconds, to indicate configuration time.
| * Rename circuits_preemptive to preemptive_circuitsNick Mathewson2021-12-072-15/+15
| | | | | | | | | | | | This obeys a few conventions: * adjective before noun * config objects end with "config"
* | Merge branch 'bug252' into 'main'eta2021-12-072-2/+33
|\ \ | |/ |/| | | | | | | | | Make DNS fields in arti-client/src/client.rs configurable Closes #252 See merge request tpo/core/arti!171
| * Rename ClientDNSConfig -> ClientTimeoutConfigNeel Chauhan2021-12-072-11/+14
| |
| * Make DNS fields in arti-client/src/client.rs configurableNeel Chauhan2021-12-032-2/+30
| |
* | Merge branch 'preemptive-config' into 'main'eta2021-12-073-0/+42
|\ \ | | | | | | | | | | | | | | | | | | Allow configurability on preemptive circuits Closes #245 See merge request tpo/core/arti!164
| * | Allow configurability on preemptive circuitsNeel Chauhan2021-12-073-0/+42
| |/
* / Resolve roughly half of the XXXXs.Nick Mathewson2021-12-061-1/+3
|/ | | | | | | | We want to only use TODO in the codebase for non-blockers, and open tickets for anything that is a bigger blocker than a TODO. These XXXXs seem like definite non-blockers to me. Part of arti#231.
* Idle hacking to get 90% coverage in arti-configNick Mathewson2021-12-021-0/+18
| | | | | This is just a matter of writing a few tests for some very easy functions.
* Merge branch 'readme_fixes'Nick Mathewson2021-11-301-2/+2
|\
| * run ./maint/readmes.shdagon2021-11-291-2/+2
| |
* | Merge branch 'simplify_config_helpers' into 'main'eta2021-11-291-1/+1
|\ \ | | | | | | | | | | | | Change sane_defaults() and with_directories() See merge request tpo/core/arti!155
| * | Change sane_defaults() and with_directories()Nick Mathewson2021-11-291-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | The sane_defaults() call is now the same as you get from a default builder: by convention, we just call that method Default::default(). The with_directories() constructor makes more sense as a constructor for the TorClientConfigBuilder than for TorClientConfig.
* | | Bump every crate by one patch version.Nick Mathewson2021-11-291-3/+3
|/ /
* | Merge branch 'config-updates-and-tests'Nick Mathewson2021-11-292-16/+270
|\ \
| * | Document StorageConfig defaults better.Nick Mathewson2021-11-291-8/+11
| | | | | | | | | | | | (Also fix a couple of typos)
| * | Add basic tests for high-level buildersNick Mathewson2021-11-251-0/+57
| | | | | | | | | | | | | | | Make sure that we can change elements, and we can reconstruct builders that give us the same thing.
| * | Implement builder patterns for ArtiConfig.Nick Mathewson2021-11-252-6/+181
| | | | | | | | | | | | | | | | | | | | | | | | This commit implements the "metabuilder" pattern and the "builder reconstruction" pattern for the ArtiConfig type. I'm not 100% that this will be necessary, but it will certainly help with testing.
| * | Impl and test Default for high-level configsNick Mathewson2021-11-251-3/+22
| |/
* | add semicolons if nothing returnedDaniel Eades2021-11-251-0/+1
| |
* | deglob some enums, use concise iteration syntaxDaniel Eades2021-11-251-1/+1
|/
* Resolve a pair of rustdoc warnings.Nick Mathewson2021-11-241-1/+3
|
* Implement meta-builder pattern for TorClientConfigNick Mathewson2021-11-221-10/+27
| | | | | This should be ergonomic than having to construct every section of the configuration separately.
* Make every Config type implement Eq.Nick Mathewson2021-11-211-3/+3
| | | | | Doing this is necessary for reconfiguration support, and will help a lot with testing, too.
* For every* config type, make defaults consistent.Nick Mathewson2021-11-211-0/+17
| | | | | | | | | | This patch makes sure that for every* config type we have, the defaults you get from a Builder match those you get from Serde, and that both match the value that you get from arti_defaults.toml. Later down the line I'll be adding some tests to keep these in sync. * StorageConfig still has no defaults of its own, since we aren't so sure we want other applications to use Arti's directories by default.