Support arti.d config directories, and fixes to config reloading

Fixes #474 (closed)/#271 (closed) and #544 (closed)

Doing this race-free is not entirely straightforward. I chose a different appraoch to the one I proposed in #544 (closed), in the end: if config watching is enabled, we always do one additional reload right at startup, to gather any missed config change.

assigned to @Diziet

requested review from @eta

CI is unrelated: RustSec unmaintained warning for ansi-term. See !683 (merged)

added 27 commits

• 03fbd235...51eb0e6c - 12 commits from branch tpo/core:main
• f6caa21d - tor-config sources: Remove some unneeded .to_string() from tests
• 3b4ed9a9 - config: Do process hardening on reconfigure even if not watching
• 107ff017 - config watch: Make the mpsc channel part of FileWatcher
• 65ba4e83 - config watch: Break out prepare_watcher
• b505a189 - config watch: Rescan once on startup
• 425bdd4e - config watch: Re-establish watcher on each iteration
• ae8063cd - config watch: Provide watch_dir
• 0dadbecb - tor-basic-utils: Provide IoErrorExt is_not_a_directory()
• c7fe74ce - config sources: Introduce scan() and FoundConfigFiles
• d9aec06c - config sources: Supporting reading directories
• b99d4fc1 - config sources tests: Introduce test of reading directory
• 3d598bcc - config sources tests: Break out sources_nodefaults
• 46da22f7 - config sources tests: Test results of directory scan
• 23bce4e0 - config sources: Read arti.d as well as arti.toml
• 98a9785d - config watch: Fix and reduce debounce interval

Compare with previous version

Rebased to fix conflicts and (hopefully) CI

(edit: rebase was straightforward; one conflict in a semver.md)

We had a chat about this on IRC; I was unhappy with the "autodiscovery" mechanism (i.e. that it would call readdir() on something to tell whether it was a directory or file, and behave accordingly).

I think what we agreed on is:

• tor_config's ConfigurationSources::from_cmdline, should, instead of taking a list of configuration files, take a list of enum ConfigSource { File(PathBuf), Directory(PathBuf) } (name subject to bikeshed).
• arti would accept either files or directories in its command line, treating a path as a directory if it ends in the path separator (/), and as a file otherwise.

Apart from that change, though, I'm happy with this MR; the watching logic looks sound!

Now done.

I don't much like the result (see the commit message for "tor-config: Replace dir detection with ConfigurationSource enum"), but, I don't want to get blocked any longer.

Providing .push_file() and so on wouldn't really help, because almost all the call sites want the syntactic specification functionality in ConfigurationSource::from_path.

OTOH this did get rid of FoundConfigFile, which is nice.

mentioned in commit Diziet/arti@0dd13ad1

added 4 commits

• c2712847 - tor-config: MustRead: Make public
• 5f24df38 - tor-config: Provide is_syntactically_directory helper function
• 0dd13ad1 - tor-config: Replace dir detection with ConfigurationSource enum
• 0745b97a - tor-config source: just ConfigurationSource, not FoundConfigFile

Compare with previous version

CI failure was due to rustfmt introducing a clippy warning. (I ran clippy first, and then fmt before push.)

added 1 commit

• e65c49ff - fixup! tor-config: Replace dir detection with ConfigurationSource enum

Compare with previous version

marked this merge request as draft from Diziet/arti@e65c49ff

added 1 commit

• be37e380 - tor-config: semver.md: Document change to ConfigurationSource enum

Compare with previous version

I'd rather this comment describe the actual behaviour more directly: for example

/// If the path ends in the directory separator (`/` or `\` depending on platform)
/// it is treated as a directory. Otherwise, it is assumed to be a file.
/// No other checks are made.

I did mention "for example because it ends in a directory separator".

But that's not the only thing that is syntactically a directory. For example, /foo/bar/. is syntactically a directory, and should be treated as an instruction to read that directory. (I listed all the syntaxes that were available on Unix in the tests.)

I don't think it makes sense to describe the precise algorithm here, partly because it's not portable.

I don't think this necessarily has to be exhaustive, but I could go either way; arguably if we end up changing this API we'll need to change more than just this, so maybe this makes sense.

Making it non-exhaustive causes a problem in arti's file watching system, which can handle only directories and files. I think it's worth making this exhaustive (particularly given that changing this is going to probably break the API anyway) to save on some weird edge case there (or a separate type - getting rid of the separate type was a benefit of the ConfigurationSource enum.)

Looks good to me, modulo some small comments! Feel free to merge after addressing them (and squashing).

approved this merge request

mentioned in commit Diziet/arti@9c00ec7d

added 35 commits

• be37e380...709543bb - 15 commits from branch tpo/core:main
• cd243247 - tor-config sources: Remove some unneeded .to_string() from tests
• 25b5a539 - config: Do process hardening on reconfigure even if not watching
• 8e86599d - config watch: Make the mpsc channel part of FileWatcher
• 0f9bf12a - config watch: Break out prepare_watcher
• a7bb3a73 - config watch: Rescan once on startup
• 863c6615 - config watch: Re-establish watcher on each iteration
• 587fa5f4 - config watch: Provide watch_dir
• 2fa75be6 - tor-basic-utils: Provide IoErrorExt is_not_a_directory()
• 7d088cf8 - config sources: Introduce scan() and FoundConfigFiles
• 08767f59 - config sources: Supporting reading directories
• e4fea3e1 - config sources tests: Introduce test of reading directory
• b700816e - config sources tests: Break out sources_nodefaults
• ba94c4a4 - config sources tests: Test results of directory scan
• 7d8b3e2f - config sources: Read arti.d as well as arti.toml
• 7c0637ad - config watch: Fix and reduce debounce interval
• a3005d8c - tor-config: MustRead: Make public
• e98bdf60 - tor-config: Provide is_syntactically_directory helper function
• 9c00ec7d - tor-config: Replace dir detection with ConfigurationSource enum
• 2662fd0d - tor-config source: just ConfigurationSource, not FoundConfigFile
• ae5ca437 - tor-config: semver.md: Document change to ConfigurationSource enum

Compare with previous version

marked this merge request as ready

enabled an automatic merge when the pipeline for ae5ca437 succeeds

merged

mentioned in commit 87b88a5d

mentioned in issue #271 (closed)