Skip to content

Commit

Permalink
simplification, get rid of Package dep in layout
Browse files Browse the repository at this point in the history
  • Loading branch information
boozook committed May 30, 2024
1 parent eb710e5 commit a2227e3
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 55 deletions.
2 changes: 1 addition & 1 deletion cargo/src/assets/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub fn build<'cfg>(config: &'cfg Config) -> CargoResult<AssetsArtifacts<'cfg>> {
for (package, targets, ..) in config.possible_targets()? {
let env = plan::LazyEnvBuilder::new(config, package);
let mut plans: HashMap<&Package, _> = Default::default();
let global_layout = CrossTargetLayout::new(config, package, None)?;
let global_layout = CrossTargetLayout::new(config, package.package_id(), None)?;
let mut layout = global_layout.assets_layout(config);
let mut options = HashMap::new();

Expand Down
5 changes: 3 additions & 2 deletions cargo/src/assets/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub fn plan_for<'cfg, 'env, 'l>(config: &'cfg Config,
let plan = assets_build_plan(env, metadata.assets(), opts.as_ref(), Some(root))?;

// main-assets plan:
let path = layout.as_inner().assets_plan_for(config, package);
let path = layout.as_inner().assets_plan_for(config, &package.package_id());
let mut cached = CachedPlan::new(path, plan)?;
if config.compile_options.build_config.force_rebuild {
cached.difference = Difference::Missing;
Expand All @@ -107,7 +107,8 @@ pub fn plan_for<'cfg, 'env, 'l>(config: &'cfg Config,
let assets = metadata.dev_assets();
let dev_plan = assets_build_plan(env, assets, opts.as_ref(), Some(root))?;

let path = layout.as_inner().assets_plan_for_dev(config, package);
let path = layout.as_inner()
.assets_plan_for_dev(config, &package.package_id());
let mut dev_cached = CachedPlan::new(path, dev_plan)?;

// Inheritance, if main is stale or missing - this one is too:
Expand Down
4 changes: 2 additions & 2 deletions cargo/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn new_or_init<'cfg>(config: &'cfg Config<'cfg>) -> CargoResult<()> {
for dep in deps_to_add {
// TODO call cargo add WITH PWD=path

let mut cargo = proc::cargo(config.workspace.config().into())?;
let mut cargo = proc::cargo(config.into())?;
cargo.current_dir(path);
cargo.arg("add");
cargo.arg(dep.as_ref());
Expand Down Expand Up @@ -242,7 +242,7 @@ fn cargo_add<'s>(config: &Config<'_>,
rename: Option<&str>,
features: Option<impl IntoIterator<Item = &'s str>>)
-> CargoResult<()> {
let mut cargo = proc::cargo(config.workspace.config().into())?;
let mut cargo = proc::cargo(config.into())?;
cargo.current_dir(pwd);

cargo.arg("add");
Expand Down
40 changes: 20 additions & 20 deletions cargo/src/layout/playdate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use std::path::Path;
use std::path::PathBuf;
use std::hash::{Hash, Hasher};

use cargo::core::PackageId;
use cargo::util::StableHasher;
use cargo_util::paths;
use cargo::CargoResult;
use cargo::core::Package;
use cargo::core::profiles::Profiles;
pub use playdate::layout::Name as TargetName;
use crate::config::Config;
Expand Down Expand Up @@ -59,10 +59,10 @@ impl ForTargetLayout<PathBuf> {


#[derive(Debug, Clone)]
pub struct CrossTargetLayout<'cfg> {
pub struct CrossTargetLayout {
/// Target name (e.g. `game-example-name`)
name: TargetName,
package: &'cfg Package,
package: PackageId,

/// The root directory: `/$target-dir/playdate`.
root: PathBuf,
Expand All @@ -76,20 +76,20 @@ pub struct CrossTargetLayout<'cfg> {
assets: PathBuf,
}

impl<'cfg> CrossTargetLayout<'cfg> {
pub fn new(config: &'cfg Config, package: &'cfg Package, name: Option<TargetName>) -> CargoResult<Self> {
impl<'cfg> CrossTargetLayout {
pub fn new(config: &'cfg Config, package_id: PackageId, name: Option<TargetName>) -> CargoResult<Self> {
let profiles = Profiles::new(
&config.workspace,
config.compile_options.build_config.requested_profile,
)?;
let profile = profiles.get_dir_name().as_str();
let name = name.unwrap_or_else(|| TargetName::with_package(package.name().as_str()));
let name = name.unwrap_or_else(|| TargetName::with_package(package_id.name().as_str()));
let root = config.workspace.target_dir().as_path_unlocked().join("playdate");
let dest = root.join(profile);
let target = dest.join(name.as_path());
let assets = root.join("assets");
Ok(Self { name,
package,
package: package_id,
root,
dest,
target,
Expand All @@ -101,11 +101,11 @@ impl<'cfg> CrossTargetLayout<'cfg> {

/// Global assets layout for cross-target & cross-profile assets build.
pub fn assets_layout(&self, config: &Config) -> PlaydateAssets<PathBuf> {
PlaydateAssets::global(self, config, self.package)
PlaydateAssets::global(self, config, &self.package)
}
}

impl Layout for CrossTargetLayout<'_> {
impl Layout for CrossTargetLayout {
fn root(&self) -> &Path { self.root.as_ref() }
fn dest(&self) -> Cow<Path> { self.dest.as_path().into() }

Expand All @@ -130,7 +130,7 @@ impl Layout for CrossTargetLayout<'_> {
}


impl playdate::layout::Layout for CrossTargetLayout<'_> {
impl playdate::layout::Layout for CrossTargetLayout {
fn name(&self) -> &TargetName { &self.name }
fn root(&self) -> &Path { self.dest.as_path() }
fn dest(&self) -> Cow<Path> { self.target.as_path().into() }
Expand All @@ -154,7 +154,7 @@ impl playdate::layout::Layout for CrossTargetLayout<'_> {
}
}

impl crate::layout::LayoutLockable for CrossTargetLayout<'_> {
impl crate::layout::LayoutLockable for CrossTargetLayout {
/// The lockfile filename for a build
fn lockfilename(&self) -> Cow<'static, str> { ".playdate-lock".into() }
}
Expand All @@ -170,20 +170,20 @@ pub struct PlaydateAssets<Path> {
}

impl PlaydateAssets<PathBuf> {
fn global(root: &CrossTargetLayout<'_>, config: &Config, package: &Package) -> Self {
let name = Self::name_for_package(config, package);
fn global(root: &CrossTargetLayout, config: &Config, package_id: &PackageId) -> Self {
let name = Self::name_for_package(config, package_id);
Self { name,
root: root.assets.to_owned() }
}

pub fn assets_plan_for(&self, config: &Config, package: &Package) -> PathBuf {
pub fn assets_plan_for(&self, config: &Config, package_id: &PackageId) -> PathBuf {
use playdate::layout::Layout;
let name = Self::name_for_package(config, package);
let name = Self::name_for_package(config, package_id);
self.assets_plan().with_file_name(name).with_extension("json")
}

pub fn assets_plan_for_dev(&self, config: &Config, package: &Package) -> PathBuf {
let mut path = self.assets_plan_for(config, package);
pub fn assets_plan_for_dev(&self, config: &Config, package_id: &PackageId) -> PathBuf {
let mut path = self.assets_plan_for(config, package_id);
let mut name = path.file_stem()
.map(std::ffi::OsStr::to_string_lossy)
.unwrap_or_default()
Expand All @@ -197,12 +197,12 @@ impl PlaydateAssets<PathBuf> {
path
}

fn name_for_package(config: &Config, package: &Package) -> TargetName {
fn name_for_package(config: &Config, package_id: &PackageId) -> TargetName {
let mut hasher = StableHasher::new();
let stable = package.package_id().stable_hash(config.workspace.root());
let stable = package_id.stable_hash(config.workspace.root());
stable.hash(&mut hasher);
let hash = hasher.finish();
TargetName::with_package(format!("{}-{hash:016x}", package.name()))
TargetName::with_package(format!("{}-{hash:016x}", package_id.name()))
}
}

Expand Down
79 changes: 69 additions & 10 deletions cargo/src/package/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use playdate::manifest::format::ManifestFmt;
use playdate::manifest::CrateInfoSource;
use playdate::metadata::format::Metadata;
use playdate::metadata::validation::Validate;
use playdate::metadata::validation::ValidateCrate;

use crate::assets::AssetsArtifact;
use crate::assets::AssetsArtifacts;
Expand Down Expand Up @@ -236,7 +237,8 @@ fn package_multi_target<'p>(config: &Config,
// cross-target layout:
let layout_target_name = Name::with_names(package.name().as_str(), products.first().map(|p| &p.name));
let mut layout =
CrossTargetLayout::new(config, package, Some(layout_target_name))?.lock(config.workspace.config())?;
CrossTargetLayout::new(config, package.package_id(), Some(layout_target_name))?.lock(config.workspace
.config())?;
if let Some(assets) = assets {
debug_assert_eq!(
layout.as_ref().assets_layout(config).root(),
Expand Down Expand Up @@ -321,24 +323,53 @@ fn build_manifest<Layout: playdate::layout::Layout>(config: &Config,
log.status("Manifest", msg);
});

use ::playdate::metadata::validation::Problem;
let log_problem = |pre: &str, problem: Problem| {
let msg = format!("{pre}: {problem}");
if problem.is_err() {
config.log().error(msg);
} else {
config.log().warn(msg);
}
};
let log_src_problem = |problem: Problem| {
let msg = format!("Manifest validation");
log_problem(&msg, problem)
};
let log_meta_problem = |problem: Problem| {
let msg = format!("Metadata validation");
log_problem(&msg, problem)
};

let validate = |src: &ManifestSource| {
if let Some(target) = &cargo_target {
src.validate_for(target)
.into_iter()
// .filter(Problem::is_err)
.for_each(log_meta_problem);
} else {
src.validate()
.into_iter()
// .filter(Problem::is_err)
.for_each(log_meta_problem);
}
};

let manifest = if let Some(metadata) = assets.and_then(|a| a.metadata.as_ref()) {
let source = ManifestSource::new(package, metadata.into());
// This validation not needed at this step. May be earlier:
validate(&source);
source.manifest_for_opt(cargo_target.as_deref(), dev)
} else {
let metadata = playdate_metadata(package);
let source = ManifestSource::new(package, metadata.as_ref());
// This validation not needed at this step. May be earlier:
validate(&source);
source.manifest_for_opt(cargo_target.as_deref(), dev)
};

// validation, lints
for problem in manifest.validate() {
let msg = format!("Manifest validation: {problem}");
if problem.is_err() {
config.log().error(msg);
} else {
config.log().warn(msg);
}
}
manifest.validate().into_iter().for_each(log_src_problem);

std::fs::write(layout.manifest(), manifest.to_manifest_string()?)?;
Ok(())
Expand Down Expand Up @@ -520,25 +551,47 @@ impl<'cfg> TryFrom<BuildProduct<'cfg>> for SuccessfulBuildProduct<'cfg> {
struct ManifestSource<'cfg, 'm> {
package: &'cfg Package,
authors: Vec<&'cfg str>,
bins: Vec<&'cfg str>,
examples: Vec<&'cfg str>,
metadata: Option<&'m Metadata>,
}

impl<'cfg, 'm> ManifestSource<'cfg, 'm> {
fn new(package: &'cfg Package, metadata: Option<&'m Metadata>) -> Self {
log::debug!("new manifest-source for {}", package.package_id());

let mut bins = Vec::new();
let mut examples = Vec::new();

package.targets()
.iter()
.inspect(|t| log::trace!("target: {} ({:?})", t.description_named(), t.crate_name()))
.filter(|t| !t.is_custom_build())
.for_each(|t| {
if t.is_bin() {
bins.push(t.name());
log::debug!("+bin: {}", t.description_named());
} else if t.is_example() {
examples.push(t.name());
log::debug!("+example: {}", t.description_named());
}
});

Self { authors: package.manifest()
.metadata()
.authors
.iter()
.map(|s| s.as_str())
.collect(),
bins,
examples,
package,
metadata }
}
}

impl CrateInfoSource for ManifestSource<'_, '_> {
fn name(&self) -> Cow<str> { self.package.name().as_str().into() }
// fn authors(&self) -> &[String] { &self.package.manifest().metadata().authors }
fn authors(&self) -> &[&str] { &self.authors }
fn version(&self) -> Cow<str> { self.package.version().to_string().into() }
fn description(&self) -> Option<Cow<str>> {
Expand All @@ -549,5 +602,11 @@ impl CrateInfoSource for ManifestSource<'_, '_> {
.as_deref()
.map(|s: &str| s.into())
}

fn bins(&self) -> &[&str] { &self.bins }
fn examples(&self) -> &[&str] { &self.examples }

fn metadata(&self) -> Option<impl playdate::metadata::source::MetadataSource> { self.metadata }

fn manifest_path(&self) -> Cow<Path> { Cow::Borrowed(self.package.manifest_path()) }
}
46 changes: 29 additions & 17 deletions cargo/src/proc/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::process::Command;

use build::consts::SDK_ENV_VAR;
use cargo::CargoResult;
use cargo::Config as CargoConfig;
use serde::de::DeserializeOwned;

use crate::cli::cmd::Cmd;
Expand All @@ -25,7 +24,7 @@ pub fn cargo_proxy_with<S: AsRef<OsStr>>(cfg: &Config,
cfg_args: bool)
-> CargoResult<std::process::Command> {
let rustflags = cfg.rustflags()?.rustflags_to_args_from(cfg);
let mut proc = cargo(Some(cfg.workspace.config()))?;
let mut proc = cargo(Some(cfg))?;
proc.arg(cmd);
if cfg_args {
proc.args(&cfg.args);
Expand All @@ -40,11 +39,10 @@ pub fn cargo_proxy_with<S: AsRef<OsStr>>(cfg: &Config,
}


pub fn cargo(config: Option<&CargoConfig>) -> CargoResult<std::process::Command> {
let cargo = cargo_bin_path(config);
let mut proc = std::process::Command::new(cargo.as_ref());
pub fn cargo(cfg: Option<&Config>) -> CargoResult<std::process::Command> {
let mut proc = cargo_cmd(cfg);

if let Some(cfg) = &config {
if let Some(cfg) = cfg.map(|cfg| cfg.workspace.config()) {
// transparent env:
cfg.env_config()?.iter().for_each(|(k, v)| {
let value = v.resolve(cfg);
Expand All @@ -71,20 +69,34 @@ pub fn cargo(config: Option<&CargoConfig>) -> CargoResult<std::process::Command>
}


pub fn cargo_bin_path(config: Option<&CargoConfig>) -> Cow<Path> {
if let Some(cfg) = config {
let path = cfg.cargo_exe().log_err().ok().map(Cow::from);
if path.is_some() && path == std::env::current_exe().log_err().ok().map(Into::into) {
// Seems to we're in standalone mode.
cargo_bin_path(None)
} else if let Some(path) = path {
path
pub fn cargo_cmd(cfg: Option<&Config>) -> std::process::Command {
fn cargo_path<'t>(cfg: Option<&'t Config<'t>>) -> (Cow<'t, Path>, Option<&str>) {
if let Some(cfg) = cfg {
let path = cfg.workspace.config().cargo_exe().log_err().ok().map(Cow::from);
if path.is_some() && path == std::env::current_exe().log_err().ok().map(Into::into) {
// Seems to we're in standalone mode.
cargo_path(None)
} else if let Some(path) = path {
(path, None)
} else {
cargo_path(None)
}
} else if let Some(path) = env::var_os("CARGO") {
(PathBuf::from(path).into(), None)
} else {
cargo_bin_path(None)
let arg = cfg.and_then(|cfg| cfg.rustup.as_os_str()).map(|_| "+nightly");
(Path::new("cargo").into(), arg)
}
} else {
PathBuf::from(env::var_os("CARGO").unwrap_or("cargo".into())).into()
}

let (bin, arg) = cargo_path(cfg);

let mut proc = std::process::Command::new(bin.as_ref());
if let Some(arg) = arg {
proc.arg(arg);
}

proc
}


Expand Down
Loading

0 comments on commit a2227e3

Please sign in to comment.