From 180118cf18eb608be2da636fd616cfd9ef4db0f7 Mon Sep 17 00:00:00 2001 From: Mike Hilgendorf Date: Thu, 12 Feb 2026 14:16:24 -0600 Subject: [PATCH 1/2] fix(server/checkin): ensure dependencies are cached in destructive checkins Resolves #814 --- .../destructive_ensure_dependencies_cached.nu | 288 ++++++++++++++++++ packages/server/src/checkin.rs | 2 +- packages/server/src/checkin/artifact.rs | 12 +- packages/server/src/checkin/cache.rs | 220 ++++++++++++- packages/server/src/checkin/graph.rs | 1 + 5 files changed, 517 insertions(+), 6 deletions(-) create mode 100644 packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu diff --git a/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu b/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu new file mode 100644 index 000000000..c5fb1f48f --- /dev/null +++ b/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu @@ -0,0 +1,288 @@ +use ../../test.nu * +let remote = spawn --name 'remote' +let server = spawn --name 'local' --config { + remotes: [{ + name: 'default', + url: $remote.url + }] +} + + +let dep_path = artifact { + tangram.ts: 'export default () => "dep";' +} +let dep_id = tg checkin --destructive --ignore=false $dep_path +tg index + +let path = artifact { + tangram.ts: $' + import dep from "($dep_id)"; + ' +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import dep from \"dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g\";"), + "dependencies": { + "dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"dep\";"), + "module": "ts", + }), + }), + }, + }, + "module": "ts", + }), + }) +' + +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: 'export default () => "bar";' + } +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "bar": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"bar\";"), + "module": "ts", + }), + }), + "foo": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"bar\";"), + "module": "ts", + }), + }), + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }), + }), + }) +' + +# Check that we cannot destructively checkin artifacts with external paths. +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: '' + } +} +let output = tg checkin --destructive ($path | path join 'foo') --ignore=false | complete +failure $output "destructive checkin with external path dependencies should fail" + +# Check that using a tag dependency in the cache works. +let a_path = artifact { + tangram.ts: ' + export default () => "a"; + ' +} +tg tag a $a_path +tg index + +let path = artifact { + tangram.ts: ' + import a from "a"; + ' +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import a from \"a\";"), + "dependencies": { + "a": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"a\";"), + "module": "ts", + }), + }), + "options": { + "id": "dir_01397yyk1pe2sv1ddct0f0aq0qtxjbjtw55t9d7vke752ezc8at4p0", + "tag": "a", + }, + }, + }, + "module": "ts", + }), + }) +' + +# Check for cyclic dependencies. +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: 'import * as foo from "../foo";' + } +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "bar": { + "graph": tg.graph({ + "nodes": [ + { + "kind": "file", + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": { + "index": 2, + "kind": "directory", + }, + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 0, + "kind": "file", + }, + }, + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 3, + "kind": "file", + }, + }, + }, + { + "kind": "file", + "contents": tg.blob("import * as foo from \"../foo\";"), + "dependencies": { + "../foo": { + "item": { + "index": 1, + "kind": "directory", + }, + "options": { + "path": "../foo", + }, + }, + }, + "module": "ts", + }, + ], + }), + "index": 2, + "kind": "directory", + }, + "foo": { + "graph": tg.graph({ + "nodes": [ + { + "kind": "file", + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": { + "index": 2, + "kind": "directory", + }, + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 0, + "kind": "file", + }, + }, + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 3, + "kind": "file", + }, + }, + }, + { + "kind": "file", + "contents": tg.blob("import * as foo from \"../foo\";"), + "dependencies": { + "../foo": { + "item": { + "index": 1, + "kind": "directory", + }, + "options": { + "path": "../foo", + }, + }, + }, + "module": "ts", + }, + ], + }), + "index": 1, + "kind": "directory", + }, + }) +' + +# Check that destructive checkin fails when a tag dependency is only on the remote and not in the local cache. +let remote_dep_path = artifact { + tangram.ts: ' + export default () => "remote_only"; + ' +} +tg -u $remote.url tag remote_dep $remote_dep_path + +let path = artifact { + tangram.ts: ' + import remote_dep from "remote_dep"; + ' +} +let output = tg checkin --destructive $path --ignore=false | complete +failure $output "destructive checkin should fail when tag dependency is only on the remote" diff --git a/packages/server/src/checkin.rs b/packages/server/src/checkin.rs index 2112c57c9..f07c39585 100644 --- a/packages/server/src/checkin.rs +++ b/packages/server/src/checkin.rs @@ -399,7 +399,7 @@ impl Server { task.await .map_err(|source| tg::error!(!source, "failed to run the fixup task"))?; } - self.checkin_cache(&arg, &graph, next, root, progress) + self.checkin_cache(&arg, &graph, next, root, &index_cache_entry_args, progress) .await .map_err(|source| tg::error!(!source, "failed to cache"))?; } diff --git a/packages/server/src/checkin/artifact.rs b/packages/server/src/checkin/artifact.rs index f07ec3a12..2ceda7aa5 100644 --- a/packages/server/src/checkin/artifact.rs +++ b/packages/server/src/checkin/artifact.rs @@ -313,6 +313,10 @@ impl Server { index_object_args, touched_at, )?; + graph.graphs.insert( + id.unwrap_graph_ref().clone(), + data.try_unwrap_graph().unwrap(), + ); // Set edges and ids for all original nodes in the graph. let graph_id = tg::graph::Id::try_from(id).unwrap(); @@ -753,13 +757,15 @@ impl Server { let root_index = graph.paths.get(root).unwrap(); let root_node = graph.nodes.get(root_index).unwrap(); let id = root_node.id.as_ref().unwrap().clone(); - let path = node + let Ok(path) = node .path .as_ref() .unwrap() .strip_prefix(root) - .unwrap() - .to_owned(); + .map(Path::to_owned) + else { + continue; + }; let path = if path == Path::new("") { None } else { diff --git a/packages/server/src/checkin/cache.rs b/packages/server/src/checkin/cache.rs index 21ca1d2ce..4159c0777 100644 --- a/packages/server/src/checkin/cache.rs +++ b/packages/server/src/checkin/cache.rs @@ -1,12 +1,18 @@ use { - crate::{Server, checkin::Graph, temp::Temp}, + crate::{ + Server, + checkin::{Graph, IndexCacheEntryArgs, graph::Variant}, + temp::Temp, + }, futures::stream::{self, StreamExt as _, TryStreamExt as _}, std::{ + collections::BTreeSet, os::unix::fs::PermissionsExt as _, path::{Path, PathBuf}, }, tangram_client::prelude::*, - tangram_util::iter::Ext as _, + tangram_index::Index, + tangram_util::{iter::Ext as _, path}, }; impl Server { @@ -17,9 +23,14 @@ impl Server { graph: &Graph, next: usize, root: &Path, + index_cache_entry_args: &IndexCacheEntryArgs, progress: &crate::progress::Handle, ) -> tg::Result<()> { if arg.options.destructive { + progress.spinner("checking", "checking"); + self.checkin_ensure_dependencies_are_cached(graph, root, index_cache_entry_args) + .await?; + progress.finish("checking"); progress.spinner("copying", "copying"); self.checkin_cache_destructive(graph, root).await?; progress.finish("copying"); @@ -186,4 +197,209 @@ impl Server { Ok(()) } + + async fn checkin_ensure_dependencies_are_cached( + &self, + graph: &Graph, + path: &Path, + index_cache_entry_args: &IndexCacheEntryArgs, + ) -> tg::Result<()> { + let root = graph + .paths + .get(path) + .copied() + .ok_or_else(|| tg::error!("expected a node"))?; + let will_cache = index_cache_entry_args + .iter() + .map(|arg| arg.id.clone()) + .collect::>(); + let mut visited = BTreeSet::new(); + let mut stack = vec![root]; + let root_is_dir = graph + .nodes + .get(&root) + .is_some_and(|node| node.variant.is_directory()); + + let mut artifacts = Vec::new(); + while let Some(index) = stack.pop() { + if !visited.insert(index) { + continue; + } + + let Some(node) = graph.nodes.get(&index) else { + continue; + }; + + // Make sure we're not missing local path dependencies. + if let Some(node_path) = &node.path + && index != root + { + if !root_is_dir { + return Err(tg::error!( + "cannot destructively checkin files or symlinks with path dependencies" + )); + } + if path::diff(path, node_path).is_ok_and(|path| path.starts_with("..")) { + return Err(tg::error!( + "cannot destructively check in an artifact with external path dependencies" + )); + } + } + + match &node.variant { + Variant::Directory(directory) => { + for edge in directory.entries.values() { + match edge { + tg::graph::data::Edge::Object(id) => { + if let Some(index) = graph.artifacts.get(id) { + if !will_cache.contains(id) { + return Err(tg::error!("missing cache dependency")); + } + stack.push(*index); + } + artifacts.push(id.clone()); + }, + tg::graph::data::Edge::Pointer(pointer) => { + if let Some(id) = &pointer.graph { + let artifact = tg::Artifact::with_pointer(tg::graph::Pointer { + graph: Some(tg::Graph::with_id(id.clone())), + index: pointer.index, + kind: pointer.kind, + }) + .id(); + if will_cache.contains(&artifact) { + continue; + } + let data = graph.graphs.get(id); + let ids = self.graph_ids(id, data).await.map_err(|source| { + tg::error!(!source, "failed to get the graph ids") + })?; + artifacts.extend(ids); + } else { + stack.push(pointer.index); + } + }, + } + } + }, + Variant::File(file) => { + for dependency in file.dependencies.values().flatten() { + if let Some(edge) = &dependency.item { + match edge { + tg::graph::data::Edge::Object(id) => { + let Ok(id) = tg::artifact::Id::try_from(id.clone()) else { + continue; + }; + if let Some(index) = graph.artifacts.get(&id) { + if !will_cache.contains(&id) { + return Err(tg::error!("missing cache dependency")); + } + stack.push(*index); + } + artifacts.push(id.clone()); + }, + tg::graph::data::Edge::Pointer(pointer) => { + if let Some(id) = &pointer.graph { + let artifact = + tg::Artifact::with_pointer(tg::graph::Pointer { + graph: Some(tg::Graph::with_id(id.clone())), + index: pointer.index, + kind: pointer.kind, + }) + .id(); + if will_cache.contains(&artifact) { + continue; + } + let data = graph.graphs.get(id); + let ids = + self.graph_ids(id, data).await.map_err(|source| { + tg::error!(!source, "failed to get the graph ids") + })?; + artifacts.extend(ids); + } else { + stack.push(pointer.index); + } + }, + } + } + } + }, + Variant::Symlink(symlink) => { + if let Some(edge) = &symlink.artifact { + match edge { + tg::graph::data::Edge::Object(id) => { + if let Some(index) = graph.artifacts.get(id) { + if !will_cache.contains(id) { + return Err(tg::error!("missing cache dependency")); + } + stack.push(*index); + } + artifacts.push(id.clone()); + }, + tg::graph::data::Edge::Pointer(pointer) => { + if let Some(id) = &pointer.graph { + let artifact = tg::Artifact::with_pointer(tg::graph::Pointer { + graph: Some(tg::Graph::with_id(id.clone())), + index: pointer.index, + kind: pointer.kind, + }) + .id(); + if will_cache.contains(&artifact) { + continue; + } + let data = graph.graphs.get(id); + let ids = self.graph_ids(id, data).await.map_err(|source| { + tg::error!(!source, "failed to get the graph ids") + })?; + artifacts.extend(ids); + } else { + stack.push(pointer.index); + } + }, + } + } + }, + } + } + let touched_at = time::OffsetDateTime::now_utc().unix_timestamp(); + let all_cached = self + .index + .touch_cache_entries(&artifacts, touched_at) + .await + .map_err(|source| { + tg::error!(!source, "failed to get the cache entries from the index") + })? + .into_iter() + .all(|entry| entry.is_some()); + if !all_cached { + return Err(tg::error!("missing cache dependency")); + } + Ok(()) + } + + pub(crate) async fn graph_ids( + &self, + graph: &tg::graph::Id, + data: Option<&tg::graph::Data>, + ) -> tg::Result> { + let data = if let Some(data) = data { + data.clone() + } else { + tg::Graph::with_id(graph.clone()) + .data(self) + .await + .map_err(|source| tg::error!(!source, "failed to get the graph data"))? + }; + let graph = tg::Graph::with_id(graph.clone()); + let mut nodes = Vec::with_capacity(data.nodes.len()); + for (index, node) in data.nodes.into_iter().enumerate() { + let artifact = tg::Artifact::with_pointer(tg::graph::Pointer { + graph: Some(graph.clone()), + index, + kind: node.kind(), + }); + nodes.push(artifact.id()); + } + Ok(nodes) + } } diff --git a/packages/server/src/checkin/graph.rs b/packages/server/src/checkin/graph.rs index f31def269..c921dbbdd 100644 --- a/packages/server/src/checkin/graph.rs +++ b/packages/server/src/checkin/graph.rs @@ -11,6 +11,7 @@ use { #[derive(Clone, Debug, Default)] pub struct Graph { pub artifacts: im::HashMap, + pub graphs: im::HashMap, pub ids: im::HashMap, tg::id::BuildHasher>, pub next: usize, pub nodes: im::OrdMap>, From 78a81c866d2a4ac7b2c1fb9f15a0df7e5ffedf77 Mon Sep 17 00:00:00 2001 From: Mike Hilgendorf Date: Fri, 13 Feb 2026 14:31:41 -0600 Subject: [PATCH 2/2] fixes --- .../destructive_cache_cyclic_dependencies.nu | 137 +++++++++ .../destructive_cache_id_dependency.nu | 42 +++ .../destructive_cache_path_dependency.nu | 45 +++ .../destructive_cache_tag_dependency.nu | 44 +++ .../destructive_ensure_dependencies_cached.nu | 288 ------------------ .../destructive_external_path_dependency.nu | 15 + .../destructive_remote_only_tag_dependency.nu | 46 +++ packages/server/src/checkin.rs | 18 +- packages/server/src/checkin/artifact.rs | 7 +- packages/server/src/checkin/cache.rs | 47 +-- packages/server/src/checkin/graph.rs | 1 - 11 files changed, 378 insertions(+), 312 deletions(-) create mode 100644 packages/cli/tests/checkin/destructive_cache_cyclic_dependencies.nu create mode 100644 packages/cli/tests/checkin/destructive_cache_id_dependency.nu create mode 100644 packages/cli/tests/checkin/destructive_cache_path_dependency.nu create mode 100644 packages/cli/tests/checkin/destructive_cache_tag_dependency.nu delete mode 100644 packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu create mode 100644 packages/cli/tests/checkin/destructive_external_path_dependency.nu create mode 100644 packages/cli/tests/checkin/destructive_remote_only_tag_dependency.nu diff --git a/packages/cli/tests/checkin/destructive_cache_cyclic_dependencies.nu b/packages/cli/tests/checkin/destructive_cache_cyclic_dependencies.nu new file mode 100644 index 000000000..21bff9af7 --- /dev/null +++ b/packages/cli/tests/checkin/destructive_cache_cyclic_dependencies.nu @@ -0,0 +1,137 @@ +use ../../test.nu * + +let server = spawn + +# Check for cyclic dependencies. +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: 'import * as foo from "../foo";' + } +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "bar": { + "graph": tg.graph({ + "nodes": [ + { + "kind": "file", + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": { + "index": 2, + "kind": "directory", + }, + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 0, + "kind": "file", + }, + }, + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 3, + "kind": "file", + }, + }, + }, + { + "kind": "file", + "contents": tg.blob("import * as foo from \"../foo\";"), + "dependencies": { + "../foo": { + "item": { + "index": 1, + "kind": "directory", + }, + "options": { + "path": "../foo", + }, + }, + }, + "module": "ts", + }, + ], + }), + "index": 2, + "kind": "directory", + }, + "foo": { + "graph": tg.graph({ + "nodes": [ + { + "kind": "file", + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": { + "index": 2, + "kind": "directory", + }, + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 0, + "kind": "file", + }, + }, + }, + { + "kind": "directory", + "entries": { + "tangram.ts": { + "index": 3, + "kind": "file", + }, + }, + }, + { + "kind": "file", + "contents": tg.blob("import * as foo from \"../foo\";"), + "dependencies": { + "../foo": { + "item": { + "index": 1, + "kind": "directory", + }, + "options": { + "path": "../foo", + }, + }, + }, + "module": "ts", + }, + ], + }), + "index": 1, + "kind": "directory", + }, + }) +' diff --git a/packages/cli/tests/checkin/destructive_cache_id_dependency.nu b/packages/cli/tests/checkin/destructive_cache_id_dependency.nu new file mode 100644 index 000000000..c5eed27ca --- /dev/null +++ b/packages/cli/tests/checkin/destructive_cache_id_dependency.nu @@ -0,0 +1,42 @@ +use ../../test.nu * +let remote = spawn --name 'remote' +let server = spawn --name 'local' --config { + remotes: [{ + name: 'default', + url: $remote.url + }] +} + +let dep_path = artifact { + tangram.ts: 'export default () => "dep";' +} +let dep_id = tg checkin --destructive --ignore=false $dep_path +tg index + +let path = artifact { + tangram.ts: $' + import dep from "($dep_id)"; + ' +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import dep from \"dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g\";"), + "dependencies": { + "dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"dep\";"), + "module": "ts", + }), + }), + }, + }, + "module": "ts", + }), + }) +' diff --git a/packages/cli/tests/checkin/destructive_cache_path_dependency.nu b/packages/cli/tests/checkin/destructive_cache_path_dependency.nu new file mode 100644 index 000000000..9a3122fe1 --- /dev/null +++ b/packages/cli/tests/checkin/destructive_cache_path_dependency.nu @@ -0,0 +1,45 @@ +use ../../test.nu * + +let server = spawn + +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: 'export default () => "bar";' + } +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "bar": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"bar\";"), + "module": "ts", + }), + }), + "foo": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import * as bar from \"../bar\";"), + "dependencies": { + "../bar": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"bar\";"), + "module": "ts", + }), + }), + "options": { + "path": "../bar", + }, + }, + }, + "module": "ts", + }), + }), + }) +' diff --git a/packages/cli/tests/checkin/destructive_cache_tag_dependency.nu b/packages/cli/tests/checkin/destructive_cache_tag_dependency.nu new file mode 100644 index 000000000..740671111 --- /dev/null +++ b/packages/cli/tests/checkin/destructive_cache_tag_dependency.nu @@ -0,0 +1,44 @@ +use ../../test.nu * + +let server = spawn + +# Check that using a tag dependency in the cache works. +let a_path = artifact { + tangram.ts: ' + export default () => "a"; + ' +} +tg tag a $a_path +tg index + +let path = artifact { + tangram.ts: ' + import a from "a"; + ' +} +let id = tg checkin --destructive $path --ignore=false +tg index + +let object = tg object get --blobs --depth=inf --pretty $id +snapshot $object ' + tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import a from \"a\";"), + "dependencies": { + "a": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"a\";"), + "module": "ts", + }), + }), + "options": { + "id": "dir_01397yyk1pe2sv1ddct0f0aq0qtxjbjtw55t9d7vke752ezc8at4p0", + "tag": "a", + }, + }, + }, + "module": "ts", + }), + }) +' diff --git a/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu b/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu deleted file mode 100644 index c5fb1f48f..000000000 --- a/packages/cli/tests/checkin/destructive_ensure_dependencies_cached.nu +++ /dev/null @@ -1,288 +0,0 @@ -use ../../test.nu * -let remote = spawn --name 'remote' -let server = spawn --name 'local' --config { - remotes: [{ - name: 'default', - url: $remote.url - }] -} - - -let dep_path = artifact { - tangram.ts: 'export default () => "dep";' -} -let dep_id = tg checkin --destructive --ignore=false $dep_path -tg index - -let path = artifact { - tangram.ts: $' - import dep from "($dep_id)"; - ' -} -let id = tg checkin --destructive $path --ignore=false -tg index - -let object = tg object get --blobs --depth=inf --pretty $id -snapshot $object ' - tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("import dep from \"dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g\";"), - "dependencies": { - "dir_01eg1hz5n9tvjmzbsvxgwegxa5jnx10pgpx47k99b9z200v7kqdm7g": { - "item": tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("export default () => \"dep\";"), - "module": "ts", - }), - }), - }, - }, - "module": "ts", - }), - }) -' - -let path = artifact { - foo: { - tangram.ts: 'import * as bar from "../bar";' - } - bar: { - tangram.ts: 'export default () => "bar";' - } -} -let id = tg checkin --destructive $path --ignore=false -tg index - -let object = tg object get --blobs --depth=inf --pretty $id -snapshot $object ' - tg.directory({ - "bar": tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("export default () => \"bar\";"), - "module": "ts", - }), - }), - "foo": tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("import * as bar from \"../bar\";"), - "dependencies": { - "../bar": { - "item": tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("export default () => \"bar\";"), - "module": "ts", - }), - }), - "options": { - "path": "../bar", - }, - }, - }, - "module": "ts", - }), - }), - }) -' - -# Check that we cannot destructively checkin artifacts with external paths. -let path = artifact { - foo: { - tangram.ts: 'import * as bar from "../bar";' - } - bar: { - tangram.ts: '' - } -} -let output = tg checkin --destructive ($path | path join 'foo') --ignore=false | complete -failure $output "destructive checkin with external path dependencies should fail" - -# Check that using a tag dependency in the cache works. -let a_path = artifact { - tangram.ts: ' - export default () => "a"; - ' -} -tg tag a $a_path -tg index - -let path = artifact { - tangram.ts: ' - import a from "a"; - ' -} -let id = tg checkin --destructive $path --ignore=false -tg index - -let object = tg object get --blobs --depth=inf --pretty $id -snapshot $object ' - tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("import a from \"a\";"), - "dependencies": { - "a": { - "item": tg.directory({ - "tangram.ts": tg.file({ - "contents": tg.blob("export default () => \"a\";"), - "module": "ts", - }), - }), - "options": { - "id": "dir_01397yyk1pe2sv1ddct0f0aq0qtxjbjtw55t9d7vke752ezc8at4p0", - "tag": "a", - }, - }, - }, - "module": "ts", - }), - }) -' - -# Check for cyclic dependencies. -let path = artifact { - foo: { - tangram.ts: 'import * as bar from "../bar";' - } - bar: { - tangram.ts: 'import * as foo from "../foo";' - } -} -let id = tg checkin --destructive $path --ignore=false -tg index - -let object = tg object get --blobs --depth=inf --pretty $id -snapshot $object ' - tg.directory({ - "bar": { - "graph": tg.graph({ - "nodes": [ - { - "kind": "file", - "contents": tg.blob("import * as bar from \"../bar\";"), - "dependencies": { - "../bar": { - "item": { - "index": 2, - "kind": "directory", - }, - "options": { - "path": "../bar", - }, - }, - }, - "module": "ts", - }, - { - "kind": "directory", - "entries": { - "tangram.ts": { - "index": 0, - "kind": "file", - }, - }, - }, - { - "kind": "directory", - "entries": { - "tangram.ts": { - "index": 3, - "kind": "file", - }, - }, - }, - { - "kind": "file", - "contents": tg.blob("import * as foo from \"../foo\";"), - "dependencies": { - "../foo": { - "item": { - "index": 1, - "kind": "directory", - }, - "options": { - "path": "../foo", - }, - }, - }, - "module": "ts", - }, - ], - }), - "index": 2, - "kind": "directory", - }, - "foo": { - "graph": tg.graph({ - "nodes": [ - { - "kind": "file", - "contents": tg.blob("import * as bar from \"../bar\";"), - "dependencies": { - "../bar": { - "item": { - "index": 2, - "kind": "directory", - }, - "options": { - "path": "../bar", - }, - }, - }, - "module": "ts", - }, - { - "kind": "directory", - "entries": { - "tangram.ts": { - "index": 0, - "kind": "file", - }, - }, - }, - { - "kind": "directory", - "entries": { - "tangram.ts": { - "index": 3, - "kind": "file", - }, - }, - }, - { - "kind": "file", - "contents": tg.blob("import * as foo from \"../foo\";"), - "dependencies": { - "../foo": { - "item": { - "index": 1, - "kind": "directory", - }, - "options": { - "path": "../foo", - }, - }, - }, - "module": "ts", - }, - ], - }), - "index": 1, - "kind": "directory", - }, - }) -' - -# Check that destructive checkin fails when a tag dependency is only on the remote and not in the local cache. -let remote_dep_path = artifact { - tangram.ts: ' - export default () => "remote_only"; - ' -} -tg -u $remote.url tag remote_dep $remote_dep_path - -let path = artifact { - tangram.ts: ' - import remote_dep from "remote_dep"; - ' -} -let output = tg checkin --destructive $path --ignore=false | complete -failure $output "destructive checkin should fail when tag dependency is only on the remote" diff --git a/packages/cli/tests/checkin/destructive_external_path_dependency.nu b/packages/cli/tests/checkin/destructive_external_path_dependency.nu new file mode 100644 index 000000000..b9f67ca8c --- /dev/null +++ b/packages/cli/tests/checkin/destructive_external_path_dependency.nu @@ -0,0 +1,15 @@ +use ../../test.nu * + +let server = spawn + +# Check that we cannot destructively checkin artifacts with external paths. +let path = artifact { + foo: { + tangram.ts: 'import * as bar from "../bar";' + } + bar: { + tangram.ts: '' + } +} +let output = tg checkin --destructive ($path | path join 'foo') --ignore=false | complete +failure $output "destructive checkin with external path dependencies should fail" diff --git a/packages/cli/tests/checkin/destructive_remote_only_tag_dependency.nu b/packages/cli/tests/checkin/destructive_remote_only_tag_dependency.nu new file mode 100644 index 000000000..4de00d4b0 --- /dev/null +++ b/packages/cli/tests/checkin/destructive_remote_only_tag_dependency.nu @@ -0,0 +1,46 @@ +use ../../test.nu * +let remote = spawn --name 'remote' +let server = spawn --name 'local' --config { + remotes: [{ + name: 'default', + url: $remote.url + }] +} + +# Check that destructive checkin fails when a tag dependency is only on the remote and not in the local cache. +let remote_dep_path = artifact { + tangram.ts: ' + export default () => "remote_only"; + ' +} +tg -u $remote.url tag remote_dep $remote_dep_path + +let path = artifact { + tangram.ts: ' + import remote_dep from "remote_dep"; + ' +} +let id = tg checkin --destructive $path --ignore=false +let object = tg get $id --depth=inf --blobs --pretty +snapshot $object ' + tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("import remote_dep from \"remote_dep\";"), + "dependencies": { + "remote_dep": { + "item": tg.directory({ + "tangram.ts": tg.file({ + "contents": tg.blob("export default () => \"remote_only\";"), + "module": "ts", + }), + }), + "options": { + "id": "dir_01stfjn9km4b3vq5w8bgqc0xdgkwwn4307p5kysj41z1gsvacytp1g", + "tag": "remote_dep", + }, + }, + }, + "module": "ts", + }), + }) +' diff --git a/packages/server/src/checkin.rs b/packages/server/src/checkin.rs index f07c39585..6260a1856 100644 --- a/packages/server/src/checkin.rs +++ b/packages/server/src/checkin.rs @@ -48,6 +48,8 @@ type IndexCacheEntryArgs = Vec; type StoreArgs = IndexMap; +type GraphData = IndexMap; + impl Server { #[tracing::instrument(fields(path = ?arg.path), level = "trace", name = "checkin", skip_all)] pub(crate) async fn checkin_with_context( @@ -365,6 +367,7 @@ impl Server { let mut store_args = IndexMap::default(); let mut index_object_args = IndexMap::default(); let mut index_cache_entry_args = Vec::new(); + let mut graph_data = IndexMap::default(); // Create blobs. self.checkin_create_blobs( @@ -389,6 +392,7 @@ impl Server { &mut store_args, &mut index_object_args, &mut index_cache_entry_args, + &mut graph_data, root, touched_at, )?; @@ -399,9 +403,17 @@ impl Server { task.await .map_err(|source| tg::error!(!source, "failed to run the fixup task"))?; } - self.checkin_cache(&arg, &graph, next, root, &index_cache_entry_args, progress) - .await - .map_err(|source| tg::error!(!source, "failed to cache"))?; + self.checkin_cache( + &arg, + &graph, + next, + root, + &index_cache_entry_args, + &mut graph_data, + progress, + ) + .await + .map_err(|source| tg::error!(!source, "failed to cache"))?; } // Store. diff --git a/packages/server/src/checkin/artifact.rs b/packages/server/src/checkin/artifact.rs index 2ceda7aa5..f8bde4e96 100644 --- a/packages/server/src/checkin/artifact.rs +++ b/packages/server/src/checkin/artifact.rs @@ -2,7 +2,7 @@ use { crate::{ Server, checkin::{ - Graph, IndexCacheEntryArgs, IndexObjectArgs, StoreArgs, + Graph, GraphData, IndexCacheEntryArgs, IndexObjectArgs, StoreArgs, graph::{Contents, Node, Petgraph, Variant}, path::Paths, }, @@ -31,6 +31,7 @@ impl Server { store_args: &mut StoreArgs, index_object_args: &mut IndexObjectArgs, index_cache_entry_args: &mut IndexCacheEntryArgs, + graph_data: &mut GraphData, root: &Path, touched_at: i64, ) -> tg::Result<()> { @@ -67,6 +68,7 @@ impl Server { paths, store_args, index_object_args, + graph_data, scc, touched_at, )?; @@ -269,6 +271,7 @@ impl Server { paths: &Paths, store_args: &mut StoreArgs, index_object_args: &mut IndexObjectArgs, + graph_data: &mut GraphData, scc: &[usize], touched_at: i64, ) -> tg::Result<()> { @@ -313,7 +316,7 @@ impl Server { index_object_args, touched_at, )?; - graph.graphs.insert( + graph_data.insert( id.unwrap_graph_ref().clone(), data.try_unwrap_graph().unwrap(), ); diff --git a/packages/server/src/checkin/cache.rs b/packages/server/src/checkin/cache.rs index 4159c0777..b16cb9832 100644 --- a/packages/server/src/checkin/cache.rs +++ b/packages/server/src/checkin/cache.rs @@ -1,7 +1,7 @@ use { crate::{ Server, - checkin::{Graph, IndexCacheEntryArgs, graph::Variant}, + checkin::{Graph, GraphData, IndexCacheEntryArgs, graph::Variant}, temp::Temp, }, futures::stream::{self, StreamExt as _, TryStreamExt as _}, @@ -9,14 +9,15 @@ use { collections::BTreeSet, os::unix::fs::PermissionsExt as _, path::{Path, PathBuf}, + pin::pin, }, tangram_client::prelude::*, - tangram_index::Index, tangram_util::{iter::Ext as _, path}, }; impl Server { #[tracing::instrument(level = "trace", skip_all)] + #[allow(clippy::too_many_arguments)] pub(super) async fn checkin_cache( &self, arg: &tg::checkin::Arg, @@ -24,12 +25,19 @@ impl Server { next: usize, root: &Path, index_cache_entry_args: &IndexCacheEntryArgs, + graph_data: &mut GraphData, progress: &crate::progress::Handle, ) -> tg::Result<()> { if arg.options.destructive { progress.spinner("checking", "checking"); - self.checkin_ensure_dependencies_are_cached(graph, root, index_cache_entry_args) - .await?; + self.checkin_ensure_dependencies_are_cached( + graph, + root, + index_cache_entry_args, + graph_data, + progress, + ) + .await?; progress.finish("checking"); progress.spinner("copying", "copying"); self.checkin_cache_destructive(graph, root).await?; @@ -203,6 +211,8 @@ impl Server { graph: &Graph, path: &Path, index_cache_entry_args: &IndexCacheEntryArgs, + graph_data: &mut GraphData, + progress: &crate::progress::Handle, ) -> tg::Result<()> { let root = graph .paths @@ -270,7 +280,7 @@ impl Server { if will_cache.contains(&artifact) { continue; } - let data = graph.graphs.get(id); + let data = graph_data.get(id); let ids = self.graph_ids(id, data).await.map_err(|source| { tg::error!(!source, "failed to get the graph ids") })?; @@ -310,7 +320,7 @@ impl Server { if will_cache.contains(&artifact) { continue; } - let data = graph.graphs.get(id); + let data = graph_data.get(id); let ids = self.graph_ids(id, data).await.map_err(|source| { tg::error!(!source, "failed to get the graph ids") @@ -347,7 +357,7 @@ impl Server { if will_cache.contains(&artifact) { continue; } - let data = graph.graphs.get(id); + let data = graph_data.get(id); let ids = self.graph_ids(id, data).await.map_err(|source| { tg::error!(!source, "failed to get the graph ids") })?; @@ -361,19 +371,20 @@ impl Server { }, } } - let touched_at = time::OffsetDateTime::now_utc().unix_timestamp(); - let all_cached = self - .index - .touch_cache_entries(&artifacts, touched_at) + + progress.spinner("dependencies", "caching dependencies"); + let stream = self + .cache(tg::cache::Arg { artifacts }) .await - .map_err(|source| { - tg::error!(!source, "failed to get the cache entries from the index") - })? - .into_iter() - .all(|entry| entry.is_some()); - if !all_cached { - return Err(tg::error!("missing cache dependency")); + .map_err(|source| tg::error!(!source, "failed to cache dependencies"))?; + let mut stream = pin!(stream); + while let Some(event) = stream.next().await { + if matches!(event, Ok(tg::progress::Event::Output(()))) { + break; + } + progress.forward(event); } + progress.finish("dependencies"); Ok(()) } diff --git a/packages/server/src/checkin/graph.rs b/packages/server/src/checkin/graph.rs index c921dbbdd..f31def269 100644 --- a/packages/server/src/checkin/graph.rs +++ b/packages/server/src/checkin/graph.rs @@ -11,7 +11,6 @@ use { #[derive(Clone, Debug, Default)] pub struct Graph { pub artifacts: im::HashMap, - pub graphs: im::HashMap, pub ids: im::HashMap, tg::id::BuildHasher>, pub next: usize, pub nodes: im::OrdMap>,