[ffx] Wrap plugin injections in single Injection
By creating a single Injector trait, the ffx plugin signature is
simplified.
This appears to help with memory usage as well as compile
time/parallelism while compiling:
HEAD: fx build ffx 495.12s user 170.13s system 384% cpu 2:52.93 total
CL: fx build ffx 362.33s user 166.46s system 1134% cpu 46.609 total
Bug: 74452
MULTIPLY: ffx_core_impl_lib_test
Change-Id: I767cf94166f42eaac2ea0a5e1f6e4856c9582ca9
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/518460
Commit-Queue: Matthew Boetger <boetger@google.com>
Reviewed-by: James Tucker <raggi@google.com>
diff --git a/src/developer/ffx/build/ffx.gni b/src/developer/ffx/build/ffx.gni
index e863af7..382523a 100644
--- a/src/developer/ffx/build/ffx.gni
+++ b/src/developer/ffx/build/ffx.gni
@@ -55,6 +55,7 @@
"//third_party/rust_crates:anyhow",
"//third_party/rust_crates:argh",
"//third_party/rust_crates:async-net",
+ "//third_party/rust_crates:async-trait",
"//third_party/rust_crates:futures",
"//third_party/rust_crates:hex",
"//third_party/rust_crates:lazy_static",
diff --git a/src/developer/ffx/build/templates/plugins.md b/src/developer/ffx/build/templates/plugins.md
index d4edd31..258a616 100644
--- a/src/developer/ffx/build/templates/plugins.md
+++ b/src/developer/ffx/build/templates/plugins.md
@@ -1,55 +1,32 @@
-pub async fn ffx_plugin_impl<D, DFut, R, RFut, E, EFut, F, FFut>(
+pub async fn ffx_plugin_impl<I: ffx_core::Injector>(
{% if not includes_execution and not includes_subcommands %}
- _daemon_factory: D,
- _remote_factory: R,
- _fastboot_factory: F,
- _is_experiment: E,
+ _injector: I,
_cmd: {{suite_args_lib}}::FfxPluginCommand,
{% else %}
- daemon_factory: D,
- remote_factory: R,
- fastboot_factory: F,
- is_experiment: E,
+ injector: I,
cmd: {{suite_args_lib}}::FfxPluginCommand,
{% endif %}
) -> Result<(), anyhow::Error>
- where
- D: Fn() -> DFut,
- DFut: std::future::Future<
- Output = anyhow::Result<fidl_fuchsia_developer_bridge::DaemonProxy>,
- >,
- R: Fn() -> RFut,
- RFut: std::future::Future<
- Output = anyhow::Result<
- fidl_fuchsia_developer_remotecontrol::RemoteControlProxy,
- >,
- >,
- F: Fn() -> FFut,
- FFut: std::future::Future<
- Output = anyhow::Result<fidl_fuchsia_developer_bridge::FastbootProxy>,
- >,
- E: Fn(&'static str) -> EFut,
- EFut: std::future::Future<Output = bool>,
{
{% if includes_execution %}
{% if includes_subcommands %}
match cmd.subcommand {
Some(sub) => match sub {
{% for plugin in plugins %}
- {{suite_subcommand_lib}}::Subcommand::{{plugin.enum}}(c) => {{plugin.lib}}_suite::ffx_plugin_impl(daemon_factory, remote_factory, fastboot_factory, is_experiment, c).await,
+ {{suite_subcommand_lib}}::Subcommand::{{plugin.enum}}(c) => {{plugin.lib}}_suite::ffx_plugin_impl(injector, c).await,
{% endfor %}
},
- None => {{execution_lib}}::ffx_plugin_impl(daemon_factory, remote_factory, fastboot_factory, is_experiment, cmd).await
+ None => {{execution_lib}}::ffx_plugin_impl(injector, cmd).await
}
{% else %}
- {{execution_lib}}::ffx_plugin_impl(daemon_factory, remote_factory, fastboot_factory, is_experiment, cmd).await
+ {{execution_lib}}::ffx_plugin_impl(injector, cmd).await
{% endif %}
{% else %}
{% if includes_subcommands %}
match cmd.subcommand {
{% for plugin in plugins %}
- {{suite_subcommand_lib}}::Subcommand::{{plugin.enum}}(c) => {{plugin.lib}}_suite::ffx_plugin_impl(daemon_factory, remote_factory, fastboot_factory, is_experiment, c).await,
+ {{suite_subcommand_lib}}::Subcommand::{{plugin.enum}}(c) => {{plugin.lib}}_suite::ffx_plugin_impl(injector, c).await,
{% endfor %}
}
{% else %}
diff --git a/src/developer/ffx/core/macro/src/impl.rs b/src/developer/ffx/core/macro/src/impl.rs
index 3428a47..7d6d4e6 100644
--- a/src/developer/ffx/core/macro/src/impl.rs
+++ b/src/developer/ffx/core/macro/src/impl.rs
@@ -277,7 +277,7 @@
let output_fut = Ident::new(&format!("{}_fut", factory_name), Span::call_site());
let output_fut_res =
Ident::new(&format!("{}_fut_res", factory_name), Span::call_site());
- let implementation = quote! { let #output_fut = #factory_name(); };
+ let implementation = quote! { let #output_fut = injector.#factory_name(); };
let testing =
generate_fake_test_proxy_method(pat_ident.ident.clone(), proxy_type_path);
let arg = match proxy_wrapper_type {
@@ -371,7 +371,7 @@
async {
// This needs to be a block in order for this `use` to avoid conflicting with a plugins own `use` for this trait.
use futures::TryFutureExt;
- remote_factory().await?
+ injector.remote_factory().await?
.connect(#selector, #server_end.into_channel())
.map_ok_or_else(|e| Result::<(), anyhow::Error>::Err(anyhow::anyhow!(e)), |fidl_result| {
fidl_result
@@ -425,10 +425,7 @@
} = parse_arguments(input.sig.inputs.clone(), &proxies)?;
let mut outer_args: Punctuated<_, Token!(,)> = Punctuated::new();
- outer_args.push(quote! {daemon_factory: D});
- outer_args.push(quote! {remote_factory: R});
- outer_args.push(quote! {fastboot_factory: F});
- outer_args.push(quote! {is_experiment: E});
+ outer_args.push(quote! {injector: I});
outer_args.push(quote! {#cmd});
let implementation = if proxies_to_generate.len() > 0 {
@@ -446,7 +443,7 @@
let gated_impl = if let Some(key) = proxies.experiment_key {
quote! {
- if is_experiment(#key).await {
+ if injector.is_experiment(#key).await {
#implementation
} else {
println!("This is an experimental subcommand. To enable this subcommand run 'ffx config set {} true'", #key);
@@ -459,27 +456,7 @@
let res = quote! {
#input
- pub async fn ffx_plugin_impl<D, R, DFut, RFut, E, EFut, F, FFut>(
- #outer_args
- ) -> anyhow::Result<()>
- where
- D: Fn() -> DFut,
- DFut: std::future::Future<
- Output = anyhow::Result<fidl_fuchsia_developer_bridge::DaemonProxy>,
- >,
- R: Fn() -> RFut,
- RFut: std::future::Future<
- Output = anyhow::Result<
- fidl_fuchsia_developer_remotecontrol::RemoteControlProxy
- >,
- >,
- E: Fn(&'static str) -> EFut,
- EFut: std::future::Future<Output = bool>,
- F: Fn() -> FFut,
- FFut: std::future::Future<
- Output = anyhow::Result<fidl_fuchsia_developer_bridge::FastbootProxy>,
- >,
- {
+ pub async fn ffx_plugin_impl<I: ffx_core::Injector>(#outer_args) -> anyhow::Result<()> {
#gated_impl
}
diff --git a/src/developer/ffx/core/src/lib.rs b/src/developer/ffx/core/src/lib.rs
index 0bd6875..1dede9a 100644
--- a/src/developer/ffx/core/src/lib.rs
+++ b/src/developer/ffx/core/src/lib.rs
@@ -6,6 +6,9 @@
use {
anyhow::Result,
+ async_trait::async_trait,
+ fidl_fuchsia_developer_bridge::{DaemonProxy, FastbootProxy},
+ fidl_fuchsia_developer_remotecontrol::RemoteControlProxy,
futures::stream::{FuturesUnordered, StreamExt, TryStream},
futures::{future::FusedFuture, Future},
pin_project::pin_project,
@@ -19,6 +22,14 @@
pub use ffx_build_version::build_info;
+#[async_trait]
+pub trait Injector {
+ async fn daemon_factory(&self) -> Result<DaemonProxy>;
+ async fn remote_factory(&self) -> Result<RemoteControlProxy>;
+ async fn fastboot_factory(&self) -> Result<FastbootProxy>;
+ async fn is_experiment(&self, key: &str) -> bool;
+}
+
// Error type for wrapping errors known to an `ffx` command and whose occurrence should
// not a priori be considered a bug in ffx.
// TODO(fxbug.dev/57592): consider extending this to allow custom types from plugins.
diff --git a/src/developer/ffx/src/main.rs b/src/developer/ffx/src/main.rs
index b798618..020e80f 100644
--- a/src/developer/ffx/src/main.rs
+++ b/src/developer/ffx/src/main.rs
@@ -6,8 +6,9 @@
analytics::{add_crash_event, get_notice},
anyhow::{anyhow, Context, Result},
async_once::Once,
+ async_trait::async_trait,
ffx_core::metrics::{add_fx_launch_event, init_metrics_svc},
- ffx_core::{ffx_bail, ffx_error, FfxError},
+ ffx_core::{ffx_bail, ffx_error, FfxError, Injector},
ffx_daemon::{get_daemon_proxy_single_link, is_daemon_running},
ffx_lib_args::{from_env, Ffx},
ffx_lib_sub_command::Subcommand,
@@ -16,8 +17,8 @@
fidl_fuchsia_developer_remotecontrol::{RemoteControlMarker, RemoteControlProxy},
fuchsia_async::TimeoutExt,
futures::{Future, FutureExt},
- lazy_static::lazy_static,
ring::digest::{Context as ShaContext, Digest, SHA256},
+ std::default::Default,
std::error::Error,
std::fs::File,
std::io::{BufReader, Read},
@@ -80,14 +81,78 @@
const DAEMON_CONNECTION_ISSUE: &str = "\
Timed out waiting on the Daemon.\nRun `ffx doctor` for further diagnostics";
-lazy_static! {
- static ref DAEMON_ONCE: Once<DaemonProxy> = Once::new();
+struct Injection {
+ daemon_once: Once<DaemonProxy>,
}
-// This could get called multiple times by the plugin system via multiple threads - so make sure
-// the spawning only happens one thread at a time.
-async fn get_daemon_proxy() -> Result<DaemonProxy> {
- DAEMON_ONCE.get_or_try_init(init_daemon_proxy()).await.map(|proxy| proxy.clone())
+impl Default for Injection {
+ fn default() -> Self {
+ Self { daemon_once: Once::new() }
+ }
+}
+
+#[async_trait]
+impl Injector for Injection {
+ // This could get called multiple times by the plugin system via multiple threads - so make sure
+ // the spawning only happens one thread at a time.
+ async fn daemon_factory(&self) -> Result<DaemonProxy> {
+ self.daemon_once.get_or_try_init(init_daemon_proxy()).await.map(|proxy| proxy.clone())
+ }
+
+ async fn fastboot_factory(&self) -> Result<FastbootProxy> {
+ let daemon_proxy = self.daemon_factory().await?;
+ let (fastboot_proxy, fastboot_server_end) = create_proxy::<FastbootMarker>()?;
+ let app: Ffx = argh::from_env();
+ let result = timeout(
+ proxy_timeout().await?,
+ daemon_proxy.get_fastboot(
+ app.target().await?.as_ref().map(|s| s.as_str()),
+ fastboot_server_end,
+ ),
+ )
+ .await
+ .context("timeout")?
+ .context("connecting to Fastboot")?;
+
+ match result {
+ Ok(_) => Ok(fastboot_proxy),
+ Err(DaemonError::NonFastbootDevice) => Err(ffx_error!(NON_FASTBOOT_MSG).into()),
+ Err(DaemonError::TargetAmbiguous) => Err(ffx_error!(TARGET_AMBIGUOUS_MSG).into()),
+ Err(DaemonError::TargetNotFound) => Err(ffx_error!(TARGET_NOT_FOUND_MSG).into()),
+ Err(DaemonError::TargetCacheError) => Err(ffx_error!(TARGET_FAILURE_MSG).into()),
+ Err(e) => Err(anyhow!("unexpected failure connecting to Fastboot: {:?}", e)),
+ }
+ }
+
+ async fn remote_factory(&self) -> Result<RemoteControlProxy> {
+ let daemon_proxy = self.daemon_factory().await?;
+ let (remote_proxy, remote_server_end) = create_proxy::<RemoteControlMarker>()?;
+ let app: Ffx = argh::from_env();
+
+ let result = timeout(
+ proxy_timeout().await?,
+ daemon_proxy.get_remote_control(
+ app.target().await?.as_ref().map(|s| s.as_str()),
+ remote_server_end,
+ ),
+ )
+ .await
+ .context("timeout")?
+ .context("connecting to target via daemon")?;
+
+ match result {
+ Ok(_) => Ok(remote_proxy),
+ Err(DaemonError::TargetAmbiguous) => Err(ffx_error!(TARGET_AMBIGUOUS_MSG).into()),
+ Err(DaemonError::TargetNotFound) => Err(ffx_error!(TARGET_NOT_FOUND_MSG).into()),
+ Err(DaemonError::TargetCacheError) => Err(ffx_error!(TARGET_FAILURE_MSG).into()),
+ Err(DaemonError::TargetInFastboot) => Err(ffx_error!(TARGET_IN_FASTBOOT).into()),
+ Err(e) => Err(anyhow!("unexpected failure connecting to RCS: {:?}", e)),
+ }
+ }
+
+ async fn is_experiment(&self, key: &str) -> bool {
+ ffx_config::get(key).await.unwrap_or(false)
+ }
}
async fn init_daemon_proxy() -> Result<DaemonProxy> {
@@ -185,59 +250,6 @@
}
}
-async fn get_fastboot_proxy() -> Result<FastbootProxy> {
- let daemon_proxy = get_daemon_proxy().await?;
- let (fastboot_proxy, fastboot_server_end) = create_proxy::<FastbootMarker>()?;
- let app: Ffx = argh::from_env();
- let result = timeout(
- proxy_timeout().await?,
- daemon_proxy
- .get_fastboot(app.target().await?.as_ref().map(|s| s.as_str()), fastboot_server_end),
- )
- .await
- .context("timeout")?
- .context("connecting to Fastboot")?;
-
- match result {
- Ok(_) => Ok(fastboot_proxy),
- Err(DaemonError::NonFastbootDevice) => Err(ffx_error!(NON_FASTBOOT_MSG).into()),
- Err(DaemonError::TargetAmbiguous) => Err(ffx_error!(TARGET_AMBIGUOUS_MSG).into()),
- Err(DaemonError::TargetNotFound) => Err(ffx_error!(TARGET_NOT_FOUND_MSG).into()),
- Err(DaemonError::TargetCacheError) => Err(ffx_error!(TARGET_FAILURE_MSG).into()),
- Err(e) => Err(anyhow!("unexpected failure connecting to Fastboot: {:?}", e)),
- }
-}
-
-async fn get_remote_proxy() -> Result<RemoteControlProxy> {
- let daemon_proxy = get_daemon_proxy().await?;
- let (remote_proxy, remote_server_end) = create_proxy::<RemoteControlMarker>()?;
- let app: Ffx = argh::from_env();
-
- let result = timeout(
- proxy_timeout().await?,
- daemon_proxy.get_remote_control(
- app.target().await?.as_ref().map(|s| s.as_str()),
- remote_server_end,
- ),
- )
- .await
- .context("timeout")?
- .context("connecting to target via daemon")?;
-
- match result {
- Ok(_) => Ok(remote_proxy),
- Err(DaemonError::TargetAmbiguous) => Err(ffx_error!(TARGET_AMBIGUOUS_MSG).into()),
- Err(DaemonError::TargetNotFound) => Err(ffx_error!(TARGET_NOT_FOUND_MSG).into()),
- Err(DaemonError::TargetCacheError) => Err(ffx_error!(TARGET_FAILURE_MSG).into()),
- Err(DaemonError::TargetInFastboot) => Err(ffx_error!(TARGET_IN_FASTBOOT).into()),
- Err(e) => Err(anyhow!("unexpected failure connecting to RCS: {:?}", e)),
- }
-}
-
-async fn is_experiment_subcommand_on(key: &'static str) -> bool {
- ffx_config::get(key).await.unwrap_or(false)
-}
-
fn is_daemon(subcommand: &Option<Subcommand>) -> bool {
if let Some(Subcommand::FfxDaemonPlugin(ffx_daemon_plugin_args::DaemonCommand {
subcommand: ffx_daemon_plugin_sub_command::Subcommand::FfxDaemonStart(_),
@@ -316,14 +328,7 @@
});
let command_start = Instant::now();
- let res = ffx_lib_suite::ffx_plugin_impl(
- get_daemon_proxy,
- get_remote_proxy,
- get_fastboot_proxy,
- is_experiment_subcommand_on,
- app,
- )
- .await;
+ let res = ffx_lib_suite::ffx_plugin_impl(Injection::default(), app).await;
let command_done = Instant::now();
log::info!("Command completed. Success: {}", res.is_ok());
