fix: detect presence of correct artifacts directory when collecting i…

…nput, remove dead code

packages/server/src/artifact/checkin/input.rs

@@ -44,36 +44,29 @@ struct State {
 impl Server {
 	pub(super) async fn create_input_graph(
 		&self,
-		arg: tg::artifact::checkin::Arg,
+		mut arg: tg::artifact::checkin::Arg,
 		progress: Option<&crate::progress::Handle<tg::artifact::checkin::Output>>,
 	) -> tg::Result<Graph> {
-		// Make a best guess for the input.
-		let path = self.find_root(&arg.path).await.map_err(
+		// Find the root path.
+		arg.path = self.find_root(&arg.path).await.map_err(
 			|source| tg::error!(!source, %path = arg.path.display(), "failed to find root path for checkin"),
 		)?;
 
-		// Create a graph.
-		let mut graph = 'a: {
-			let arg_ = tg::artifact::checkin::Arg {
-				path,
-				..arg.clone()
-			};
-			let graph = self.collect_input(arg_, progress).await?;
+		// Create the ignore tree.
+		let ignore = self.ignore_for_checkin().await?;
 
-			// Check if the graph contains the path.
-			if graph.contains_path(&arg.path).await {
-				break 'a graph;
-			}
+		// Initialize state.
+		let state = RwLock::new(State {
+			ignore,
+			roots: Vec::new(),
+			visited: BTreeMap::new(),
+			graph: Graph { nodes: Vec::new() },
+		});
 
-			// Otherwise use the parent.
-			let path = arg
-				.path
-				.parent()
-				.ok_or_else(|| tg::error!("cannot check in root"))?
-				.to_owned();
-			let arg_ = tg::artifact::checkin::Arg { path, ..arg };
-			self.collect_input(arg_, progress).await?
-		};
+		// Create the graph.
+		self.create_input_graph_inner(None, arg.path.as_ref(), &arg, &state, progress)
+			.await?;
+		let State { mut graph, .. } = state.into_inner();
 
 		// Validate the input graph.
 		graph.validate().await?;
@@ -109,31 +102,7 @@ impl Server {
 		Ok(path.to_owned())
 	}
 
-	async fn collect_input(
-		&self,
-		arg: tg::artifact::checkin::Arg,
-		progress: Option<&crate::progress::Handle<tg::artifact::checkin::Output>>,
-	) -> tg::Result<Graph> {
-		// Create the ignore tree.
-		let ignore = self.ignore_for_checkin().await?;
-
-		// Initialize state.
-		let state = RwLock::new(State {
-			ignore,
-			roots: Vec::new(),
-			visited: BTreeMap::new(),
-			graph: Graph { nodes: Vec::new() },
-		});
-
-		// Collect input.
-		self.collect_input_inner(None, arg.path.as_ref(), &arg, &state, progress)
-			.await?;
-
-		let State { graph, .. } = state.into_inner();
-		Ok(graph)
-	}
-
-	async fn collect_input_inner(
+	async fn create_input_graph_inner(
 		&self,
 		referrer: Option<usize>,
 		path: &Path,
@@ -211,7 +180,7 @@ impl Server {
 
 		// Collect any dangling directories.
 		for dangling in dangling {
-			Box::pin(self.collect_input_inner(None, &dangling, arg, state, progress))
+			Box::pin(self.create_input_graph_inner(None, &dangling, arg, state, progress))
 				.await
 				.map_err(
 					|source| tg::error!(!source, %path = dangling.display(), "failed to collect dangling directory"),
@@ -221,7 +190,7 @@ impl Server {
 		// If this is a module file, ensure the parent is collected.
 		if metadata.is_file() && tg::package::is_module_path(&absolute_path) {
 			let parent = absolute_path.parent().unwrap().to_owned();
-			let parent = Box::pin(self.collect_input_inner(None, &parent, arg, state, progress))
+			let parent = Box::pin(self.create_input_graph_inner(None, &parent, arg, state, progress))
 				.await
 				.map_err(
 					|source| tg::error!(!source, %path = absolute_path.display(), "failed to collect input of parent"),
@@ -317,7 +286,7 @@ impl Server {
 					let path = path.join(&name);
 
 					// Follow the edge.
-					let node = Box::pin(self.collect_input_inner(
+					let node = Box::pin(self.create_input_graph_inner(
 						Some(referrer),
 						name.as_ref(),
 						arg,
@@ -506,15 +475,15 @@ impl Server {
 						// Get the input of the referent.
 						let child = if is_external {
 							// If this is an external import, treat it as a root using the absolute path.
-							self.collect_input_inner(Some(referrer), &absolute_path, &arg, state, progress).await.map_err(|source| tg::error!(!source, "failed to collect child input"))?
+							self.create_input_graph_inner(Some(referrer), &absolute_path, &arg, state, progress).await.map_err(|source| tg::error!(!source, "failed to collect child input"))?
 						} else {
 							// Otherwise, treat it as a relative path.
 
 							// Get the parent of the referrer.
 							let parent = state.read().await.graph.nodes[referrer].parent.ok_or_else(|| tg::error!("expected a parent"))?;
 
 							// Recurse.
-							self.collect_input_inner(Some(parent), import_path.as_ref(), &arg, state, progress).await.map_err(|source| tg::error!(!source, "failed to collect child input"))?
+							self.create_input_graph_inner(Some(parent), import_path.as_ref(), &arg, state, progress).await.map_err(|source| tg::error!(!source, "failed to collect child input"))?
 						};
 
 
@@ -610,8 +579,20 @@ impl Server {
 		)?;
 
 		// Check if this is a checkin of an artifact.
+		let artifacts_path = {
+			let _permit = self.file_descriptor_semaphore.acquire().await.unwrap();
+			let mut artifacts_path = None;
+			for ancestor in path.ancestors().skip(1) {
+				let path = ancestor.join(".tangram/artifacts");
+				if matches!(tokio::fs::try_exists(&path).await, Ok(true)) {
+					artifacts_path.replace(path);
+					break;
+				}
+			}
+			artifacts_path.unwrap_or(self.artifacts_path())
+		};
 		let path = crate::util::path::diff(&arg.path, &target_absolute_path)?;
-		let edge = if let Ok(subpath) = target_absolute_path.strip_prefix(self.artifacts_path()) {
+		let edge = if let Ok(subpath) = target_absolute_path.strip_prefix(artifacts_path) {
 			let mut components = subpath.components();
 			let object = components
 				.next()
@@ -701,15 +682,6 @@ impl Server {
 }
 
 impl Graph {
-	async fn contains_path(&self, path: &Path) -> bool {
-		for node in &self.nodes {
-			if node.arg.path == path {
-				return true;
-			}
-		}
-		false
-	}
-
 	#[allow(dead_code)]
 	pub(super) async fn print(&self) {
 		let mut visited = BTreeSet::new();

packages/server/src/artifact/checkin/output.rs

@@ -715,8 +715,22 @@ impl Server {
 		output: &Graph,
 		symlink: usize,
 	) -> tg::Result<PathBuf> {
-		// Get the outgoing edge.
-		if let Some(output_edge) = output.nodes[symlink].edges.first() {
+		// Get the artifact and subpath, if they exist..
+		let artifact_and_subpath = output.nodes[symlink]
+			.edges
+			.first()
+			.map(|edge| (output.nodes[edge.node].id.clone(), edge.subpath.clone()))
+			.or_else(|| {
+				let tg::artifact::Data::Symlink(tg::symlink::Data::Normal {
+					artifact: Some(artifact),
+					subpath,
+				}) = output.nodes[symlink].data.clone()
+				else {
+					return None;
+				};
+				Some((artifact, subpath))
+			});
+		if let Some((artifact, subpath)) = artifact_and_subpath {
 			// Find how deep this node is in the output tree.
 			let mut input_index = output.nodes[symlink].input;
 			let mut depth = 0;
@@ -729,8 +743,8 @@ impl Server {
 			}
 
 			// Create the target.
-			let id = output.nodes[output_edge.node].id.to_string();
-			let subpath = output_edge.subpath.clone().unwrap_or_default();
+			let id = artifact.to_string();
+			let subpath = subpath.unwrap_or_default();
 
 			let components = (0..depth)
 				.map(|_| std::path::Component::ParentDir)
@@ -739,16 +753,19 @@ impl Server {
 				))))
 				.chain(subpath.components());
 
-			Ok(components.collect())
-		} else if let tg::artifact::Data::Symlink(tg::symlink::Data::Normal {
+			return Ok(components.collect());
+		}
+
+		// Otherwise write the target directly.
+		if let tg::artifact::Data::Symlink(tg::symlink::Data::Normal {
 			artifact: None,
 			subpath: Some(target),
 		}) = &output.nodes[symlink].data
 		{
-			Ok(target.clone())
-		} else {
-			return Err(tg::error!("invalid symlink"));
+			return Ok(target.clone());
 		}
+
+		Err(tg::error!(?symlink = &output.nodes[symlink], "invalid symlink"))
 	}
 }
 

packages/server/src/artifact/checkin/tests.rs

@@ -1,7 +1,7 @@
 use crate::{util::fs::cleanup, Config, Server};
 use futures::{Future, FutureExt as _};
 use insta::assert_snapshot;
-use std::{panic::AssertUnwindSafe, pin::pin};
+use std::{panic::AssertUnwindSafe, path::PathBuf, pin::pin};
 use tangram_client as tg;
 use tangram_futures::stream::TryStreamExt as _;
 use tangram_temp::{self as temp, Temp};
@@ -633,6 +633,50 @@ async fn destructive_package() -> tg::Result<()> {
 	.await
 }
 
+#[tokio::test]
+async fn external_symlink_roundtrip() -> tg::Result<()> {
+	let temp = Temp::new();
+	let config = Config::with_path(temp.path().to_owned());
+	let server = Server::start(config).await?;
+	let result = AssertUnwindSafe(async {
+		let directory = tg::directory! {
+			"file" => "contents",
+		}
+		.into();
+		let symlink = tg::symlink!(Some(directory), Some(PathBuf::from("file")));
+
+		// Check out the artifact.
+		let temp = Temp::new();
+		let arg = tg::artifact::checkout::Arg {
+			dependencies: true,
+			force: false,
+			path: Some(temp.path().to_owned()),
+		};
+		let path = tg::Artifact::from(symlink.clone())
+			.check_out(&server, arg)
+			.await?;
+
+		// Check the artifact back in.
+		let arg = tg::artifact::checkin::Arg {
+			deterministic: true,
+			destructive: false,
+			ignore: true,
+			locked: true,
+			path,
+		};
+		let artifact = tg::Artifact::check_in(&server, arg)
+			.await?
+			.try_unwrap_symlink()
+			.map_err(|_| tg::error!("expected a symlink"))?;
+		assert_eq!(artifact.id(&server).await?, symlink.id(&server).await?);
+		Ok::<_, tg::Error>(())
+	})
+	.catch_unwind()
+	.await;
+	cleanup(temp, server).await;
+	result.unwrap()
+}
+
 async fn test<F, Fut>(
 	artifact: temp::Artifact,
 	path: &str,

packages/server/src/artifact/checkout.rs

@@ -218,10 +218,9 @@ impl Server {
 			let artifact_path = self.artifacts_path().join(id.to_string());
 
 			// If there is already a file system object at the path, then return.
-			if tokio::fs::try_exists(&path)
-				.await
-				.map_err(|source| tg::error!(!source, %path = path.display(), "failed to stat the path"))?
-			{
+			if tokio::fs::try_exists(&path).await.map_err(
+				|source| tg::error!(!source, %path = path.display(), "failed to stat the path"),
+			)? {
 				let output = tg::artifact::checkout::Output {
 					path: artifact_path,
 				};