diff --git a/.bazelrc b/.bazelrc index 6b8968e35..1965b666a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -45,7 +45,7 @@ build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect # TODO(aaronmondal): Extend these flags until we can run with clippy::pedantic. -build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use +build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms build --@rules_rust//:clippy.toml=//:clippy.toml test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml diff --git a/nativelink-metric-collector/src/metrics_visitors.rs b/nativelink-metric-collector/src/metrics_visitors.rs index 48254af49..d826d7428 100644 --- a/nativelink-metric-collector/src/metrics_visitors.rs +++ b/nativelink-metric-collector/src/metrics_visitors.rs @@ -33,8 +33,9 @@ impl From for CollectionKind { fn from(kind: MetricKind) -> Self { match kind { MetricKind::Counter => CollectionKind::Counter, - MetricKind::String => CollectionKind::String, - _ => CollectionKind::String, + MetricKind::Default | MetricKind::String | MetricKind::Component => { + CollectionKind::String + } } } } diff --git a/nativelink-metric/src/lib.rs b/nativelink-metric/src/lib.rs index cf0695168..423e2cd91 100644 --- a/nativelink-metric/src/lib.rs +++ b/nativelink-metric/src/lib.rs @@ -68,11 +68,10 @@ pub enum MetricKind { impl From for MetricKind { fn from(value: u64) -> Self { match value { - 0 => MetricKind::Default, + 0 | 4_u64..=u64::MAX => MetricKind::Default, 1 => MetricKind::Counter, 2 => MetricKind::String, 3 => MetricKind::Component, - _ => MetricKind::Default, } } } diff --git a/nativelink-scheduler/src/memory_awaited_action_db.rs b/nativelink-scheduler/src/memory_awaited_action_db.rs index 8e0102ba4..bd33bad3b 100644 --- a/nativelink-scheduler/src/memory_awaited_action_db.rs +++ b/nativelink-scheduler/src/memory_awaited_action_db.rs @@ -239,8 +239,7 @@ impl SortedAwaitedActions { ActionStage::CacheCheck => &mut self.cache_check, ActionStage::Queued => &mut self.queued, ActionStage::Executing => &mut self.executing, - ActionStage::Completed(_) => &mut self.completed, - ActionStage::CompletedFromCache(_) => &mut self.completed, + ActionStage::Completed(_) | ActionStage::CompletedFromCache(_) => &mut self.completed, } } diff --git a/nativelink-scheduler/src/simple_scheduler_state_manager.rs b/nativelink-scheduler/src/simple_scheduler_state_manager.rs index 1d30c9766..abcad0aba 100644 --- a/nativelink-scheduler/src/simple_scheduler_state_manager.rs +++ b/nativelink-scheduler/src/simple_scheduler_state_manager.rs @@ -393,8 +393,9 @@ where ActionStage::CacheCheck => OperationStageFlags::CacheCheck, ActionStage::Queued => OperationStageFlags::Queued, ActionStage::Executing => OperationStageFlags::Executing, - ActionStage::Completed(_) => OperationStageFlags::Completed, - ActionStage::CompletedFromCache(_) => OperationStageFlags::Completed, + ActionStage::Completed(_) | ActionStage::CompletedFromCache(_) => { + OperationStageFlags::Completed + } }; if !filter.stages.intersects(stage_flag) { return false; diff --git a/nativelink-scheduler/src/store_awaited_action_db.rs b/nativelink-scheduler/src/store_awaited_action_db.rs index 296d8a5c2..f98a4b549 100644 --- a/nativelink-scheduler/src/store_awaited_action_db.rs +++ b/nativelink-scheduler/src/store_awaited_action_db.rs @@ -235,10 +235,7 @@ const CLIENT_ID_TO_OPERATION_ID_KEY_PREFIX: &str = "cid_"; struct OperationIdToAwaitedAction<'a>(Cow<'a, OperationId>); impl OperationIdToAwaitedAction<'_> { fn borrow(&self) -> OperationIdToAwaitedAction<'_> { - match self.0 { - Cow::Borrowed(operation_id) => OperationIdToAwaitedAction(Cow::Borrowed(operation_id)), - Cow::Owned(ref operation_id) => OperationIdToAwaitedAction(Cow::Borrowed(operation_id)), - } + OperationIdToAwaitedAction(Cow::Borrowed(self.0.as_ref())) } } impl SchedulerStoreKeyProvider for OperationIdToAwaitedAction<'_> { diff --git a/nativelink-store/src/compression_store.rs b/nativelink-store/src/compression_store.rs index 1e3038228..dd50ce7ea 100644 --- a/nativelink-store/src/compression_store.rs +++ b/nativelink-store/src/compression_store.rs @@ -159,8 +159,7 @@ struct UploadState { impl UploadState { pub fn new(store: &CompressionStore, upload_size: UploadSizeInfo) -> Result { let input_max_size = match upload_size { - UploadSizeInfo::ExactSize(sz) => sz, - UploadSizeInfo::MaxSize(sz) => sz, + UploadSizeInfo::MaxSize(sz) | UploadSizeInfo::ExactSize(sz) => sz, }; let max_index_count = (input_max_size / u64::from(store.config.block_size)) + 1; diff --git a/nativelink-util/src/action_messages.rs b/nativelink-util/src/action_messages.rs index 6ec09f81a..75c99e700 100644 --- a/nativelink-util/src/action_messages.rs +++ b/nativelink-util/src/action_messages.rs @@ -215,24 +215,21 @@ impl ActionUniqueQualifier { /// Get the instance_name of the action. pub const fn instance_name(&self) -> &String { match self { - Self::Cachable(action) => &action.instance_name, - Self::Uncachable(action) => &action.instance_name, + Self::Cachable(action) | Self::Uncachable(action) => &action.instance_name, } } /// Get the digest function of the action. pub const fn digest_function(&self) -> DigestHasherFunc { match self { - Self::Cachable(action) => action.digest_function, - Self::Uncachable(action) => action.digest_function, + Self::Cachable(action) | Self::Uncachable(action) => action.digest_function, } } /// Get the digest of the action. pub const fn digest(&self) -> DigestInfo { match self { - Self::Cachable(action) => action.digest, - Self::Uncachable(action) => action.digest, + Self::Cachable(action) | Self::Uncachable(action) => action.digest, } } } diff --git a/nativelink-util/src/platform_properties.rs b/nativelink-util/src/platform_properties.rs index 3bcf998bf..0a223974b 100644 --- a/nativelink-util/src/platform_properties.rs +++ b/nativelink-util/src/platform_properties.rs @@ -114,10 +114,10 @@ impl PlatformPropertyValue { pub fn as_str(&self) -> Cow { match self { - Self::Exact(value) => Cow::Borrowed(value), + Self::Exact(value) | Self::Priority(value) | Self::Unknown(value) => { + Cow::Borrowed(value) + } Self::Minimum(value) => Cow::Owned(value.to_string()), - Self::Priority(value) => Cow::Borrowed(value), - Self::Unknown(value) => Cow::Borrowed(value), } } } diff --git a/nativelink-util/src/retry.rs b/nativelink-util/src/retry.rs index 8c4962236..37674c7ce 100644 --- a/nativelink-util/src/retry.rs +++ b/nativelink-util/src/retry.rs @@ -62,7 +62,6 @@ pub struct Retrier { fn to_error_code(code: Code) -> ErrorCode { match code { Code::Cancelled => ErrorCode::Cancelled, - Code::Unknown => ErrorCode::Unknown, Code::InvalidArgument => ErrorCode::InvalidArgument, Code::DeadlineExceeded => ErrorCode::DeadlineExceeded, Code::NotFound => ErrorCode::NotFound, @@ -99,22 +98,24 @@ impl Retrier { retry_codes.contains(&to_error_code(code)) } else { match code { - Code::InvalidArgument => false, - Code::FailedPrecondition => false, - Code::OutOfRange => false, - Code::Unimplemented => false, - Code::NotFound => false, - Code::AlreadyExists => false, - Code::PermissionDenied => false, - Code::Unauthenticated => false, - Code::Cancelled => true, - Code::Unknown => true, - Code::DeadlineExceeded => true, - Code::ResourceExhausted => true, - Code::Aborted => true, - Code::Internal => true, - Code::Unavailable => true, - Code::DataLoss => true, + // TODO(SchahinRohani): Handle all cases properly so there is no need for a wildcard match + // All cases where a retry should happen are commented out here and replaced with a wildcard match. + Code::InvalidArgument + | Code::FailedPrecondition + | Code::OutOfRange + | Code::Unimplemented + | Code::NotFound + | Code::AlreadyExists + | Code::PermissionDenied + | Code::Unauthenticated => false, + // Code::Cancelled + // | Code::Unknown + // | Code::DeadlineExceeded + // | Code::ResourceExhausted + // | Code::Aborted + // | Code::Internal + // | Code::Unavailable + // | Code::DataLoss => true, _ => true, } }