Merge pull request #339 from boozook/fix-assets-missed-crate-root

Fix assets missed crate root (#338, #319)
boozook · May 4, 2024 · bc61d14 · bc61d14
2 parents e5a1b1e + f02701c
commit bc61d14
Show file tree

Hide file tree

Showing 10 changed files with 821 additions and 51 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -193,7 +193,7 @@ jobs:
       - name: Test
         run: |
           cargo test -p=playdate-build-utils --all-features
-          cargo test -p=playdate-build --all-features
+          cargo test -p=playdate-build --all-features -- --nocapture
           cargo test -p=playdate-device
           cargo test -p=playdate-tool --all-features
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cargo/Cargo.toml b/cargo/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "cargo-playdate"
-version = "0.4.9"
+version = "0.4.10"
 readme = "README.md"
 description = "Build tool for neat yellow console."
 keywords = ["playdate", "build", "cargo", "plugin", "cargo-subcommand"]

diff --git a/cargo/src/assets/mod.rs b/cargo/src/assets/mod.rs
@@ -214,11 +214,21 @@ pub fn build<'cfg>(config: &'cfg Config) -> CargoResult<AssetsArtifacts<'cfg>> {
 						AssetKind::Package => "",
 						AssetKind::Dev => "dev-",
 					};
+
+					let dep_root = dependency.manifest_path().parent().unwrap();
+
 					config.log()
 					      .status("Build", format!("{kind_prefix}assets for {}", dep_pkg_id));
 					config.log().verbose(|mut log| {
-						            let s = format!("destination: {}", dest.as_relative_to_root(config).display());
-						            log.status("", s)
+						            let dest = format!("destination: {}", dest.as_relative_to_root(config).display());
+						            log.status("", dest);
+						            let src = format!("root {}", dep_root.as_relative_to_root(config).display());
+						            log.status("", src);
+						            if dep_root != &plan.path {
+							            let path = plan.plan.crate_root();
+							            let src = format!("root (plan) {}", path.as_relative_to_root(config).display());
+							            log.status("", src);
+						            }
 					            });
 
 

diff --git a/cargo/src/assets/plan.rs b/cargo/src/assets/plan.rs
@@ -166,8 +166,8 @@ pub struct CachedPlan<'t, 'cfg> {
 impl<'t, 'cfg> CachedPlan<'t, 'cfg> {
 	#[must_use = "Cached plan must be used"]
 	fn new(path: PathBuf, plan: AssetsPlan<'t, 'cfg>) -> CargoResult<Self> {
-		let mut serializable = plan.serializable_flatten().collect::<Vec<_>>();
-		serializable.sort_by_key(|(_, (p, _))| p.to_string_lossy().to_string());
+		let mut serializable = plan.iter_flatten().collect::<Vec<_>>();
+		serializable.sort_by_key(|(_, _, (p, _))| p.to_string_lossy().to_string());
 		let json = serde_json::to_string(&serializable)?;
 
 		let difference = if path.try_exists()? {

diff --git a/support/build/Cargo.toml b/support/build/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "playdate-build"
-version = "0.2.5"
+version = "0.2.6"
 readme = "README.md"
 description = "Utils that help to build package for Playdate"
 keywords = ["playdate", "package", "encoding", "manifest", "assets"]

diff --git a/support/build/src/assets/mod.rs b/support/build/src/assets/mod.rs
@@ -12,6 +12,7 @@ use crate::metadata::format::AssetsOptions;
 
 pub mod plan;
 pub mod resolver;
+mod tests;
 
 use self::plan::*;
 
@@ -121,14 +122,23 @@ pub fn apply_build_plan<'l, 'r, P: AsRef<Path>>(plan: BuildPlan<'l, 'r>,
 		AssetsBuildMethod::Link => &link_method,
 	};
 
-	let mut results = HashMap::with_capacity(plan.as_inner().len());
-	for entry in plan.into_inner().drain(..) {
+	let (mut plan, crate_root) = plan.into_parts();
+	// let mut results = HashMap::with_capacity(plan.as_inner().len());
+	let mut results = HashMap::with_capacity(plan.len());
+	// for entry in plan.into_inner().drain(..) {
+	for entry in plan.drain(..) {
 		let current: Vec<_> = match &entry {
-			Mapping::AsIs(inc, ..) => vec![method(&inc.source(), &inc.target(), false)],
-			Mapping::Into(inc, ..) => vec![method(&inc.source(), &inc.target(), true)],
+			Mapping::AsIs(inc, ..) => {
+				let source = abs_or_rel_crate_any(inc.source(), crate_root);
+				vec![method(&source, &inc.target(), false)]
+			},
+			Mapping::Into(inc, ..) => {
+				let source = abs_or_rel_crate_any(inc.source(), crate_root);
+				vec![method(&source, &inc.target(), true)]
+			},
 			Mapping::ManyInto { sources, target, .. } => {
 				sources.iter()
-				       .map(|inc| (inc.source(), target.join(inc.target())))
+				       .map(|inc| (abs_or_rel_crate_any(inc.source(), crate_root), target.join(inc.target())))
 				       .map(|(ref source, ref target)| method(source, target, false))
 				       .collect()
 			},

diff --git a/support/build/src/assets/plan.rs b/support/build/src/assets/plan.rs
@@ -1,7 +1,7 @@
 use std::hash::Hash;
 use std::borrow::Cow;
 use std::str::FromStr;
-use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR};
+use std::path::{Path, PathBuf, MAIN_SEPARATOR};
 
 use wax::{Glob, Pattern};
 
@@ -17,7 +17,7 @@ use super::resolver::*;
 pub fn build_plan<'l, 'r, 'c: 'l, V>(env: &'c Env,
                                      assets: &PlayDateMetadataAssets<V>,
                                      options: &AssetsOptions,
-                                     crate_root: Option<&Path>)
+                                     crate_root: Option<&'c Path>)
                                      -> Result<BuildPlan<'l, 'r>, super::Error>
 	where V: Value
 {
@@ -32,14 +32,22 @@ pub fn build_plan<'l, 'r, 'c: 'l, V>(env: &'c Env,
 	let mut include_unresolved = Vec::new();
 	let mut exclude_exprs = Vec::new();
 
+	const PATH_SEPARATOR: [char; 2] = [MAIN_SEPARATOR, '/'];
+
 	let enver = EnvResolver::new();
 	let crate_root = crate_root.unwrap_or_else(|| env.cargo_manifest_dir());
 	let link_behavior = options.link_behavior();
 
 	let to_relative = |s: &String| -> String {
-		let p = Path::new(&s);
+		let p = Path::new(s);
 		if p.is_absolute() || p.has_root() {
-			p.components().skip(1).collect::<PathBuf>().display().to_string()
+			let trailing_sep = p.components().count() > 1 && s.ends_with(PATH_SEPARATOR);
+			let mut s = p.components().skip(1).collect::<PathBuf>().display().to_string();
+			// preserve trailing separator
+			if trailing_sep && !s.ends_with(PATH_SEPARATOR) {
+				s.push(MAIN_SEPARATOR);
+			}
+			unixish_path_pattern(&s).into_owned()
 		} else {
 			s.to_owned()
 		}
@@ -87,16 +95,18 @@ pub fn build_plan<'l, 'r, 'c: 'l, V>(env: &'c Env,
 	let mut mappings = Vec::new();
 	for (k, v) in map_unresolved.into_iter() {
 		let key = PathBuf::from(k.as_str());
-		let value = v.as_str();
-		let into_dir = k.as_str().ends_with(MAIN_SEPARATOR_STR);
-		let source_exists = Path::new(value).try_exists()?;
+		let value = Cow::Borrowed(v.as_str());
+		let into_dir = k.as_str().ends_with(PATH_SEPARATOR);
+		let source_exists = abs_or_rel_crate_existing(Path::new(value.as_ref()), crate_root)?.is_some();
 
 		let mapping = match (source_exists, into_dir) {
-			(true, true) => Mapping::Into(Match::new(value, key), (k, v)),
-			(true, false) => Mapping::AsIs(Match::new(value, key), (k, v)),
+			(true, true) => Mapping::Into(Match::new(value.as_ref(), key), (k, v)),
+			(true, false) => Mapping::AsIs(Match::new(value.as_ref(), key), (k, v)),
 			(false, _) => {
 				let mut resolved = resolve_includes(value, crate_root, &exclude_exprs, link_behavior)?;
 
+				debug!("Possible ManyInto, resolved: {}", resolved.len());
+
 				// filter resolved includes:
 				let _excluded: Vec<_> = resolved.extract_if(|inc| {
 					                                let path = key.join(inc.target());
@@ -185,7 +195,37 @@ pub fn build_plan<'l, 'r, 'c: 'l, V>(env: &'c Env,
 
 	// TODO: find source duplicates and warn!
 
-	Ok(BuildPlan(mappings))
+	Ok(BuildPlan { plan: mappings,
+	               crate_root })
+}
+
+
+/// Make path relative to `crate_root` if it isn't absolute, checking existence.
+/// Returns `None` if path doesn't exist.
+pub fn abs_or_rel_crate_existing<'t, P1, P2>(p: P1, root: P2) -> std::io::Result<Option<Cow<'t, Path>>>
+	where P1: 't + AsRef<Path> + Into<Cow<'t, Path>>,
+	      P2: AsRef<Path> {
+	let p = if p.as_ref().is_absolute() && p.as_ref().try_exists()? {
+		Some(p.into())
+	} else {
+		let abs = root.as_ref().join(p);
+		if abs.try_exists()? {
+			Some(Cow::Owned(abs))
+		} else {
+			None
+		}
+	};
+	Ok(p)
+}
+
+/// Same as [`abs_or_rel_crate_existing`], but returns given `p` as fallback.
+#[inline]
+pub fn abs_or_rel_crate_any<'t, P1, P2>(p: P1, root: P2) -> Cow<'t, Path>
+	where P1: 't + AsRef<Path> + Into<Cow<'t, Path>> + Clone,
+	      P2: AsRef<Path> {
+	abs_or_rel_crate_existing(p.clone(), root).ok()
+	                                          .flatten()
+	                                          .unwrap_or(p.into())
 }
 
 
@@ -223,15 +263,26 @@ fn possibly_matching<P: Into<PathBuf>>(path: &Path, expr: P) -> bool {
 
 
 #[derive(Debug, PartialEq, Eq, Hash, serde::Serialize)]
-pub struct BuildPlan<'left, 'right>(Vec<Mapping<'left, 'right>>);
+pub struct BuildPlan<'left, 'right> {
+	plan: Vec<Mapping<'left, 'right>>,
+	crate_root: &'left Path,
+}
 
 impl<'left, 'right> BuildPlan<'left, 'right> {
-	pub fn into_inner(self) -> Vec<Mapping<'left, 'right>> { self.0 }
-	pub fn as_inner(&self) -> &[Mapping<'left, 'right>] { &self.0[..] }
+	pub fn into_inner(self) -> Vec<Mapping<'left, 'right>> { self.plan }
+	pub fn as_inner(&self) -> &[Mapping<'left, 'right>] { &self.plan[..] }
+	pub fn into_parts(self) -> (Vec<Mapping<'left, 'right>>, &'left Path) { (self.plan, self.crate_root) }
+
+	pub fn crate_root(&self) -> &Path { self.crate_root }
+	pub fn set_crate_root(&mut self, path: &'left Path) -> &Path {
+		let old = self.crate_root;
+		self.crate_root = path;
+		old
+	}
 }
 
 impl<'left, 'right> AsRef<[Mapping<'left, 'right>]> for BuildPlan<'left, 'right> {
-	fn as_ref(&self) -> &[Mapping<'left, 'right>] { &self.0[..] }
+	fn as_ref(&self) -> &[Mapping<'left, 'right>] { &self.plan[..] }
 }
 
 impl BuildPlan<'_, '_> {
@@ -282,27 +333,30 @@ impl BuildPlan<'_, '_> {
 		                      })
 	}
 
-	pub fn serializable_flatten(
+	pub fn iter_flatten(
 		&self)
-		-> impl Iterator<Item = (PathBuf, (PathBuf, Option<std::time::SystemTime>))> + '_ {
-		let pair = |inc: &Match| (inc.target().to_path_buf(), inc.source().to_path_buf());
+		-> impl Iterator<Item = (MappingKind, PathBuf, (PathBuf, Option<std::time::SystemTime>))> + '_ {
+		let pair = |inc: &Match| {
+			(inc.target().to_path_buf(), abs_or_rel_crate_any(inc.source(), self.crate_root).to_path_buf())
+		};
 
 		self.as_inner()
 		    .iter()
 		    .flat_map(move |mapping| {
 			    let mut rows = Vec::new();
+			    let kind = mapping.kind();
 			    match mapping {
 				    Mapping::AsIs(inc, _) | Mapping::Into(inc, _) => rows.push(pair(inc)),
 			       Mapping::ManyInto { sources, target, .. } => {
 				       rows.extend(sources.iter()
 				                          .map(|inc| pair(&Match::new(inc.source(), target.join(inc.target())))));
 			       },
 			    };
-			    rows.into_iter()
+			    rows.into_iter().map(move |(l, r)| (kind, l, r))
 		    })
-		    .map(|(t, p)| {
+		    .map(|(k, t, p)| {
 			    let time = p.metadata().ok().and_then(|m| m.modified().ok());
-			    (t, (p, time))
+			    (k, t, (p, time))
 		    })
 	}
 }
@@ -366,4 +420,33 @@ impl Mapping<'_, '_> {
 			Mapping::ManyInto { sources, .. } => sources.iter().collect(),
 		}
 	}
+
+	pub fn kind(&self) -> MappingKind {
+		match self {
+			Mapping::AsIs(..) => MappingKind::AsIs,
+			Mapping::Into(..) => MappingKind::Into,
+			Mapping::ManyInto { .. } => MappingKind::ManyInto,
+		}
+	}
+}
+
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize)]
+pub enum MappingKind {
+	/// Copy source __to__ target.
+	AsIs,
+	/// Copy source __into__ target as-is, preserving related path.
+	Into,
+	/// Copy sources __into__ target as-is, preserving matched path.
+	ManyInto,
+}
+
+impl std::fmt::Display for MappingKind {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		match self {
+			Self::AsIs => "as-is".fmt(f),
+			Self::Into => "into".fmt(f),
+			Self::ManyInto => "many-into".fmt(f),
+		}
+	}
 }