Make invocation recorder not require a repo

Summary: Will be used in the next diff so there is no dependency of log commands to repo

Reviewed By: iguridi

Differential Revision: D64153503

fbshipit-source-id: 12d8d4ed2061c92995b7614cf5212efcf8a6673d

app/buck2/src/lib.rs

@@ -51,7 +51,9 @@ use buck2_client_ctx::version::BuckVersion;
 use buck2_cmd_starlark_client::StarlarkCommand;
 use buck2_common::argv::Argv;
 use buck2_common::invocation_paths::InvocationPaths;
+use buck2_common::invocation_paths_result::InvocationPathsResult;
 use buck2_common::invocation_roots::find_invocation_roots;
+use buck2_common::invocation_roots::BuckCliError;
 use buck2_core::buck2_env_anyhow;
 use buck2_core::fs::paths::file_name::FileNameBuf;
 use buck2_event_observer::verbosity::Verbosity;
@@ -294,25 +296,44 @@ impl CommandKind {
         common_opts: BeforeSubcommandOptions,
     ) -> ExitResult {
         let roots = find_invocation_roots(process.working_dir.path());
-        let paths = roots
-            .map(|r| InvocationPaths {
-                roots: r,
-                isolation: common_opts.isolation_dir.clone(),
-            })
-            .map_err(buck2_error::Error::from);
-
+        let paths_anyhow = roots.map(|r| InvocationPaths {
+            roots: r,
+            isolation: common_opts.isolation_dir.clone(),
+        });
+
+        let paths_result = match paths_anyhow {
+            Ok(paths) => InvocationPathsResult::Paths(paths.clone()),
+            Err(err) => match err.downcast_ref::<BuckCliError>() {
+                Some(BuckCliError::NoBuckRoot(_)) => {
+                    InvocationPathsResult::OutsideOfRepo(buck2_error::Error::from(err))
+                }
+                None => InvocationPathsResult::OtherError(buck2_error::Error::from(err)),
+            },
+        };
         // Handle the daemon command earlier: it wants to fork, but the things we do below might
         // want to create threads.
         #[cfg(not(client_only))]
         if let CommandKind::Daemon(cmd) = self {
             return cmd
-                .exec(process.log_reload_handle.dupe(), paths?, false, || {})
+                .exec(
+                    process.log_reload_handle.dupe(),
+                    paths_result.get_result()?,
+                    false,
+                    || {},
+                )
                 .into();
         }
         thread::scope(|scope| {
             // Spawn a thread to have stack size independent on linker/environment.
             match thread_spawn_scoped("buck2-main", scope, move || {
-                self.exec_no_daemon(common_opts, process, immediate_config, matches, argv, paths)
+                self.exec_no_daemon(
+                    common_opts,
+                    process,
+                    immediate_config,
+                    matches,
+                    argv,
+                    paths_result,
+                )
             }) {
                 Ok(t) => match t.join() {
                     Ok(res) => res,
@@ -330,7 +351,7 @@ impl CommandKind {
         immediate_config: &ImmediateConfigContext,
         matches: &clap::ArgMatches,
         argv: Argv,
-        paths: buck2_error::Result<InvocationPaths>,
+        paths: InvocationPathsResult,
     ) -> ExitResult {
         if common_opts.no_buckd {
             // `no_buckd` can't work in a client-only binary
@@ -348,7 +369,7 @@ impl CommandKind {
             #[cfg(not(client_only))]
             let v = buck2_daemon::no_buckd::start_in_process_daemon(
                 immediate_config.daemon_startup_config()?,
-                paths.clone()?,
+                paths.clone().get_result()?,
                 &runtime,
             )?;
             #[cfg(client_only)]
@@ -375,6 +396,7 @@ impl CommandKind {
             &runtime,
             common_opts.oncall,
             common_opts.client_metadata,
+            common_opts.isolation_dir,
         );
 
         match self {

app/buck2_client_ctx/src/client_ctx.rs

@@ -16,7 +16,9 @@ use buck2_cli_proto::client_context::PreemptibleWhen as GrpcPreemptibleWhen;
 use buck2_cli_proto::ClientContext;
 use buck2_common::argv::Argv;
 use buck2_common::invocation_paths::InvocationPaths;
+use buck2_common::invocation_paths_result::InvocationPathsResult;
 use buck2_core::error::buck2_hard_error_env;
+use buck2_core::fs::paths::file_name::FileNameBuf;
 use buck2_core::fs::working_dir::WorkingDir;
 use buck2_event_observer::verbosity::Verbosity;
 use buck2_util::cleanup_ctx::AsyncCleanupContext;
@@ -42,7 +44,7 @@ use crate::subscribers::recorder::try_get_invocation_recorder;
 pub struct ClientCommandContext<'a> {
     init: fbinit::FacebookInit,
     pub(crate) immediate_config: &'a ImmediateConfigContext<'a>,
-    paths: buck2_error::Result<InvocationPaths>,
+    paths: InvocationPathsResult,
     pub working_dir: WorkingDir,
     pub verbosity: Verbosity,
     /// When set, this function is called to launch in process daemon.
@@ -59,13 +61,14 @@ pub struct ClientCommandContext<'a> {
     runtime: &'a Runtime,
     oncall: Option<String>,
     pub(crate) client_metadata: Vec<ClientMetadata>,
+    pub(crate) isolation: FileNameBuf,
 }
 
 impl<'a> ClientCommandContext<'a> {
     pub fn new(
         init: fbinit::FacebookInit,
         immediate_config: &'a ImmediateConfigContext<'a>,
-        paths: buck2_error::Result<InvocationPaths>,
+        paths: InvocationPathsResult,
         working_dir: WorkingDir,
         verbosity: Verbosity,
         start_in_process_daemon: Option<Box<dyn FnOnce() -> anyhow::Result<()> + Send + Sync>>,
@@ -78,6 +81,7 @@ impl<'a> ClientCommandContext<'a> {
         runtime: &'a Runtime,
         oncall: Option<String>,
         client_metadata: Vec<ClientMetadata>,
+        isolation: FileNameBuf,
     ) -> Self {
         ClientCommandContext {
             init,
@@ -95,6 +99,7 @@ impl<'a> ClientCommandContext<'a> {
             runtime,
             oncall,
             client_metadata,
+            isolation,
         }
     }
 
@@ -104,8 +109,18 @@ impl<'a> ClientCommandContext<'a> {
 
     pub fn paths(&self) -> anyhow::Result<&InvocationPaths> {
         match &self.paths {
-            Ok(p) => Ok(p),
-            Err(e) => Err(e.dupe().into()),
+            InvocationPathsResult::Paths(p) => Ok(p),
+            InvocationPathsResult::OutsideOfRepo(e) | InvocationPathsResult::OtherError(e) => {
+                Err(e.dupe().into())
+            }
+        }
+    }
+
+    pub fn maybe_paths(&self) -> anyhow::Result<Option<&InvocationPaths>> {
+        match &self.paths {
+            InvocationPathsResult::Paths(p) => Ok(Some(p)),
+            InvocationPathsResult::OutsideOfRepo(_) => Ok(None), // commands like log don't need a root but still need to create an invocation record
+            InvocationPathsResult::OtherError(e) => Err(e.dupe().into()),
         }
     }
 

app/buck2_client_ctx/src/subscribers/recorder.rs

@@ -93,7 +93,7 @@ pub(crate) struct InvocationRecorder<'a> {
     isolation_dir: String,
     start_time: Instant,
     async_cleanup_context: AsyncCleanupContext<'a>,
-    build_count_manager: BuildCountManager,
+    build_count_manager: Option<BuildCountManager>,
     trace_id: TraceId,
     command_end: Option<buck2_data::CommandEnd>,
     command_duration: Option<prost_types::Duration>,
@@ -220,7 +220,7 @@ impl<'a> InvocationRecorder<'a> {
         sanitized_argv: Vec<String>,
         trace_id: TraceId,
         isolation_dir: String,
-        build_count_manager: BuildCountManager,
+        build_count_manager: Option<BuildCountManager>,
         filesystem: String,
         restarted_trace_id: Option<TraceId>,
         log_size_counter_bytes: Option<Arc<AtomicU64>>,
@@ -360,7 +360,7 @@ impl<'a> InvocationRecorder<'a> {
         &mut self,
         is_success: bool,
         command_name: &str,
-    ) -> anyhow::Result<BuildCount> {
+    ) -> anyhow::Result<Option<BuildCount>> {
         if let Some(stats) = &self.file_watcher_stats {
             if let Some(merge_base) = &stats.branched_from_revision {
                 match &self.parsed_target_patterns {
@@ -374,11 +374,17 @@ impl<'a> InvocationRecorder<'a> {
                         // fallthrough to 0 below
                     }
                     Some(v) => {
-                        return self
-                            .build_count_manager
-                            .increment(merge_base, v, is_success)
-                            .await
-                            .context("Error recording build count");
+                        return if let Some(build_count) = &self.build_count_manager {
+                            Some(
+                                build_count
+                                    .increment(merge_base, v, is_success)
+                                    .await
+                                    .context("Error recording build count"),
+                            )
+                            .transpose()
+                        } else {
+                            Ok(None)
+                        };
                     }
                 };
             }
@@ -876,7 +882,8 @@ impl<'a> InvocationRecorder<'a> {
                     .build_count(command.is_success, command_data.variant_name())
                     .await
                 {
-                    Ok(build_count) => build_count,
+                    Ok(Some(build_count)) => build_count,
+                    Ok(None) => Default::default(),
                     Err(e) => {
                         let _ignored = soft_error!("build_count_error", e.into());
                         Default::default()
@@ -1709,11 +1716,16 @@ pub(crate) fn try_get_invocation_recorder<'a>(
         .as_ref()
         .map(|path| path.resolve(&ctx.working_dir));
 
+    let paths = ctx.maybe_paths()?;
+
     let filesystem;
     #[cfg(fbcode_build)]
     {
-        let root = std::path::Path::to_owned(ctx.paths()?.project_root().root().to_buf().as_ref());
-        if detect_eden::is_eden(root).unwrap_or(false) {
+        let is_eden = paths.map_or(false, |paths| {
+            let root = std::path::Path::to_owned(paths.project_root().root().to_buf().as_ref());
+            detect_eden::is_eden(root).unwrap_or(false)
+        });
+        if is_eden {
             filesystem = "eden".to_owned();
         } else {
             filesystem = "default".to_owned();
@@ -1724,15 +1736,17 @@ pub(crate) fn try_get_invocation_recorder<'a>(
         filesystem = "default".to_owned();
     }
 
+    let build_count = paths.map(|p| BuildCountManager::new(p.build_count_dir()));
+
     let recorder = InvocationRecorder::new(
         ctx.fbinit(),
         ctx.async_cleanup_context().dupe(),
         write_to_path,
         command_name,
         sanitized_argv,
         ctx.trace_id.dupe(),
-        ctx.paths()?.isolation.as_str().to_owned(),
-        BuildCountManager::new(ctx.paths()?.build_count_dir()),
+        ctx.isolation.to_string(),
+        build_count,
         filesystem,
         ctx.restarted_trace_id.dupe(),
         log_size_counter_bytes,

app/buck2_common/src/invocation_paths_result.rs

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under both the MIT license found in the
+ * LICENSE-MIT file in the root directory of this source tree and the Apache
+ * License, Version 2.0 found in the LICENSE-APACHE file in the root directory
+ * of this source tree.
+ */
+
+use crate::invocation_paths::InvocationPaths;
+
+#[derive(Clone)]
+pub enum InvocationPathsResult {
+    OtherError(buck2_error::Error),
+    Paths(InvocationPaths),
+    OutsideOfRepo(buck2_error::Error), // this error ignored for creating invocation record for log commands
+}
+
+impl InvocationPathsResult {
+    pub fn get_result(self) -> anyhow::Result<InvocationPaths> {
+        match self {
+            InvocationPathsResult::OtherError(e) => Err(e.into()),
+            InvocationPathsResult::Paths(paths) => Ok(paths),
+            InvocationPathsResult::OutsideOfRepo(e) => Err(e.into()),
+        }
+    }
+}

app/buck2_common/src/invocation_roots.rs

@@ -21,7 +21,7 @@ use buck2_core::fs::project::ProjectRoot;
 use once_cell::sync::Lazy;
 
 #[derive(Debug, buck2_error::Error)]
-enum BuckCliError {
+pub enum BuckCliError {
     #[error(
         "Couldn't find a buck project root for directory `{}`. Expected to find a .buckconfig file.", _0.display()
     )]

app/buck2_common/src/lib.rs

@@ -39,6 +39,7 @@ pub mod http;
 pub mod ignores;
 pub mod init;
 pub mod invocation_paths;
+pub mod invocation_paths_result;
 pub mod invocation_roots;
 pub mod io;
 pub mod kill_util;