Skip to content

Commit

Permalink
feat: factor engines into global cache key (#8173)
Browse files Browse the repository at this point in the history
### Description

Changing the `engines` field in the root `package.json` can affect the
execution of tasks so it should affect the global cache.

This PR adds the `engines` struct to the global cache key.

### Testing Instructions

Added new integration test verifying that changing `engines` results in
a cache miss.
  • Loading branch information
chris-olszewski committed May 31, 2024
1 parent a31da1b commit 0d5b628
Show file tree
Hide file tree
Showing 53 changed files with 314 additions and 230 deletions.
22 changes: 21 additions & 1 deletion crates/turborepo-lib/src/hash/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub struct GlobalHashable<'a> {
// These are None in single package mode
pub root_external_dependencies_hash: Option<&'a str>,
pub root_internal_dependencies_hash: Option<&'a str>,
pub engines: HashMap<&'a str, &'a str>,
pub env: &'a [String],
pub resolved_env_vars: EnvironmentVariablePairs,
pub pass_through_env: &'a [String],
Expand Down Expand Up @@ -303,6 +304,24 @@ impl From<GlobalHashable<'_>> for Builder<HeapAllocator> {
}
}

{
let mut entries = builder
.reborrow()
.init_engines(hashable.engines.len() as u32);

// get a sorted iterator over keys and values of the hashmap
// and set the entries in the capnp message

let mut hashable: Vec<_> = hashable.engines.iter().collect();
hashable.sort_by(|a, b| a.0.cmp(b.0));

for (i, (key, value)) in hashable.iter().enumerate() {
let mut entry = entries.reborrow().get(i as u32);
entry.set_key(key);
entry.set_value(value);
}
}

if let Some(root_external_dependencies_hash) = hashable.root_external_dependencies_hash {
builder.set_root_external_deps_hash(root_external_dependencies_hash);
}
Expand Down Expand Up @@ -407,14 +426,15 @@ mod test {
global_file_hash_map: &global_file_hash_map,
root_external_dependencies_hash: Some("0000000000000000"),
root_internal_dependencies_hash: Some("0000000000000001"),
engines: Default::default(),
env: &["env".to_string()],
resolved_env_vars: vec![],
pass_through_env: &["pass_through_env".to_string()],
env_mode: EnvMode::Strict,
framework_inference: true,
};

assert_eq!(global_hash.hash(), "8d5ecbdc3ff2b3f2");
assert_eq!(global_hash.hash(), "5072bd005ec02799");
}

#[test_case(vec![], "459c029558afe716" ; "empty")]
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/hash/proto.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ struct GlobalHashable {
passThroughEnv @6 :List(Text);
envMode @7 :EnvMode;
frameworkInference @8 :Bool;
engines @9 :List(Entry);


enum EnvMode {
Expand Down
17 changes: 14 additions & 3 deletions crates/turborepo-lib/src/run/global_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use tracing::debug;
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, RelativeUnixPathBuf};
use turborepo_env::{get_global_hashable_env_vars, DetailedMap, EnvironmentVariableMap};
use turborepo_lockfiles::Lockfile;
use turborepo_repository::package_manager::{self, PackageManager};
use turborepo_repository::{
package_graph::PackageInfo,
package_manager::{self, PackageManager},
};
use turborepo_scm::SCM;

use crate::{
Expand Down Expand Up @@ -41,9 +44,10 @@ pub enum Error {
pub struct GlobalHashableInputs<'a> {
pub global_cache_key: &'static str,
pub global_file_hash_map: HashMap<RelativeUnixPathBuf, String>,
// These are `None` in single package mode
// This is `None` in single package mode
pub root_external_dependencies_hash: Option<&'a str>,
pub root_internal_dependencies_hash: Option<&'a str>,
pub engines: Option<HashMap<&'a str, &'a str>>,
pub env: &'a [String],
// Only Option to allow #[derive(Default)]
pub resolved_env_vars: Option<DetailedMap>,
Expand All @@ -57,6 +61,7 @@ pub struct GlobalHashableInputs<'a> {
pub fn get_global_hash_inputs<'a, L: ?Sized + Lockfile>(
root_external_dependencies_hash: Option<&'a str>,
root_internal_dependencies_hash: Option<&'a str>,
root_package: &'a PackageInfo,
root_path: &AbsoluteSystemPath,
package_manager: &PackageManager,
lockfile: Option<&L>,
Expand All @@ -68,6 +73,8 @@ pub fn get_global_hash_inputs<'a, L: ?Sized + Lockfile>(
framework_inference: bool,
hasher: &SCM,
) -> Result<GlobalHashableInputs<'a>, Error> {
let engines = root_package.package_json.engines();

let global_hashable_env_vars =
get_global_hashable_env_vars(env_at_execution_start, global_env)?;

Expand Down Expand Up @@ -104,6 +111,7 @@ pub fn get_global_hash_inputs<'a, L: ?Sized + Lockfile>(
global_file_hash_map,
root_external_dependencies_hash,
root_internal_dependencies_hash,
engines,
env: global_env,
resolved_env_vars: Some(global_hashable_env_vars),
pass_through_env: global_pass_through_env,
Expand Down Expand Up @@ -180,6 +188,7 @@ impl<'a> GlobalHashableInputs<'a> {
global_file_hash_map: &self.global_file_hash_map,
root_external_dependencies_hash: self.root_external_dependencies_hash,
root_internal_dependencies_hash: self.root_internal_dependencies_hash,
engines: self.engines.clone().unwrap_or_default(),
env: self.env,
resolved_env_vars: self
.resolved_env_vars
Expand All @@ -200,7 +209,7 @@ mod tests {
use turbopath::AbsoluteSystemPathBuf;
use turborepo_env::EnvironmentVariableMap;
use turborepo_lockfiles::Lockfile;
use turborepo_repository::package_manager::PackageManager;
use turborepo_repository::{package_graph::PackageInfo, package_manager::PackageManager};
use turborepo_scm::SCM;

use super::get_global_hash_inputs;
Expand All @@ -222,6 +231,7 @@ mod tests {
.unwrap();

let env_var_map = EnvironmentVariableMap::default();
let package_info = PackageInfo::default();
let lockfile: Option<&dyn Lockfile> = None;
#[cfg(windows)]
let file_deps = ["C:\\some\\path".to_string()];
Expand All @@ -230,6 +240,7 @@ mod tests {
let result = get_global_hash_inputs(
None,
None,
&package_info,
&root,
&PackageManager::Pnpm,
lockfile,
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ impl Run {
get_global_hash_inputs(
root_external_dependencies_hash.as_deref(),
root_internal_dependencies_hash.as_deref(),
root_workspace,
&self.repo_root,
self.pkg_dep_graph.package_manager(),
self.pkg_dep_graph.lockfile(),
Expand Down
5 changes: 5 additions & 0 deletions crates/turborepo-lib/src/run/summary/global_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct GlobalHashSummary<'a> {
pub hash_of_external_dependencies: &'a str,
pub hash_of_internal_dependencies: &'a str,
pub environment_variables: GlobalEnvVarSummary<'a>,
pub engines: Option<BTreeMap<&'a str, &'a str>>,
}

impl<'a> TryFrom<GlobalHashableInputs<'a>> for GlobalHashSummary<'a> {
Expand All @@ -48,6 +49,7 @@ impl<'a> TryFrom<GlobalHashableInputs<'a>> for GlobalHashSummary<'a> {
resolved_env_vars,
pass_through_env,
env_at_execution_start,
engines,
..
} = global_hashable_inputs;

Expand All @@ -62,6 +64,8 @@ impl<'a> TryFrom<GlobalHashableInputs<'a>> for GlobalHashSummary<'a> {
)
.transpose()?;

let engines = engines.map(|engines| engines.into_iter().collect());

Ok(Self {
root_key: global_cache_key,
files: global_file_hash_map.into_iter().collect(),
Expand All @@ -81,6 +85,7 @@ impl<'a> TryFrom<GlobalHashableInputs<'a>> for GlobalHashSummary<'a> {
.map(|vars| vars.by_source.matching.to_secret_hashable()),
pass_through,
},
engines,
})
}
}
14 changes: 14 additions & 0 deletions crates/turborepo-lib/src/run/summary/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,20 @@ impl<'a> RunSummary<'a> {
.unwrap_or_default()
.join(", ")
)?;
cwriteln!(
tab_writer,
ui,
GREY,
" Engines Values\t=\t{}",
self.global_hash_summary
.engines
.as_ref()
.map(|engines| engines
.iter()
.map(|(key, value)| format!("{key}={value}"))
.join(", "))
.unwrap_or_default()
)?;

tab_writer.flush()?;
println!();
Expand Down
18 changes: 17 additions & 1 deletion crates/turborepo-repository/src/package_json.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::{collections::BTreeMap, str::FromStr};
use std::{
collections::{BTreeMap, HashMap},
str::FromStr,
};

use anyhow::Result;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -79,6 +82,19 @@ impl PackageJson {
.filter(|command| !command.is_empty())
.map(|command| command.as_str())
}

pub fn engines(&self) -> Option<HashMap<&str, &str>> {
let engines = self.other.get("engines")?.as_object()?;
Some(
engines
.iter()
.filter_map(|(key, value)| {
let value = value.as_str()?;
Some((key.as_str(), value))
})
.collect(),
)
}
}

impl FromStr for PackageJson {
Expand Down
7 changes: 4 additions & 3 deletions turborepo-tests/integration/tests/dry-json/monorepo.t
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ Setup
"configured": [],
"inferred": [],
"passthrough": null
}
},
"engines": null
}

$ cat tmpjson.log | jq 'keys'
Expand All @@ -50,7 +51,7 @@ Setup
"taskId": "my-app#build",
"task": "build",
"package": "my-app",
"hash": "270f1ef47a80f1d1",
"hash": "0555ce94ca234049",
"inputs": {
".env.local": "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
"package.json": "1746e0db2361085b5953a6a3beab08c24af5bc08"
Expand Down Expand Up @@ -110,7 +111,7 @@ Setup
"taskId": "util#build",
"task": "build",
"package": "util",
"hash": "fad2a643cb480b55",
"hash": "bf1798d3e46e1b48",
"inputs": {
"package.json": "e755064fd7893809d10fc067bb409c7ae516327f"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,16 @@ Setup
"configured": [],
"inferred": [],
"passthrough": null
}
},
"engines": null
},
"envMode": "strict",
"frameworkInference": true,
"tasks": [
{
"taskId": "build",
"task": "build",
"hash": "12c592ddc0e53a5c",
"hash": "e2b99dad85a4ff66",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ Setup
"configured": [],
"inferred": [],
"passthrough": null
}
},
"engines": null
},
"envMode": "strict",
"frameworkInference": true,
"tasks": [
{
"taskId": "build",
"task": "build",
"hash": "81a933c332d3f388",
"hash": "7ece7b62aad25615",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down Expand Up @@ -87,7 +88,7 @@ Setup
{
"taskId": "test",
"task": "test",
"hash": "785d8ef1115bde3b",
"hash": "cb5839f7284aa5f3",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
5 changes: 3 additions & 2 deletions turborepo-tests/integration/tests/dry-json/single-package.t
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ Setup
"configured": [],
"inferred": [],
"passthrough": null
}
},
"engines": null
},
"envMode": "strict",
"frameworkInference": true,
"tasks": [
{
"taskId": "build",
"task": "build",
"hash": "81a933c332d3f388",
"hash": "7ece7b62aad25615",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
Loading

0 comments on commit 0d5b628

Please sign in to comment.