| Commit message (Collapse) | Author | Age | Files | Lines |
| |\
| |
| |
| |
| | |
Disable clippy::clone_on_ref_ptr
See merge request tpo/core/arti!352
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | | |
Fixed conflict in crates/arti-client/src/lib.rs as per tree
from
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/364/#note_2782166
ie 13e55b8d7c22c26e55ba75823409b477f1bce66b
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | | |
Otherwise, it's impossible to get a static sqlite linkage without
also getting native-tls, even if you wanted rustls.
Closes #302.
|
| |/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This makes using the `PreferredRuntime` the first-class option inside
`arti-client`, freeing users who don't want to think about runtimes from
having to do so.
`TorClient::create_unbootstrapped` and `builder` now automatically
use this runtime, leaving only `builder_custom` for users who wish to
manually specify a runtime.
This lets us clean up the docs a lot: mentions of using custom runtimes
are now relegated to nearer the end of the crate-level documentation,
and we mostly just link to `tor_rtcompat`'s docs to explain more there.
Instead, we take some more time to explain how you use the builder API
to create clients synchronously.
Other doc cleanups included getting rid of the explanation of `TorAddr`
in the main crate-level doc; this is already well-documented elsewhere,
and is something users should discover organically later.
fixes arti#326
|
| | | |
|
| | | |
|
| |\ \
| |/
|/|
| |
| | |
make arti return a web page when receiving http request on socks port
See merge request tpo/core/arti!348
|
| | | |
|
| | | |
|
| |\ \
| |/
|/|
| |
| |
| |
| | |
Make a TorClientBuilder API.
Closes #350
See merge request tpo/core/arti!337
|
| | |
| |
| |
| |
| |
| |
| |
| | |
This is a defensive API choice to protect against the possibility
that we'll want to add a bunch of other non-config options in
the future.
Closes #350
|
| |\ \
| |/
|/|
| |
| | |
Improve error messages from arti cli
See merge request tpo/core/arti!331
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| | |
This makes arti less awkward to use.
|
| |/
|
|
| |
IMO this clarifies things a bit, and makes things more deterministic.
|
| |\ |
|
| | | |
|
| | | |
|
| |/
|
|
|
|
|
|
|
|
| |
This change is possible now that #293 is done.
As an immediate benefit, it allows us to start monitoring the
configuration files immediately, and not only after we're done
bootstrapping the client.
Closes #336.
|
| |
|
|
| |
Closes #338.
|
| |\
| |
| |
| |
| |
| |
| | |
Allow creating unbootstrapped `TorClient`s (and `DirMgr`s)
Closes #293
See merge request tpo/core/arti!298
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This commit changes how the `TorClient` type works, enabling it to be
constructed synchronously without initiating the bootstrapping process.
Daemon tasks are still started on construction (although some of them
won't do anything if the client isn't bootstrapped).
The old bootstrap() methods are now reimplemented in terms of the new
create_unbootstrapped() and bootstrap_existing() methods.
This required refactoring how the `DirMgr` works to enable the same sort
of thing there.
closes #293
|
| |/
|
|
|
| |
I'm not in love with this solution; the others just seem a bit ugly
too.
|
| |
|
|
|
|
|
|
|
|
|
| |
At least by default, we should have Error be private, and not expose
it as part of our APIs.
To keep functionality in `arti`, I had to add an `ExitTimeout` error
kind.
For interface consistency, I also re-exported ErrorKind and HasError
from `arti_client`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes these messages:
warning: this URL is not a hyperlink
--> crates/arti/src/watch_cfg.rs:115:5
|
115 | /// https://github.com/notify-rs/notify/issues/165 and
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com/notify-rs/notify/issues/165>`
|
= note: `#[warn(rustdoc::bare_urls)]` on by default
= note: bare URLs are not automatically turned into clickable links
warning: this URL is not a hyperlink
--> crates/arti/src/watch_cfg.rs:116:5
|
116 | /// https://github.com/notify-rs/notify/pull/166 .
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com/notify-rs/notify/pull/166>`
|
= note: bare URLs are not automatically turned into clickable links
|
| |\
| |
| |
| |
| |
| |
| | |
Watch configuration files and reload them when they change
Closes #270
See merge request tpo/core/arti!280
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
(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.)
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since the user can put their logfiles and configuration files in the
same directory, writing to the log can trigger an event from
`notify`. If we log every non-interesting event from `notify`, then
we'll trigger the logs every time we log, and fill up the disk.
This commit removes the offending log and adds a comment about why.
If we someday decide we do need to log here, maybe we can rate-limit
the messages or something.
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| | |
This way, if there are a bunch of changes at once, we only reload
one time.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Due to limitations in notify and the OS APIs it uses, it isn't
actually so useful to watch a single file. Instead, we have to
watch the directories that contain the files, and filter out any
events that aren't about the specific files we care about.
I've put the logic here into a new type, but I've left the type
un-exported: its API is pretty ugly, inasmuch as the caller needs to
jump through hoops to only get the events that they want. That's
not too bad so long as the API is private, but we'd want better if
we were exposing this.
|
| | | |
|
| | |
| |
| |
| | |
We don't yet do much with these, but we can avoid discarding them.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
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?
|
| | |
| |
| |
| | |
Closes #270
|
| |\ \
| | |
| | |
| | |
| | | |
Fix typos
See merge request tpo/core/arti!285
|
| | |/ |
|
| |/
|
|
|
|
|
| |
Found via clippy::needless_borrow. In some cases I removed needless
`[..]` too. See also:
needless_borrow suggestion doesn't go far enough
https://github.com/rust-lang/rust-clippy/issues/8389
|
| |
|
|
|
| |
TorClient doesn't need to be wrapped in an Arc any longer, thanks
to other refactoring.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| | |
|
| | |
|
| |
|
|
|
| |
This avoids a future confusion with the new `SpawnBlocking` trait in
async_executors v0.5, and better describes what the trait provides.
|
| |
|
|
|
|
|
|
|
|
|
| |
This commit puts the native-tls crate behind a feature. The feature
is off-by-default in the tor-rtcompat crate, but can be enabled
either from arti or arti-client.
There is an included script that I used to test that tor-rtcompat
could build and run its tests with all subsets of its features.
Closes #300
|
| |
|
|
|
| |
This should help avoid some amount of temptation towards API
proliferation.
|