From 5440caf7db05e1d192596332ec3a3d2c4441b6b7 Mon Sep 17 00:00:00 2001 From: Konstantin Shabanov Date: Fri, 11 Nov 2022 15:24:59 +0600 Subject: [PATCH 1/2] Introduce spin_loader::local::{absolutize, parent_dir} Extract repetitive patterns into fns. Probably this isn't the best place to put them, but there is no decision about the introduction of some util-like library (yet). Signed-off-by: Konstantin Shabanov --- Cargo.lock | 3 --- crates/build/Cargo.toml | 1 - crates/build/src/lib.rs | 17 +++++++-------- crates/cloud/Cargo.toml | 1 - crates/loader/src/bindle/assets.rs | 3 ++- crates/loader/src/local/assets.rs | 7 ++++-- crates/loader/src/local/mod.rs | 33 +++++++++++++++++++++-------- crates/publish/Cargo.toml | 1 - crates/publish/src/bindle_writer.rs | 3 ++- crates/publish/src/expander.rs | 10 +++------ crates/publish/src/lib.rs | 17 --------------- src/commands/deploy.rs | 4 ++-- src/commands/new.rs | 11 ++++------ src/lib.rs | 18 +--------------- 14 files changed, 51 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a183046ef6..3347459cfb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -529,7 +529,6 @@ dependencies = [ "itertools", "log", "mime_guess", - "path-absolutize", "regex", "reqwest", "semver 1.0.14", @@ -4039,7 +4038,6 @@ version = "0.6.0" dependencies = [ "anyhow", "futures", - "path-absolutize", "spin-loader", "subprocess", "tracing", @@ -4255,7 +4253,6 @@ dependencies = [ "futures", "itertools", "mime_guess", - "path-absolutize", "reqwest", "semver 1.0.14", "serde", diff --git a/crates/build/Cargo.toml b/crates/build/Cargo.toml index b3dcdc58d4..7b7027dad6 100644 --- a/crates/build/Cargo.toml +++ b/crates/build/Cargo.toml @@ -7,7 +7,6 @@ edition = { workspace = true } [dependencies] anyhow = "1.0.57" futures = "0.3.21" -path-absolutize = "3.0.11" spin-loader = { path = "../loader" } subprocess = "0.2.8" tracing = { version = "0.1", features = [ "log" ] } diff --git a/crates/build/src/lib.rs b/crates/build/src/lib.rs index 155b7a72a6..8c4102d14b 100644 --- a/crates/build/src/lib.rs +++ b/crates/build/src/lib.rs @@ -3,15 +3,18 @@ //! A library for building Spin components. use anyhow::{bail, Context, Result}; -use path_absolutize::Absolutize; -use spin_loader::local::config::{RawAppManifest, RawComponentManifest}; +use spin_loader::local::{ + absolutize, + config::{RawAppManifest, RawComponentManifest}, + parent_dir, +}; use std::path::{Path, PathBuf}; use subprocess::{Exec, Redirection}; use tracing::log; /// If present, run the build command of each component. pub async fn build(app: RawAppManifest, src: &Path) -> Result<()> { - let src = src.absolutize()?; + let src = absolutize(src)?; if app.components.iter().all(|c| c.build.is_none()) { println!("No build command found!"); @@ -81,11 +84,7 @@ async fn build_component(raw: RawComponentManifest, src: impl AsRef) -> Re /// Constructs the absolute working directory in which to run the build command. fn construct_workdir(src: impl AsRef, workdir: Option>) -> Result { - let mut cwd = src - .as_ref() - .parent() - .context("The application file did not have a parent directory.")? - .to_path_buf(); + let mut cwd = parent_dir(src)?; if let Some(workdir) = workdir { // Using `Path::has_root` as `is_relative` and `is_absolute` have @@ -95,7 +94,7 @@ fn construct_workdir(src: impl AsRef, workdir: Option>) - bail!("The workdir specified in the application file must be relative."); } cwd.push(workdir); - cwd = cwd.absolutize()?.to_path_buf(); + cwd = absolutize(cwd)?; } Ok(cwd) diff --git a/crates/cloud/Cargo.toml b/crates/cloud/Cargo.toml index 0e7cd63914..769bde42a7 100644 --- a/crates/cloud/Cargo.toml +++ b/crates/cloud/Cargo.toml @@ -19,7 +19,6 @@ cloud-openapi = { git = "https://github.com/fermyon/cloud-openapi" } itertools = "0.10.0" log = "0.4" mime_guess = { version = "2.0" } -path-absolutize = "3.0.11" regex = "1.5" reqwest = { version = "0.11", features = ["stream"] } semver = "1.0" diff --git a/crates/loader/src/bindle/assets.rs b/crates/loader/src/bindle/assets.rs index b871004c28..3c443c5897 100644 --- a/crates/loader/src/bindle/assets.rs +++ b/crates/loader/src/bindle/assets.rs @@ -14,6 +14,7 @@ use crate::digest::file_sha256_string; use crate::{ assets::{create_dir, ensure_under}, bindle::utils::BindleReader, + local::parent_dir, }; pub(crate) async fn prepare_component( @@ -88,7 +89,7 @@ impl Copier { p.sha256, to.display() ); - fs::create_dir_all(to.parent().expect("Cannot copy to file '/'")).await?; + fs::create_dir_all(parent_dir(&to).expect("Cannot copy to file '/'")).await?; let mut stream = self .reader .get_parcel_stream(&p.sha256) diff --git a/crates/loader/src/local/assets.rs b/crates/loader/src/local/assets.rs index 147d3d58aa..a38f0f0de6 100644 --- a/crates/loader/src/local/assets.rs +++ b/crates/loader/src/local/assets.rs @@ -1,6 +1,9 @@ #![deny(missing_docs)] -use crate::assets::{create_dir, ensure_all_under, ensure_under, to_relative}; +use crate::{ + assets::{create_dir, ensure_all_under, ensure_under, to_relative}, + local::parent_dir, +}; use anyhow::{anyhow, bail, ensure, Context, Result}; use futures::{future, stream, StreamExt}; use spin_manifest::DirectoryMount; @@ -206,7 +209,7 @@ async fn copy(file: &FileMount, dir: impl AsRef) -> Result<()> { tracing::trace!("Copying asset file '{from:?}' -> '{to:?}'"); - tokio::fs::create_dir_all(to.parent().context("Cannot copy to file '/'")?).await?; + tokio::fs::create_dir_all(parent_dir(&to).context("Cannot copy to file '/'")?).await?; let _ = tokio::fs::copy(&from, &to) .await diff --git a/crates/loader/src/local/mod.rs b/crates/loader/src/local/mod.rs index 3c4ff90512..0732464235 100644 --- a/crates/loader/src/local/mod.rs +++ b/crates/loader/src/local/mod.rs @@ -41,10 +41,7 @@ pub async fn from_file( base_dst: impl AsRef, bindle_connection: &Option, ) -> Result { - let app = app - .as_ref() - .absolutize() - .context("Failed to resolve absolute path to manifest file")?; + let app = absolutize(app)?; let manifest = raw_manifest_from_file(&app).await?; validate_raw_app_manifest(&manifest)?; @@ -67,6 +64,28 @@ pub async fn raw_manifest_from_file(app: &impl AsRef) -> Result) -> Result { + let path_buf = file.as_ref().parent().ok_or_else(|| { + anyhow::anyhow!( + "Failed to get containing directory for file '{}'", + file.as_ref().display() + ) + })?; + + absolutize(path_buf) +} + +/// Returns absolute path to the file +pub fn absolutize(path: impl AsRef) -> Result { + let path = path.as_ref(); + + Ok(path + .absolutize() + .with_context(|| format!("Failed to resolve absolute path to: {}", path.display()))? + .to_path_buf()) +} + /// Converts a raw application manifest into Spin configuration while handling /// the Spin manifest and API version. async fn prepare_any_version( @@ -156,11 +175,7 @@ async fn core( bindle_connection: &Option, ) -> Result { let id = raw.id; - - let src = src - .as_ref() - .parent() - .expect("The application file did not have a parent directory."); + let src = parent_dir(src)?; let source = match raw.source { config::RawModuleSource::FileReference(p) => { let p = match p.is_absolute() { diff --git a/crates/publish/Cargo.toml b/crates/publish/Cargo.toml index 86ac907310..67f70b607f 100644 --- a/crates/publish/Cargo.toml +++ b/crates/publish/Cargo.toml @@ -11,7 +11,6 @@ dunce = "1.0" futures = "0.3.14" itertools = "0.10.3" mime_guess = { version = "2.0" } -path-absolutize = "3.0.11" reqwest = "0.11" semver = "1.0" serde = { version = "1.0", features = [ "derive" ] } diff --git a/crates/publish/src/bindle_writer.rs b/crates/publish/src/bindle_writer.rs index 4f753d9740..bd3af1d519 100644 --- a/crates/publish/src/bindle_writer.rs +++ b/crates/publish/src/bindle_writer.rs @@ -3,6 +3,7 @@ use crate::expander::expand_manifest; use anyhow::{Context, Result}; use bindle::{Invoice, Parcel}; +use spin_loader::local::parent_dir; use std::{ collections::BTreeMap, path::{Path, PathBuf}, @@ -24,7 +25,7 @@ pub async fn prepare_bindle( ) })?; - let source_dir = crate::app_dir(&app_file)?; + let source_dir = parent_dir(&app_file)?; write(&source_dir, &dest_dir, &invoice, &sources) .await diff --git a/crates/publish/src/expander.rs b/crates/publish/src/expander.rs index 55db8d88f1..5eb31ee69e 100644 --- a/crates/publish/src/expander.rs +++ b/crates/publish/src/expander.rs @@ -3,12 +3,11 @@ use crate::bindle_writer::{self, ParcelSources}; use anyhow::{Context, Result}; use bindle::{BindleSpec, Condition, Group, Invoice, Label, Parcel}; -use path_absolutize::Absolutize; use semver::BuildMetadata; use spin_loader::{ bindle::config as bindle_schema, digest::{bytes_sha256_string, file_sha256_string}, - local::{config as local_schema, validate_raw_app_manifest, UrlSource}, + local::{absolutize, config as local_schema, parent_dir, validate_raw_app_manifest, UrlSource}, }; use std::path::{Path, PathBuf}; @@ -18,14 +17,11 @@ pub(crate) async fn expand_manifest( buildinfo: Option, scratch_dir: impl AsRef, ) -> Result<(Invoice, ParcelSources)> { - let app_file = app_file - .as_ref() - .absolutize() - .context("Failed to resolve absolute path to manifest file")?; + let app_file = absolutize(app_file)?; let manifest = spin_loader::local::raw_manifest_from_file(&app_file).await?; validate_raw_app_manifest(&manifest)?; let local_schema::RawAppManifestAnyVersion::V1(manifest) = manifest; - let app_dir = crate::app_dir(&app_file)?; + let app_dir = parent_dir(&app_file)?; // * create a new spin.toml-like document where // - each component changes its `files` entry to a group name diff --git a/crates/publish/src/lib.rs b/crates/publish/src/lib.rs index 6623dd37fc..87329c86fc 100644 --- a/crates/publish/src/lib.rs +++ b/crates/publish/src/lib.rs @@ -8,20 +8,3 @@ mod expander; pub use bindle_pusher::push_all; pub use bindle_writer::prepare_bindle; - -use anyhow::Result; -use std::path::{Path, PathBuf}; - -pub(crate) fn app_dir(app_file: impl AsRef) -> Result { - let path_buf = app_file - .as_ref() - .parent() - .ok_or_else(|| { - anyhow::anyhow!( - "Failed to get containing directory for app file '{}'", - app_file.as_ref().display() - ) - })? - .to_owned(); - Ok(path_buf) -} diff --git a/src/commands/deploy.rs b/src/commands/deploy.rs index 98db013797..37399751f3 100644 --- a/src/commands/deploy.rs +++ b/src/commands/deploy.rs @@ -14,7 +14,7 @@ use spin_http::routes::RoutePattern; use spin_http::WELL_KNOWN_HEALTH_PATH; use spin_loader::bindle::BindleConnectionInfo; use spin_loader::local::config::{RawAppManifest, RawAppManifestAnyVersion}; -use spin_loader::local::{assets, config}; +use spin_loader::local::{assets, config, parent_dir}; use spin_manifest::ApplicationTrigger; use spin_manifest::{HttpTriggerConfiguration, TriggerConfig}; use tokio::fs; @@ -425,7 +425,7 @@ impl DeployCommand { async fn compute_buildinfo(&self, cfg: &RawAppManifest) -> Result { let mut sha256 = Sha256::new(); - let app_folder = crate::app_dir(&self.app)?; + let app_folder = parent_dir(&self.app)?; for x in cfg.components.iter() { match &x.source { diff --git a/src/commands/new.rs b/src/commands/new.rs index 47a95a2aa2..aa142a51e9 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -6,9 +6,9 @@ use std::{ use anyhow::{anyhow, Context, Result}; use clap::Parser; -use path_absolutize::Absolutize; use tokio::{fs::File, io::AsyncReadExt}; +use spin_loader::local::absolutize; use spin_templates::{RunOptions, Template, TemplateManager}; /// Scaffold a new application or component based on a template. @@ -111,17 +111,14 @@ impl FromStr for ParameterValue { } async fn values_from_file(file: impl AsRef) -> Result> { - let file = file - .as_ref() - .absolutize() - .context("Failed to resolve absolute path to values file")?; + let file = absolutize(file)?; let mut buf = vec![]; - File::open(file.as_ref()) + File::open(&file) .await? .read_to_end(&mut buf) .await - .with_context(|| anyhow!("Cannot read values file from {:?}", file.as_ref()))?; + .with_context(|| anyhow!("Cannot read values file from {:?}", file))?; toml::from_slice(&buf).context("Failed to deserialize values file") } diff --git a/src/lib.rs b/src/lib.rs index 7212eca0f4..08ca4eefee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,25 +2,9 @@ pub mod commands; pub(crate) mod opts; mod sloth; -use std::path::{Path, PathBuf}; - -use anyhow::{anyhow, Result}; +use anyhow::Result; use semver::BuildMetadata; -pub(crate) fn app_dir(app_file: impl AsRef) -> Result { - let path_buf = app_file - .as_ref() - .parent() - .ok_or_else(|| { - anyhow!( - "Failed to get containing directory for app file '{}'", - app_file.as_ref().display() - ) - })? - .to_owned(); - Ok(path_buf) -} - pub(crate) fn parse_buildinfo(buildinfo: &str) -> Result { Ok(BuildMetadata::new(buildinfo)?) } From 33a72139524399c26a76d1016eb57c3725168225 Mon Sep 17 00:00:00 2001 From: Konstantin Shabanov Date: Fri, 11 Nov 2022 17:11:26 +0600 Subject: [PATCH 2/2] chore: Simplify spin_build::build manifest_file is enough to build the app. Convert it info raw app inside rather then providing it through args. Signed-off-by: Konstantin Shabanov --- crates/build/src/lib.rs | 21 ++++++++++----------- src/commands/build.rs | 5 +---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/build/src/lib.rs b/crates/build/src/lib.rs index 8c4102d14b..76a6df71b8 100644 --- a/crates/build/src/lib.rs +++ b/crates/build/src/lib.rs @@ -4,17 +4,17 @@ use anyhow::{bail, Context, Result}; use spin_loader::local::{ - absolutize, - config::{RawAppManifest, RawComponentManifest}, - parent_dir, + config::{RawAppManifestAnyVersion, RawComponentManifest}, + parent_dir, raw_manifest_from_file, }; use std::path::{Path, PathBuf}; use subprocess::{Exec, Redirection}; use tracing::log; /// If present, run the build command of each component. -pub async fn build(app: RawAppManifest, src: &Path) -> Result<()> { - let src = absolutize(src)?; +pub async fn build(manifest_file: &Path) -> Result<()> { + let RawAppManifestAnyVersion::V1(app) = raw_manifest_from_file(&manifest_file).await?; + let app_dir = parent_dir(manifest_file)?; if app.components.iter().all(|c| c.build.is_none()) { println!("No build command found!"); @@ -24,7 +24,7 @@ pub async fn build(app: RawAppManifest, src: &Path) -> Result<()> { let results = futures::future::join_all( app.components .into_iter() - .map(|c| build_component(c, &src)) + .map(|c| build_component(c, &app_dir)) .collect::>(), ) .await; @@ -40,14 +40,14 @@ pub async fn build(app: RawAppManifest, src: &Path) -> Result<()> { } /// Run the build command of the component. -async fn build_component(raw: RawComponentManifest, src: impl AsRef) -> Result<()> { +async fn build_component(raw: RawComponentManifest, app_dir: &Path) -> Result<()> { match raw.build { Some(b) => { println!( "Executing the build command for component {}: {}", raw.id, b.command ); - let workdir = construct_workdir(src.as_ref(), b.workdir.as_ref())?; + let workdir = construct_workdir(app_dir, b.workdir.as_ref())?; if b.workdir.is_some() { println!("Working directory: {:?}", workdir); } @@ -83,8 +83,8 @@ async fn build_component(raw: RawComponentManifest, src: impl AsRef) -> Re } /// Constructs the absolute working directory in which to run the build command. -fn construct_workdir(src: impl AsRef, workdir: Option>) -> Result { - let mut cwd = parent_dir(src)?; +fn construct_workdir(app_dir: &Path, workdir: Option>) -> Result { + let mut cwd = app_dir.to_owned(); if let Some(workdir) = workdir { // Using `Path::has_root` as `is_relative` and `is_absolute` have @@ -94,7 +94,6 @@ fn construct_workdir(src: impl AsRef, workdir: Option>) - bail!("The workdir specified in the application file must be relative."); } cwd.push(workdir); - cwd = absolutize(cwd)?; } Ok(cwd) diff --git a/src/commands/build.rs b/src/commands/build.rs index 1ec32bc8e2..7e3683dd6a 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -3,8 +3,6 @@ use std::{ffi::OsString, path::PathBuf}; use anyhow::Result; use clap::Parser; -use spin_loader::local::{config::RawAppManifestAnyVersion, raw_manifest_from_file}; - use crate::opts::{APP_CONFIG_FILE_OPT, BUILD_UP_OPT, DEFAULT_MANIFEST_FILE}; use super::up::UpCommand; @@ -35,9 +33,8 @@ impl BuildCommand { .app .as_deref() .unwrap_or_else(|| DEFAULT_MANIFEST_FILE.as_ref()); - let RawAppManifestAnyVersion::V1(app) = raw_manifest_from_file(&manifest_file).await?; - spin_build::build(app, manifest_file).await?; + spin_build::build(manifest_file).await?; if self.up { let mut cmd = UpCommand::parse_from(