Skip to content

Commit 8036f28

Browse files
fix(bun): properly handle bun lockfile keys (#10137)
### Description We were not properly handling recursing when traversing the lockfile. Resolving a package would give a `package@version` key instead of the key into the `packages` array. Our `all_dependencies` implementation assumed that we were given the key into `packages`. This is a little odd as the keys can have different "paths" (keys to the packages object) that map to identical entries. This PR adds a check to verify that the entries are in fact the same package by comparing checksums. ### Testing Instructions Fixed unit tests and added one with mismatched checksums. `create-turbo` using bun should no longer display ` WARNING Unable to calculate transitive closures: No lockfile entry found for '@types/[email protected]'`
1 parent 8b66b5b commit 8036f28

File tree

2 files changed

+85
-17
lines changed

2 files changed

+85
-17
lines changed

crates/turborepo-lockfiles/src/bun/de.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,17 @@ impl<'de> Deserialize<'de> for PackageEntry {
5050
.pop_front()
5151
.and_then(val_to_info)
5252
.or_else(|| vals.pop_front().and_then(val_to_info));
53+
let checksum = vals.pop_front().and_then(|val| match val {
54+
Vals::Str(sha) => Some(sha),
55+
Vals::Info(_) => None,
56+
});
5357
Ok(Self {
5458
ident: key,
5559
info,
5660
// The rest are only necessary for serializing a lockfile and aren't needed until adding
5761
// `prune` support
5862
registry: None,
59-
checksum: None,
63+
checksum,
6064
root: None,
6165
})
6266
}
@@ -117,7 +121,7 @@ mod test {
117121
.collect(),
118122
..Default::default()
119123
}),
120-
checksum: None,
124+
checksum: Some("sha".into()),
121125
root: None,
122126
}
123127
);

crates/turborepo-lockfiles/src/bun/mod.rs

+79-15
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::Lockfile;
1313
mod de;
1414
mod id;
1515

16-
type Map<K, V> = std::collections::HashMap<K, V>;
16+
type Map<K, V> = std::collections::BTreeMap<K, V>;
1717

1818
#[derive(Debug, thiserror::Error)]
1919
pub enum Error {
@@ -25,11 +25,23 @@ pub enum Error {
2525
Print(#[from] biome_formatter::PrintError),
2626
#[error("Turborepo cannot serialize Bun lockfiles.")]
2727
NotImplemented,
28+
#[error("{ident} had two entries with differing checksums: {sha1}, {sha2}")]
29+
MismatchedShas {
30+
ident: String,
31+
sha1: String,
32+
sha2: String,
33+
},
34+
}
35+
36+
#[derive(Debug)]
37+
pub struct BunLockfile {
38+
data: BunLockfileData,
39+
key_to_entry: HashMap<String, String>,
2840
}
2941

3042
#[derive(Debug, Deserialize)]
3143
#[serde(rename_all = "camelCase")]
32-
pub struct BunLockfile {
44+
pub struct BunLockfileData {
3345
#[allow(unused)]
3446
lockfile_version: i32,
3547
workspaces: Map<String, WorkspaceEntry>,
@@ -92,6 +104,7 @@ impl Lockfile for BunLockfile {
92104
version: &str,
93105
) -> Result<Option<crate::Package>, crate::Error> {
94106
let workspace_entry = self
107+
.data
95108
.workspaces
96109
.get(workspace_path)
97110
.ok_or_else(|| crate::Error::MissingWorkspace(workspace_path.into()))?;
@@ -100,7 +113,7 @@ impl Lockfile for BunLockfile {
100113
if let Some((_key, entry)) = self.package_entry(&workspace_key) {
101114
let mut version = entry.version().to_string();
102115
// Check for any patches
103-
if let Some(patch) = self.patched_dependencies.get(&entry.ident) {
116+
if let Some(patch) = self.data.patched_dependencies.get(&entry.ident) {
104117
version.push('+');
105118
version.push_str(patch);
106119
}
@@ -130,14 +143,19 @@ impl Lockfile for BunLockfile {
130143
&self,
131144
key: &str,
132145
) -> Result<Option<std::collections::HashMap<String, String>>, crate::Error> {
146+
let entry_key = self
147+
.key_to_entry
148+
.get(key)
149+
.ok_or_else(|| crate::Error::MissingPackage(key.into()))?;
133150
let entry = self
151+
.data
134152
.packages
135-
.get(key)
153+
.get(entry_key)
136154
.ok_or_else(|| crate::Error::MissingPackage(key.into()))?;
137155

138156
let mut deps = HashMap::new();
139157
for (dependency, version) in entry.info.iter().flat_map(|info| info.all_dependencies()) {
140-
let parent_key = format!("{key}/{dependency}");
158+
let parent_key = format!("{entry_key}/{dependency}");
141159
let Some((dep_key, _)) = self.package_entry(&parent_key) else {
142160
return Err(crate::Error::MissingPackage(dependency.to_string()));
143161
};
@@ -173,7 +191,7 @@ impl Lockfile for BunLockfile {
173191
}
174192

175193
fn human_name(&self, package: &crate::Package) -> Option<String> {
176-
let entry = self.packages.get(&package.key)?;
194+
let entry = self.data.packages.get(&package.key)?;
177195
Some(entry.ident.clone())
178196
}
179197
}
@@ -188,7 +206,7 @@ impl BunLockfile {
188206
// present in the lockfile
189207
fn package_entry(&self, key: &str) -> Option<(&str, &PackageEntry)> {
190208
let (key, entry) =
191-
PossibleKeyIter::new(key).find_map(|k| self.packages.get_key_value(k))?;
209+
PossibleKeyIter::new(key).find_map(|k| self.data.packages.get_key_value(k))?;
192210
Some((key, entry))
193211
}
194212
}
@@ -217,8 +235,25 @@ impl FromStr for BunLockfile {
217235
)
218236
.map_err(Error::from)?;
219237
let strict_json = format.print().map_err(Error::from)?;
220-
let this = serde_json::from_str(strict_json.as_code())?;
221-
Ok(this)
238+
let data: BunLockfileData = serde_json::from_str(strict_json.as_code())?;
239+
let mut key_to_entry = HashMap::with_capacity(data.packages.len());
240+
for (path, info) in data.packages.iter() {
241+
if let Some(prev_path) = key_to_entry.insert(info.ident.clone(), path.clone()) {
242+
let prev_info = data
243+
.packages
244+
.get(&prev_path)
245+
.expect("we just got this path from the packages list");
246+
if prev_info.checksum != info.checksum {
247+
return Err(Error::MismatchedShas {
248+
ident: info.ident.clone(),
249+
sha1: prev_info.checksum.clone().unwrap_or_default(),
250+
sha2: info.checksum.clone().unwrap_or_default(),
251+
}
252+
.into());
253+
}
254+
}
255+
}
256+
Ok(Self { data, key_to_entry })
222257
}
223258
}
224259
impl PackageEntry {
@@ -248,6 +283,7 @@ impl PackageInfo {
248283
#[cfg(test)]
249284
mod test {
250285
use pretty_assertions::assert_eq;
286+
use serde_json::json;
251287
use test_case::test_case;
252288

253289
use super::*;
@@ -282,17 +318,20 @@ mod test {
282318
"validate-npm-package-name",
283319
]
284320
.as_slice();
321+
// Both @turbo/gen and log-symbols depend on the same version of chalk
322+
// log-symbols version wins out, but this is okay since they are the same exact
323+
// version of chalk.
285324
const TURBO_GEN_CHALK_DEPS: &[&str] = [
286-
"@turbo/gen/chalk/ansi-styles",
287-
"@turbo/gen/chalk/escape-string-regexp",
288-
"@turbo/gen/chalk/supports-color",
325+
"log-symbols/chalk/ansi-styles",
326+
"log-symbols/chalk/escape-string-regexp",
327+
"log-symbols/chalk/supports-color",
289328
]
290329
.as_slice();
291330
const CHALK_DEPS: &[&str] = ["ansi-styles", "supports-color"].as_slice();
292331

293-
#[test_case("@turbo/gen", TURBO_GEN_DEPS)]
294-
#[test_case("@turbo/gen/chalk", TURBO_GEN_CHALK_DEPS)]
295-
#[test_case("chalk", CHALK_DEPS)]
332+
#[test_case("@turbo/gen@1.13.4", TURBO_GEN_DEPS)]
333+
#[test_case("chalk@2.4.2", TURBO_GEN_CHALK_DEPS)]
334+
#[test_case("chalk@4.1.2", CHALK_DEPS)]
296335
fn test_all_dependencies(key: &str, expected: &[&str]) {
297336
let lockfile = BunLockfile::from_str(BASIC_LOCKFILE).unwrap();
298337
let mut expected = expected.to_vec();
@@ -320,4 +359,29 @@ mod test {
320359
crate::Package::new("[email protected]", "3.0.0+patches/[email protected]")
321360
);
322361
}
362+
363+
#[test]
364+
fn test_failure_if_mismatched_keys() {
365+
let contents = serde_json::to_string(&json!({
366+
"lockfileVersion": 0,
367+
"workspaces": {
368+
"": {
369+
"name": "test",
370+
"dependencies": {
371+
"foo": "^1.0.0",
372+
"bar": "^1.0.0",
373+
}
374+
}
375+
},
376+
"packages": {
377+
"bar": ["[email protected]", { "dependencies": { "shared": "^1.0.0" } }, "sha512-goodbye"],
378+
"bar/shared": ["[email protected]", {}, "sha512-bar"],
379+
"foo": ["[email protected]", { "dependencies": { "shared": "^1.0.0" } }, "sha512-hello"],
380+
"foo/shared": ["[email protected]", { }, "sha512-foo"],
381+
}
382+
}))
383+
.unwrap();
384+
let lockfile = BunLockfile::from_str(&contents);
385+
assert!(lockfile.is_err(), "matching packages have differing shas");
386+
}
323387
}

0 commit comments

Comments
 (0)