diff --git a/Cargo.lock b/Cargo.lock index e90b8466f0c82..dcd0c6920a17a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6434,9 +6434,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "serde" -version = "1.0.189" +version = "1.0.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e422a44e74ad4001bdc8eede9a4570ab52f71190e9c076d14369f38b9200537" +checksum = "91d3c334ca1ee894a2c6f6ad698fe8c435b76d504b13d436f0685d648d6d96f7" dependencies = [ "serde_derive", ] @@ -6473,9 +6473,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.189" +version = "1.0.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e48d1f918009ce3145511378cf68d613e3b3d9137d67272562080d68a2b32d5" +checksum = "67c5609f394e5c2bd7fc51efda478004ea80ef42fee983d5c67a65e34f32c0e3" dependencies = [ "proc-macro2", "quote", @@ -6596,11 +6596,11 @@ dependencies = [ [[package]] name = "serde_yaml" -version = "0.9.21" +version = "0.9.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9d684e3ec7de3bf5466b32bd75303ac16f0736426e5a4e0d6e489559ce1249c" +checksum = "3cc7a1570e38322cfe4154732e5110f887ea57e22b76f4bfd32b5bdd3368666c" dependencies = [ - "indexmap 1.9.3", + "indexmap 2.0.0", "itoa", "ryu", "serde", @@ -9436,7 +9436,7 @@ dependencies = [ "pretty_assertions", "serde", "serde_json", - "serde_yaml 0.9.21", + "serde_yaml 0.9.27", "tiny-gradient", "tokio-util", "tracing", @@ -10522,7 +10522,7 @@ dependencies = [ "semver 1.0.18", "serde", "serde_json", - "serde_yaml 0.9.21", + "serde_yaml 0.9.27", "sha2", "shared_child", "svix-ksuid", @@ -10582,7 +10582,7 @@ dependencies = [ "semver 1.0.18", "serde", "serde_json", - "serde_yaml 0.9.21", + "serde_yaml 0.9.27", "test-case", "thiserror", "turbopath", @@ -10613,7 +10613,7 @@ dependencies = [ "regex", "serde", "serde_json", - "serde_yaml 0.9.21", + "serde_yaml 0.9.27", "tempfile", "test-case", "thiserror", diff --git a/crates/turborepo-lockfiles/Cargo.toml b/crates/turborepo-lockfiles/Cargo.toml index 46778bc2b20da..366fe2b70d59d 100644 --- a/crates/turborepo-lockfiles/Cargo.toml +++ b/crates/turborepo-lockfiles/Cargo.toml @@ -14,7 +14,7 @@ regex = "1" semver = "1.0.17" serde = { version = "1.0.126", features = ["derive", "rc"] } serde_json = "1.0.86" -serde_yaml = "0.9" +serde_yaml = "0.9.27" thiserror = "1.0.38" turbopath = { path = "../turborepo-paths" } diff --git a/crates/turborepo-lockfiles/fixtures/berry_semver.lock b/crates/turborepo-lockfiles/fixtures/berry_semver.lock new file mode 100644 index 0000000000000..0d3e01c24bf25 --- /dev/null +++ b/crates/turborepo-lockfiles/fixtures/berry_semver.lock @@ -0,0 +1,31 @@ +# This file is generated by running "yarn install" inside your project. +# Manual changes might be lost - proceed with caution! + +__metadata: + version: 6 + cacheKey: 8 + +"file-source@npm:2": + version: 2.6.1 + resolution: "file-source@npm:2.6.1" + dependencies: + stream-source: 0.10 + checksum: db27232df214b27ddd7026bbd202caf0bde31123811ebac0a9772d6a41735efdc3ee8050661f56d02d43811bde79938adb5ed8cbfe5f813c7cd09b86d19b6c84 + languageName: node + linkType: hard + +"foo@workspace:packages/foo": + version: 0.0.0-use.local + resolution: "foo@workspace:packages/foo" + dependencies: + file-source: 2 + languageName: unknown + linkType: soft + +"stream-source@npm:0.10": + version: 0.10.12 + resolution: "stream-source@npm:0.10.12" + checksum: 65a0e0a38014dcfa6e219e92d83bde5fe716cf16df339e9d7e951525c0d129771416de4d77c3389c38cdd20a43d9575f90cc91041d6fa0c2f053b989780e3591 + languageName: node + linkType: hard + diff --git a/crates/turborepo-lockfiles/src/berry/de.rs b/crates/turborepo-lockfiles/src/berry/de.rs index dd1004090bdc8..6cf2504517669 100644 --- a/crates/turborepo-lockfiles/src/berry/de.rs +++ b/crates/turborepo-lockfiles/src/berry/de.rs @@ -1,63 +1,150 @@ -use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; -// Newtype used for correct deserialization -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Default, Clone)] -pub struct SemverString(pub String); +use serde::Deserialize; -impl From for String { - fn from(value: SemverString) -> Self { - value.0 - } +use super::{BerryPackage, DependencyMeta, LockfileData, Metadata}; + +const METADATA_KEY: &str = "__metadata"; + +/// Union type of yarn.lock metadata entry and package entries. +/// Only as a workaround for serde_yaml behavior around parsing numbers as +/// strings. +// In the ideal world this would be an enum, but serde_yaml currently has behavior +// where using `#[serde(untagged)]` or `#[serde(flatten)]` affects how it handles +// YAML numbers being parsed as Strings. +// If these macros are present, then it will refuse to parse 1 or 1.0 as a String +// and will instead only parse them as an int/float respectively. +// If these macros aren't present, then it will happily parse 1 or 1.0 as +// "1" and "1.0". +#[derive(Debug, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub(crate) struct Entry { + version: String, + language_name: Option, + dependencies: Option>, + peer_dependencies: Option>, + dependencies_meta: Option>, + peer_dependencies_meta: Option>, + bin: Option>, + link_type: Option, + resolution: Option, + checksum: Option, + conditions: Option, + cache_key: Option, } -impl AsRef for SemverString { - fn as_ref(&self) -> &str { - self.0.as_str() - } +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("missing resolution for entry {0}")] + MissingResolution(String), + #[error("multiple entry {0} has fields that should only appear in metadata")] + InvalidMetadataFields(String), + #[error("lockfile missing {METADATA_KEY} entry")] + MissingMetadata, } -impl<'de> Deserialize<'de> for SemverString { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - // We use this to massage numerical semver versions to strings - // e.g. 2 - #[derive(Deserialize)] - #[serde(untagged)] - enum StringOrNum { - String(String), - Int(u64), - Float(f32), - } +impl TryFrom> for LockfileData { + type Error = Error; - match StringOrNum::deserialize(deserializer)? { - StringOrNum::String(s) => Ok(SemverString(s)), - StringOrNum::Int(x) => Ok(SemverString(x.to_string())), - StringOrNum::Float(f) => Ok(SemverString(f.to_string())), + fn try_from(mut value: BTreeMap) -> Result { + let Entry { + version, cache_key, .. + } = value.remove(METADATA_KEY).ok_or(Error::MissingMetadata)?; + let metadata = Metadata { version, cache_key }; + let mut packages = BTreeMap::new(); + for (key, entry) in value { + let Entry { + version, + language_name, + dependencies, + peer_dependencies, + dependencies_meta, + peer_dependencies_meta, + bin, + link_type, + resolution, + checksum, + conditions, + cache_key, + } = entry; + if cache_key.is_some() { + return Err(Error::InvalidMetadataFields(key)); + } + let resolution = resolution.ok_or_else(|| Error::MissingResolution(key.clone()))?; + packages.insert( + key, + BerryPackage { + version, + language_name, + dependencies, + peer_dependencies, + dependencies_meta, + peer_dependencies_meta, + bin, + link_type, + resolution, + checksum, + conditions, + }, + ); } + + Ok(LockfileData { metadata, packages }) } } #[cfg(test)] mod test { - use std::collections::HashMap; - use super::*; #[test] - fn test_semver() { - let input = "foo: 1.2.3 -bar: 2 -baz: latest -oop: 1.3 -"; - - let result: HashMap = serde_yaml::from_str(input).unwrap(); - - assert_eq!(result["foo"].as_ref(), "1.2.3"); - assert_eq!(result["bar"].as_ref(), "2"); - assert_eq!(result["baz"].as_ref(), "latest"); - assert_eq!(result["oop"].as_ref(), "1.3") + fn test_requires_metadata() { + let data = BTreeMap::new(); + assert!(LockfileData::try_from(data).is_err()); + } + + #[test] + fn test_rejects_cache_key_in_packages() { + let mut data = BTreeMap::new(); + data.insert( + METADATA_KEY.to_string(), + Entry { + version: "1".into(), + cache_key: Some("8".into()), + ..Default::default() + }, + ); + data.insert( + "foo".to_string(), + Entry { + version: "1".into(), + resolution: Some("resolved".into()), + cache_key: Some("8".into()), + ..Default::default() + }, + ); + assert!(LockfileData::try_from(data).is_err()); + } + + #[test] + fn test_requires_resolution() { + let mut data = BTreeMap::new(); + data.insert( + METADATA_KEY.to_string(), + Entry { + version: "1".into(), + cache_key: Some("8".into()), + ..Default::default() + }, + ); + data.insert( + "foo".to_string(), + Entry { + version: "1".into(), + resolution: None, + ..Default::default() + }, + ); + assert!(LockfileData::try_from(data).is_err()); } } diff --git a/crates/turborepo-lockfiles/src/berry/mod.rs b/crates/turborepo-lockfiles/src/berry/mod.rs index c1d28377230ad..c2d83840a8521 100644 --- a/crates/turborepo-lockfiles/src/berry/mod.rs +++ b/crates/turborepo-lockfiles/src/berry/mod.rs @@ -10,10 +10,10 @@ use std::{ iter, }; -use de::SemverString; +use de::Entry; use identifiers::{Descriptor, Locator}; use protocol_resolver::DescriptorResolver; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use thiserror::Error; use turbopath::RelativeUnixPathBuf; @@ -55,45 +55,42 @@ pub struct BerryLockfile { // This is the direct representation of the lockfile as it appears on disk. // More internal tracking is required for effectively altering the lockfile -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize)] +#[serde(try_from = "Map")] pub struct LockfileData { - #[serde(rename = "__metadata")] metadata: Metadata, - #[serde(flatten)] packages: Map, } -#[derive(Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Clone)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Clone)] struct Metadata { - version: u64, + version: String, cache_key: Option, } -#[derive(Debug, Deserialize, PartialEq, Eq, Serialize, Default, Clone)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, PartialEq, Eq, Default, Clone)] struct BerryPackage { - version: SemverString, + version: String, language_name: Option, - dependencies: Option>, - peer_dependencies: Option>, + dependencies: Option>, + peer_dependencies: Option>, dependencies_meta: Option>, peer_dependencies_meta: Option>, // Structured metadata we need to persist - bin: Option>, + bin: Option>, link_type: Option, resolution: String, checksum: Option, conditions: Option, } -#[derive(Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Clone, Copy)] +#[derive(Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Clone, Copy)] struct DependencyMeta { optional: Option, unplugged: Option, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Deserialize)] pub struct BerryManifest { resolutions: Option>, } @@ -397,7 +394,7 @@ impl Lockfile for BerryLockfile { Ok(Some(crate::Package { key: locator.to_string(), - version: package.version.clone().into(), + version: package.version.clone(), })) } @@ -619,10 +616,35 @@ mod test { fn test_deserialize_lockfile() { let lockfile: LockfileData = LockfileData::from_bytes(include_bytes!("../../fixtures/berry.lock")).unwrap(); - assert_eq!(lockfile.metadata.version, 6); + assert_eq!(lockfile.metadata.version, "6"); assert_eq!(lockfile.metadata.cache_key.as_deref(), Some("8c0")); } + #[test] + fn test_problematic_semver() { + let lockfile = + LockfileData::from_bytes(include_bytes!("../../fixtures/berry_semver.lock")).unwrap(); + assert_eq!(lockfile.metadata.version, "6"); + assert_eq!(lockfile.metadata.cache_key.as_deref(), Some("8")); + assert_eq!(lockfile.packages.len(), 3); + assert_eq!( + lockfile + .packages + .get("file-source@npm:2") + .and_then(|pkg| pkg.dependencies.as_ref()) + .and_then(|deps| deps.get("stream-source")), + Some(&"0.10".to_string()) + ); + assert_eq!( + lockfile + .packages + .get("foo@workspace:packages/foo") + .and_then(|pkg| pkg.dependencies.as_ref()) + .and_then(|deps| deps.get("file-source")), + Some(&"2".to_string()) + ); + } + #[test] fn test_roundtrip() { let contents = include_str!("../../fixtures/berry.lock"); @@ -720,7 +742,7 @@ mod test { let patch = lockfile.patches.get(&locator).unwrap(); let package = lockfile.locator_package.get(patch).unwrap(); - assert_eq!(package.version.as_ref(), "2.0.0-next.4"); + assert_eq!(package.version, "2.0.0-next.4"); assert_eq!( lockfile.patches().unwrap(), diff --git a/crates/turborepo-lockfiles/src/berry/ser.rs b/crates/turborepo-lockfiles/src/berry/ser.rs index 57b094b429a1f..20a15c6ecb545 100644 --- a/crates/turborepo-lockfiles/src/berry/ser.rs +++ b/crates/turborepo-lockfiles/src/berry/ser.rs @@ -190,12 +190,11 @@ mod test { use pretty_assertions::assert_eq; use super::*; - use crate::berry::SemverString; #[test] fn test_metadata_display() { let metadata = Metadata { - version: 6, + version: "6".into(), cache_key: Some("8c0".to_string()), }; assert_eq!( @@ -224,13 +223,13 @@ mod test { let long_key = "a".repeat(1025); let lockfile = LockfileData { metadata: Metadata { - version: 6, + version: "6".into(), cache_key: Some("8".into()), }, packages: [( long_key.clone(), BerryPackage { - version: SemverString("1.2.3".to_string()), + version: "1.2.3".to_string(), ..Default::default() }, )]