Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: all dependencies of root package contribute to global hash #8202

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions crates/turborepo-lib/src/hash/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ pub struct TaskHashable<'a> {
pub struct GlobalHashable<'a> {
pub global_cache_key: &'static str,
pub global_file_hash_map: &'a HashMap<turbopath::RelativeUnixPathBuf, String>,
// This is None in single package mode
// These are None in single package mode
pub root_external_dependencies_hash: Option<&'a str>,
pub root_internal_dependencies_hash: Option<&'a str>,
pub env: &'a [String],
pub resolved_env_vars: EnvironmentVariablePairs,
pub pass_through_env: &'a [String],
Expand Down Expand Up @@ -306,6 +307,10 @@ impl From<GlobalHashable<'_>> for Builder<HeapAllocator> {
builder.set_root_external_deps_hash(root_external_dependencies_hash);
}

if let Some(root_internal_dependencies_hash) = hashable.root_internal_dependencies_hash {
builder.set_root_internal_deps_hash(root_internal_dependencies_hash);
}

{
let mut entries = builder.reborrow().init_env(hashable.env.len() as u32);
for (i, env) in hashable.env.iter().enumerate() {
Expand Down Expand Up @@ -401,14 +406,15 @@ mod test {
global_cache_key: "global_cache_key",
global_file_hash_map: &global_file_hash_map,
root_external_dependencies_hash: Some("0000000000000000"),
root_internal_dependencies_hash: Some("0000000000000001"),
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(), "9f06917065be0a72");
assert_eq!(global_hash.hash(), "8d5ecbdc3ff2b3f2");
}

#[test_case(vec![], "459c029558afe716" ; "empty")]
Expand Down
12 changes: 6 additions & 6 deletions crates/turborepo-lib/src/hash/proto.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ struct TaskHashable {
resolvedEnvVars @9 :List(Text);
passThruEnv @10 :List(Text);
envMode @11 :EnvMode;
dotEnv @12 :List(Text);

enum EnvMode {
loose @0;
Expand All @@ -36,11 +35,12 @@ struct GlobalHashable {
globalCacheKey @0 :Text;
globalFileHashMap @1 :List(Entry);
rootExternalDepsHash @2 :Text;
env @3 :List(Text);
resolvedEnvVars @4 :List(Text);
passThroughEnv @5 :List(Text);
envMode @6 :EnvMode;
frameworkInference @7 :Bool;
rootInternalDepsHash @3 :Text;
env @4 :List(Text);
resolvedEnvVars @5 :List(Text);
passThroughEnv @6 :List(Text);
envMode @7 :EnvMode;
frameworkInference @8 :Bool;


enum EnvMode {
Expand Down
7 changes: 6 additions & 1 deletion crates/turborepo-lib/src/run/global_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ pub enum Error {
pub struct GlobalHashableInputs<'a> {
pub global_cache_key: &'static str,
pub global_file_hash_map: HashMap<RelativeUnixPathBuf, String>,
// This is `None` in single package mode
// These are `None` in single package mode
pub root_external_dependencies_hash: Option<&'a str>,
pub root_internal_dependencies_hash: Option<&'a str>,
pub env: &'a [String],
// Only Option to allow #[derive(Default)]
pub resolved_env_vars: Option<DetailedMap>,
Expand All @@ -55,6 +56,7 @@ pub struct GlobalHashableInputs<'a> {
#[allow(clippy::too_many_arguments)]
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_path: &AbsoluteSystemPath,
package_manager: &PackageManager,
lockfile: Option<&L>,
Expand Down Expand Up @@ -101,6 +103,7 @@ pub fn get_global_hash_inputs<'a, L: ?Sized + Lockfile>(
global_cache_key: GLOBAL_CACHE_KEY,
global_file_hash_map,
root_external_dependencies_hash,
root_internal_dependencies_hash,
env: global_env,
resolved_env_vars: Some(global_hashable_env_vars),
pass_through_env: global_pass_through_env,
Expand Down Expand Up @@ -176,6 +179,7 @@ impl<'a> GlobalHashableInputs<'a> {
global_cache_key: self.global_cache_key,
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,
env: self.env,
resolved_env_vars: self
.resolved_env_vars
Expand Down Expand Up @@ -224,6 +228,7 @@ mod tests {
#[cfg(not(windows))]
let file_deps = ["/some/path".to_string()];
let result = get_global_hash_inputs(
None,
None,
&root,
&PackageManager::Pnpm,
Expand Down
13 changes: 12 additions & 1 deletion crates/turborepo-lib/src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use crate::{
run::{global_hash::get_global_hash_inputs, summary::RunTracker, task_access::TaskAccess},
signal::SignalHandler,
task_graph::Visitor,
task_hash::{get_external_deps_hash, PackageInputsHashes},
task_hash::{get_external_deps_hash, get_internal_deps_hash, PackageInputsHashes},
turbo_json::TurboJson,
DaemonClient, DaemonConnector,
};
Expand Down Expand Up @@ -259,6 +259,16 @@ impl Run {
let root_external_dependencies_hash =
is_monorepo.then(|| get_external_deps_hash(&root_workspace.transitive_dependencies));

let root_internal_dependencies_hash = is_monorepo
.then(|| {
get_internal_deps_hash(
&self.scm,
&self.repo_root,
self.pkg_dep_graph.root_internal_package_dependencies(),
)
})
.transpose()?;

let global_hash_inputs = {
let env_mode = self.opts.run_opts.env_mode;
let pass_through_env = match env_mode {
Expand All @@ -271,6 +281,7 @@ impl Run {

get_global_hash_inputs(
root_external_dependencies_hash.as_deref(),
root_internal_dependencies_hash.as_deref(),
&self.repo_root,
self.pkg_dep_graph.package_manager(),
self.pkg_dep_graph.lockfile(),
Expand Down
2 changes: 2 additions & 0 deletions crates/turborepo-lib/src/run/summary/global_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
pub root_key: &'static str,
pub files: BTreeMap<RelativeUnixPathBuf, String>,
pub hash_of_external_dependencies: &'a str,
pub hash_of_internal_dependencies: &'a str,
pub environment_variables: GlobalEnvVarSummary<'a>,
}

Expand Down Expand Up @@ -65,6 +66,7 @@
files: global_file_hash_map.into_iter().collect(),
// This can be empty in single package mode
hash_of_external_dependencies: root_external_dependencies_hash.unwrap_or_default(),
hash_of_internal_dependencies: root_internal_dependencies_hash.unwrap_or_default(),

Check failure on line 69 in crates/turborepo-lib/src/run/summary/global_hash.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust check

cannot find value `root_internal_dependencies_hash` in this scope

Check failure on line 69 in crates/turborepo-lib/src/run/summary/global_hash.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (ubuntu, self-hosted, linux, x64, metal)

cannot find value `root_internal_dependencies_hash` in this scope

Check failure on line 69 in crates/turborepo-lib/src/run/summary/global_hash.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (macos, macos-12)

cannot find value `root_internal_dependencies_hash` in this scope

Check failure on line 69 in crates/turborepo-lib/src/run/summary/global_hash.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (windows, windows-latest)

cannot find value `root_internal_dependencies_hash` in this scope
chris-olszewski marked this conversation as resolved.
Show resolved Hide resolved
environment_variables: GlobalEnvVarSummary {
specified: GlobalEnvConfiguration {
env,
Expand Down
24 changes: 24 additions & 0 deletions crates/turborepo-lib/src/task_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,30 @@ pub fn get_external_deps_hash(
LockFilePackages(transitive_deps).hash()
}

pub fn get_internal_deps_hash(
scm: &SCM,
root: &AbsoluteSystemPath,
package_dirs: HashSet<&AnchoredSystemPath>,
) -> Result<String, Error> {
if package_dirs.is_empty() {
return Ok("".into());
}

let file_hashes = package_dirs
.into_par_iter()
.map(|package_dir| scm.get_package_file_hashes::<&str>(root, package_dir, &[], None))
.reduce(
|| Ok(HashMap::new()),
|acc, hashes| {
let mut acc = acc?;
let hashes = hashes?;
acc.extend(hashes.into_iter());
Ok(acc)
},
)?;
Ok(FileHashes(file_hashes).hash())
}

impl TaskHashTracker {
pub fn new(input_expanded_hashes: HashMap<TaskId<'static>, FileHashes>) -> Self {
Self {
Expand Down
33 changes: 31 additions & 2 deletions crates/turborepo-repository/src/package_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,20 @@ impl PackageGraph {
dependents
}

pub fn root_internal_package_dependencies(&self) -> HashSet<&AnchoredSystemPath> {
let dependencies = self.dependencies(&PackageNode::Workspace(PackageName::Root));
dependencies
.into_iter()
.filter_map(|package| match package {
PackageNode::Workspace(package) => Some(
self.package_dir(package)
.expect("packages in graph should have info"),
),
PackageNode::Root => None,
})
.collect()
}

/// Returns the transitive closure of the given nodes in the package
/// graph. Note that this includes the nodes themselves. If you want just
/// the dependencies, or the dependents, use `dependencies` or `ancestors`.
Expand Down Expand Up @@ -466,7 +480,6 @@ mod test {
use std::assert_matches::assert_matches;

use serde_json::json;
use turbopath::AbsoluteSystemPathBuf;

use super::*;
use crate::discovery::PackageDiscovery;
Expand Down Expand Up @@ -519,7 +532,10 @@ mod test {
AbsoluteSystemPathBuf::new(if cfg!(windows) { r"C:\repo" } else { "/repo" }).unwrap();
let pkg_graph = PackageGraph::builder(
&root,
PackageJson::from_value(json!({ "name": "root" })).unwrap(),
PackageJson::from_value(
json!({ "name": "root", "dependencies": { "a": "workspace:*"} }),
)
.unwrap(),
)
.with_package_discovery(MockDiscovery)
.with_package_jsons(Some({
Expand Down Expand Up @@ -572,6 +588,19 @@ mod test {

let pkg_version = b_external.get("c").unwrap();
assert_eq!(pkg_version, "1.2.3");
let closure =
pkg_graph.transitive_closure(Some(&PackageNode::Workspace(PackageName::Root)));
assert_eq!(
closure,
[
PackageNode::Root,
PackageNode::Workspace(PackageName::Root),
PackageNode::Workspace("a".into()),
PackageNode::Workspace("b".into()),
]
.iter()
.collect::<HashSet<_>>()
);
}

#[derive(Debug)]
Expand Down
4 changes: 4 additions & 0 deletions turborepo-tests/integration/fixtures/root_deps/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
node_modules/
.turbo
.npmrc
dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "my-app",
"scripts": {
"build": "echo building"
},
"dependencies": {
"util": "*"
}
}
10 changes: 10 additions & 0 deletions turborepo-tests/integration/fixtures/root_deps/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "monorepo",
"workspaces": [
"apps/**",
"packages/**"
],
"dependencies": {
"util": "*"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember if our integration test fixtures generally use packageManager or not, but that might be useful since I know * has some weird behavior in different npm versions, especially with workspaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember if our integration test fixtures generally use packageManager or not, but that might be useful since I know * has some weird behavior in different npm versions, especially with workspaces

Good callout. With package manager requirements we now force usage of corepack for all test fixture defaulting to npm if one isn't specified in the fixture. Will open a ticket to make this explicit

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "another",
"scripts": {
"build": "echo building"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "util",
"scripts": {
"build": "echo building"
}
}
12 changes: 12 additions & 0 deletions turborepo-tests/integration/fixtures/root_deps/turbo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"$schema": "https://turbo.build/schema.json",
"globalDependencies": ["foo.txt"],
"globalEnv": ["SOME_ENV_VAR"],
"tasks": {
"build": {
"env": ["NODE_ENV"],
"dependsOn": ["^build"],
"outputs": ["dist/**"]
}
}
}
4 changes: 2 additions & 2 deletions turborepo-tests/integration/tests/dry-json/monorepo.t
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Setup
"taskId": "my-app#build",
"task": "build",
"package": "my-app",
"hash": "ed450f573b231cb7",
"hash": "270f1ef47a80f1d1",
"inputs": {
".env.local": "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
"package.json": "1746e0db2361085b5953a6a3beab08c24af5bc08"
Expand Down Expand Up @@ -109,7 +109,7 @@ Setup
"taskId": "util#build",
"task": "build",
"package": "util",
"hash": "41b033e352a43533",
"hash": "fad2a643cb480b55",
"inputs": {
"package.json": "e755064fd7893809d10fc067bb409c7ae516327f"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Setup
{
"taskId": "build",
"task": "build",
"hash": "a6da7b8ddbe2bb84",
"hash": "12c592ddc0e53a5c",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Setup
{
"taskId": "build",
"task": "build",
"hash": "4047a6e65d7dafef",
"hash": "81a933c332d3f388",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down Expand Up @@ -86,7 +86,7 @@ Setup
{
"taskId": "test",
"task": "test",
"hash": "89d72e7337505ef6",
"hash": "785d8ef1115bde3b",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Setup
{
"taskId": "build",
"task": "build",
"hash": "4047a6e65d7dafef",
"hash": "81a933c332d3f388",
"inputs": {
".gitignore": "03b541460c1b836f96f9c0a941ceb48e91a9fd83",
"package-lock.json": "1c117cce37347befafe3a9cba1b8a609b3600021",
Expand Down
4 changes: 2 additions & 2 deletions turborepo-tests/integration/tests/dry-run.t
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Setup
my-app#build
Task = build\s* (re)
Package = my-app\s* (re)
Hash = ed450f573b231cb7
Hash = 270f1ef47a80f1d1
Cached (Local) = false
Cached (Remote) = false
Directory = apps(\/|\\)my-app (re)
Expand All @@ -50,7 +50,7 @@ Setup
util#build
Task = build\s* (re)
Package = util\s* (re)
Hash = 41b033e352a43533
Hash = fad2a643cb480b55
Cached (Local) = false
Cached (Remote) = false
Directory = packages(\/|\\)util (re)
Expand Down
Loading
Loading