Merge branch 'ptmgr-optional-dep' into 'main'

Support building without pluggable transports

Closes #649

See merge request tpo/core/arti!898

.gitlab-ci.yml

@@ -287,6 +287,12 @@ matrix-test:
     - apt-get update && apt-get install -y python3-toml python-is-python3
     - ./maint/matrix_test
 
+matrix-test-cfg:
+  stage: test
+  image: rust:latest
+  script:
+    - ./maint/matrix_test_cfg
+
 
 coverage-aggregated:
   rules:

crates/arti-client/Cargo.toml

@@ -36,7 +36,7 @@ async-std = ["tor-rtcompat/async-std"]
 bridge-client = ["tor-guardmgr/bridge-client", "tor-dirmgr/bridge-client"]
 tokio = ["tor-rtcompat/tokio", "tor-proto/tokio"]
 native-tls = ["tor-rtcompat/native-tls"]
-pt-client = ["bridge-client", "tor-chanmgr/pt-client", "tor-guardmgr/pt-client"]
+pt-client = ["bridge-client", "tor-chanmgr/pt-client", "tor-guardmgr/pt-client", "tor-ptmgr"]
 rustls = ["tor-rtcompat/rustls"]
 
 static = ["static-sqlite", "static-native-tls"]
@@ -99,7 +99,7 @@ tor-netdir = { path = "../tor-netdir", version = "0.6.0" }
 tor-netdoc = { path = "../tor-netdoc", version = "0.5.2" }
 tor-persist = { path = "../tor-persist", version = "0.5.1" }
 tor-proto = { path = "../tor-proto", version = "0.7.0" }
-tor-ptmgr = { path = "../tor-ptmgr", version = "0.1.0" }
+tor-ptmgr = { path = "../tor-ptmgr", version = "0.1.0", optional = true }
 tor-rtcompat = { path = "../tor-rtcompat", version = "0.7.0" }
 tracing = "0.1.18"
 
@@ -119,6 +119,7 @@ tokio-crate = { package = "tokio", version = "1.7", features = [
 tokio-util = { version = "0.7.0", features = ["compat"] }
 tor-rtcompat = { path = "../tor-rtcompat", version = "0.7.0", features = ["tokio", "native-tls"] }
 tracing-subscriber = "0.3.0"
+
 [package.metadata.docs.rs]
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]

crates/arti-client/src/config.rs

@@ -44,9 +44,7 @@ pub mod dir {
 }
 
 /// Types for configuring pluggable transports.
-//
-// TODO pt-client: Could we make these ignored when pt-client is not enabled? If
-// so, we could make the ptmgr dependency optional, which is something we wanted.
+#[cfg(feature = "pt-client")]
 pub mod pt {
     pub use tor_ptmgr::config::{ManagedTransportConfig, ManagedTransportConfigBuilder};
 }
@@ -245,12 +243,15 @@ pub struct BridgesConfig {
     /// Configured list of pluggable transports.
     #[builder(sub_builder, setter(custom))]
     #[builder_field_attr(serde(default))]
+    #[cfg(feature = "pt-client")]
     pub(crate) transports: TransportConfigList,
 }
 
 /// A list of configured transport binaries (type alias for macrology).
+#[cfg(feature = "pt-client")]
 type TransportConfigList = Vec<pt::ManagedTransportConfig>;
 
+#[cfg(feature = "pt-client")]
 define_list_builder_helper! {
     pub(crate) struct TransportConfigListBuilder {
         transports: [pt::ManagedTransportConfigBuilder],

crates/arti/src/cfg.rs

@@ -569,6 +569,7 @@ mod test {
         // example present in ARTI_EXAMPLE_CONFIG.
         // https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/823#note_2854365
         // and bullet points 2 and 3 in the doc for `exhaustive_1`, below.
+        #[cfg(feature = "pt-client")]
         "bridges.transports",
         "tor_network.authorities",
         "tor_network.fallback_caches",
@@ -964,6 +965,7 @@ mod test {
 
         fn parse(&self) -> config::Config {
             let s: String = chain!(iter::once(&self.section), self.lines.iter(),).join("\n");
+            eprintln!("parsing\n  --\n{}\n  --", &s);
             let c: toml::Value = toml::from_str(&s).expect(&s);
             config::Config::try_from(&c).expect(&s)
         }

crates/tor-config/src/list_builder.rs

@@ -133,11 +133,13 @@
 //! assert_eq!{ builder.build().unwrap().values, &[27, 12] }
 //! ```
 
+use std::fmt;
+use std::marker::PhantomData;
 use std::str::FromStr;
 
 use educe::Educe;
 use itertools::Itertools;
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Deserializer, Serialize};
 use thiserror::Error;
 
 pub use crate::define_list_builder_accessors;
@@ -568,7 +570,7 @@ define_list_builder_helper! {
 /// let lc = lc.build().unwrap();
 /// assert_eq!{ lc.winners, [4,5,6] }
 /// ```
-#[derive(Clone, Debug, Educe, Serialize, Deserialize)]
+#[derive(Clone, Debug, Educe, Serialize)]
 #[serde(untagged)]
 #[educe(Default)]
 #[non_exhaustive]
@@ -606,6 +608,54 @@ pub struct MultilineListBuilderError<E: std::error::Error + Clone + Send + Sync>
     error: E,
 }
 
+// We could derive this with `#[serde(untagged)]` but that produces quite terrible error
+// messages, which do not reproduce the error messages from any of the variants.
+//
+// Instead, have a manual implementation, which can see whether the input is a list or a string.
+impl<'de, EB: Deserialize<'de>> Deserialize<'de> for MultilineListBuilder<EB> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        deserializer.deserialize_any(MllbVisitor::default())
+    }
+}
+
+/// Visitor for deserialize_any for [`MultilineListBuilder`]
+#[derive(Educe)]
+#[educe(Default)]
+struct MllbVisitor<EB> {
+    /// Variance: this visitor constructs `EB`s
+    ret: PhantomData<fn() -> EB>,
+}
+
+impl<'de, EB: Deserialize<'de>> serde::de::Visitor<'de> for MllbVisitor<EB> {
+    type Value = MultilineListBuilder<EB>;
+
+    fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "list of items, or multi-line string")
+    }
+
+    fn visit_seq<A: serde::de::SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value, A::Error> {
+        let mut v = vec![];
+        while let Some(e) = seq.next_element()? {
+            v.push(e);
+        }
+        Ok(MultilineListBuilder::List(v))
+    }
+
+    fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Self::Value, E> {
+        self.visit_string(v.to_owned())
+    }
+    fn visit_string<E: serde::de::Error>(self, v: String) -> Result<Self::Value, E> {
+        Ok(MultilineListBuilder::String(v))
+    }
+
+    fn visit_none<E: serde::de::Error>(self) -> Result<Self::Value, E> {
+        Ok(MultilineListBuilder::Unspecified)
+    }
+}
+
 impl<EB> From<Option<Vec<EB>>> for MultilineListBuilder<EB> {
     fn from(list: Option<Vec<EB>>) -> Self {
         use MultilineListBuilder as MlLB;

crates/tor-guardmgr/src/guard.rs

@@ -15,7 +15,10 @@ use crate::skew::SkewObservation;
 use crate::util::randomize_time;
 use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage};
 use crate::{sample, ExternalActivity, GuardSetSelector, GuardUsageKind};
+
+#[cfg(feature = "bridge-client")]
 use safelog::Redactable as _;
+
 use tor_linkspec::{
     ChanTarget, ChannelMethod, HasAddrs, HasChanMethod, HasRelayIds, PtTarget, RelayIds,
 };
@@ -86,6 +89,7 @@ pub(crate) enum DisplayRule {
     /// $ab...".
     ///
     /// We use this for bridges.
+    #[cfg(feature = "bridge-client")]
     Redacted,
 }
 
@@ -847,6 +851,7 @@ impl std::fmt::Display for Guard {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self.sensitivity {
             DisplayRule::Sensitive => safelog::sensitive(self.display_chan_target()).fmt(f),
+            #[cfg(feature = "bridge-client")]
             DisplayRule::Redacted => self.display_chan_target().redacted().fmt(f),
         }
     }

maint/matrix_test_cfg

@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Run cargo check and cargo test with various featuresets relevant to configuration
+
+set -euo pipefail
+
+: "${CARGO:=cargo}"
+
+set -x
+
+for feat in '' bridge-client pt-client; do
+    feat_args=(--no-default-features "--features=tokio,native-tls,$feat")
+
+    $CARGO check "${feat_args[@]}" -p arti-client
+    $CARGO check "${feat_args[@]}" -p arti
+    $CARGO test "${feat_args[@]}" -p arti -- cfg
+done
+