From 11b549369e6d9c08a3afeee1c0b44dd990fd1ea2 Mon Sep 17 00:00:00 2001 From: Alexander Koz Date: Sat, 6 Jul 2024 01:01:01 +0400 Subject: [PATCH 1/3] Support target-spec json file extension in various cases (case-insensitive) --- src/cargo/core/compiler/compile_kind.rs | 31 +++++++++++++++++++------ src/cargo/util/context/mod.rs | 3 ++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index 222732ddebc..83a0e182115 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -131,7 +131,7 @@ impl CompileTarget { if name.is_empty() { anyhow::bail!("target was empty"); } - if !name.ends_with(".json") { + if !Self::name_ends_with_json(name) { return Ok(CompileTarget { name: name.into() }); } @@ -170,7 +170,7 @@ impl CompileTarget { // name without ".json") as a short name for this target. Note that the // `unwrap()` here should never trigger since we have a nonempty name // and it starts as utf-8 so it's always utf-8 - if self.name.ends_with(".json") { + if self.is_json_file() { Path::new(&self.name).file_stem().unwrap().to_str().unwrap() } else { &self.name @@ -180,11 +180,7 @@ impl CompileTarget { /// See [`CompileKind::fingerprint_hash`]. pub fn fingerprint_hash(&self) -> u64 { let mut hasher = StableHasher::new(); - match self - .name - .ends_with(".json") - .then(|| fs::read_to_string(self.name)) - { + match self.is_json_file().then(|| fs::read_to_string(self.name)) { Some(Ok(contents)) => { // This may have some performance concerns, since it is called // fairly often. If that ever seems worth fixing, consider @@ -197,4 +193,25 @@ impl CompileTarget { } hasher.finish() } + + /// Checks if name of `self` contains a filename with json ext. + /// Does not check if the path exists. + /// + /// Same as [`Self::name_ends_with_json`], use it to check strings if create `CompileTarget` isn't a way. + #[inline] + pub fn is_json_file(&self) -> bool { + Self::name_ends_with_json(self.name) + } + + /// Helper function to check if the `name` ends with ".json" (case-insensitive). + /// Does not check if the path exists. + pub fn name_ends_with_json>(name: S) -> bool { + const EXT: &'static str = ".json"; + name.as_ref().ends_with(EXT) || { + let s = name.as_ref(); + s.get((s.len() - 5)..) + .map(|ext| ext.eq_ignore_ascii_case(EXT)) + .unwrap_or_default() + } + } } diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index f275c23877a..862e6598537 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -68,6 +68,7 @@ use std::time::Instant; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; +use crate::core::compiler::CompileTarget; use crate::core::global_cache_tracker::{DeferredGlobalLastUse, GlobalCacheTracker}; use crate::core::shell::Verbosity; use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRootConfig}; @@ -2651,7 +2652,7 @@ impl BuildTargetConfig { /// Gets values of `build.target` as a list of strings. pub fn values(&self, gctx: &GlobalContext) -> CargoResult> { let map = |s: &String| { - if s.ends_with(".json") { + if CompileTarget::name_ends_with_json(s) { // Path to a target specification file (in JSON). // self.inner From 2a24eb4f57087e186fb24a8e3e25dfe9c1399a5e Mon Sep 17 00:00:00 2001 From: Alexander Koz Date: Sat, 6 Jul 2024 02:36:45 +0400 Subject: [PATCH 2/3] fix negative range (possible out of bounds) --- src/cargo/core/compiler/compile_kind.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index 83a0e182115..c93d523dd96 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -209,7 +209,7 @@ impl CompileTarget { const EXT: &'static str = ".json"; name.as_ref().ends_with(EXT) || { let s = name.as_ref(); - s.get((s.len() - 5)..) + s.get(s.len().wrapping_sub(EXT.len())..) .map(|ext| ext.eq_ignore_ascii_case(EXT)) .unwrap_or_default() } From 03f6d29685bbbcfe8c68ac8b4df12cbf61f32b20 Mon Sep 17 00:00:00 2001 From: Alexander Koz Date: Sat, 6 Jul 2024 22:32:13 +0400 Subject: [PATCH 3/3] improvements following review of relevant PR; also implemented that all as trait and share for compat usage, including outer usage of the crate. --- src/cargo/core/compiler/compile_kind.rs | 67 +++++++++++++++++++------ src/cargo/core/compiler/mod.rs | 2 +- src/cargo/util/context/mod.rs | 4 +- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index c93d523dd96..a2e276a98f3 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -7,6 +7,7 @@ use crate::util::{try_canonicalize, GlobalContext, StableHasher}; use anyhow::Context as _; use serde::Serialize; use std::collections::BTreeSet; +use std::ffi::OsStr; use std::fs; use std::hash::{Hash, Hasher}; use std::path::Path; @@ -131,7 +132,7 @@ impl CompileTarget { if name.is_empty() { anyhow::bail!("target was empty"); } - if !Self::name_ends_with_json(name) { + if !name.has_json_ext_ignore_case() { return Ok(CompileTarget { name: name.into() }); } @@ -170,7 +171,7 @@ impl CompileTarget { // name without ".json") as a short name for this target. Note that the // `unwrap()` here should never trigger since we have a nonempty name // and it starts as utf-8 so it's always utf-8 - if self.is_json_file() { + if self.has_json_ext_ignore_case() { Path::new(&self.name).file_stem().unwrap().to_str().unwrap() } else { &self.name @@ -180,7 +181,10 @@ impl CompileTarget { /// See [`CompileKind::fingerprint_hash`]. pub fn fingerprint_hash(&self) -> u64 { let mut hasher = StableHasher::new(); - match self.is_json_file().then(|| fs::read_to_string(self.name)) { + match self + .has_json_ext_ignore_case() + .then(|| fs::read_to_string(self.name)) + { Some(Ok(contents)) => { // This may have some performance concerns, since it is called // fairly often. If that ever seems worth fixing, consider @@ -193,25 +197,56 @@ impl CompileTarget { } hasher.finish() } +} - /// Checks if name of `self` contains a filename with json ext. +pub trait HasExtIgnoreCase { + /// Checks if the `self` ends with `ext` __case-insensitive__. + /// The `ext` should not have leading dot. + /// /// Does not check if the path exists. /// - /// Same as [`Self::name_ends_with_json`], use it to check strings if create `CompileTarget` isn't a way. + /// Use it only for user's input like target names, + /// but not in algorithms like filesystem traversal because on case-sensitive fs it may lead logical errors + /// if you'll have met two files "a.foo" and "a.FOO", for example. + fn has_ext_ignore_case>(&self, ext: Ext) -> bool; + + /// Checks if the `self` ends with ".json" (case-insensitive). + /// + /// See [`has_ext_ignore_case`][HasExtIgnoreCase::has_ext_ignore_case], + /// used as default implementation. #[inline] - pub fn is_json_file(&self) -> bool { - Self::name_ends_with_json(self.name) + fn has_json_ext_ignore_case(&self) -> bool { + self.has_ext_ignore_case("json") } +} - /// Helper function to check if the `name` ends with ".json" (case-insensitive). - /// Does not check if the path exists. - pub fn name_ends_with_json>(name: S) -> bool { - const EXT: &'static str = ".json"; - name.as_ref().ends_with(EXT) || { - let s = name.as_ref(); - s.get(s.len().wrapping_sub(EXT.len())..) - .map(|ext| ext.eq_ignore_ascii_case(EXT)) - .unwrap_or_default() +impl> HasExtIgnoreCase for T { + fn has_ext_ignore_case>(&self, ext: Ext) -> bool { + Path::new(self) + .extension() + .is_some_and(|s| s.eq_ignore_ascii_case(ext)) + } +} + +impl HasExtIgnoreCase for CompileTarget { + /// Checks if the name of `self` ends with `ext` __case-insensitive__. + /// The `ext` should not have leading dot. + /// + /// See [`has_ext_ignore_case`][HasExtIgnoreCase::has_ext_ignore_case], + fn has_ext_ignore_case>(&self, ext: Ext) -> bool { + self.name.has_ext_ignore_case(ext) + } +} + +impl HasExtIgnoreCase for CompileKind { + /// Checks if the `self`'s target name ends with `ext` __case-insensitive__. + /// The `ext` should not have leading dot. + /// + /// See [`has_ext_ignore_case`][HasExtIgnoreCase::has_ext_ignore_case], + fn has_ext_ignore_case>(&self, ext: Ext) -> bool { + match self { + CompileKind::Host => false, + CompileKind::Target(target) => target.has_ext_ignore_case(ext), } } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 70dfb3085d3..ef6967c3e3a 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -74,7 +74,7 @@ pub use self::build_context::{ use self::build_plan::BuildPlan; pub use self::build_runner::{BuildRunner, Metadata}; pub use self::compilation::{Compilation, Doctest, UnitOutput}; -pub use self::compile_kind::{CompileKind, CompileTarget}; +pub use self::compile_kind::{CompileKind, CompileTarget, HasExtIgnoreCase}; pub use self::crate_type::CrateType; pub use self::custom_build::LinkArgTarget; pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts}; diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 862e6598537..996ba4d9302 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -68,7 +68,7 @@ use std::time::Instant; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; -use crate::core::compiler::CompileTarget; +use crate::core::compiler::HasExtIgnoreCase as _; use crate::core::global_cache_tracker::{DeferredGlobalLastUse, GlobalCacheTracker}; use crate::core::shell::Verbosity; use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRootConfig}; @@ -2652,7 +2652,7 @@ impl BuildTargetConfig { /// Gets values of `build.target` as a list of strings. pub fn values(&self, gctx: &GlobalContext) -> CargoResult> { let map = |s: &String| { - if CompileTarget::name_ends_with_json(s) { + if s.has_json_ext_ignore_case() { // Path to a target specification file (in JSON). // self.inner