Skip to content

Commit

Permalink
Merge pull request #2162 from itowlson/fix-windows-double-backslashes
Browse files Browse the repository at this point in the history
Fix double backslashes in printed Windows paths
  • Loading branch information
itowlson authored Dec 12, 2023
2 parents d075d5d + c5ac337 commit fc417e1
Show file tree
Hide file tree
Showing 23 changed files with 151 additions and 62 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 8 additions & 3 deletions crates/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod manifest;

use anyhow::{anyhow, bail, Context, Result};
use manifest::ComponentBuildInfo;
use spin_common::paths::parent_dir;
use spin_common::{paths::parent_dir, ui::quoted_path};
use std::{
collections::HashSet,
path::{Path, PathBuf},
Expand All @@ -19,7 +19,12 @@ use crate::manifest::component_build_configs;
pub async fn build(manifest_file: &Path, component_ids: &[String]) -> Result<()> {
let components = component_build_configs(manifest_file)
.await
.with_context(|| format!("Cannot read manifest file from {}", manifest_file.display()))?;
.with_context(|| {
format!(
"Cannot read manifest file from {}",
quoted_path(manifest_file)
)
})?;
let app_dir = parent_dir(manifest_file)?;

let components_to_build = if component_ids.is_empty() {
Expand Down Expand Up @@ -69,7 +74,7 @@ fn build_component(build_info: ComponentBuildInfo, app_dir: &Path) -> Result<()>
);
let workdir = construct_workdir(app_dir, b.workdir.as_ref())?;
if b.workdir.is_some() {
println!("Working directory: {:?}", workdir);
println!("Working directory: {}", quoted_path(&workdir));
}

let exit_status = Exec::shell(&b.command)
Expand Down
1 change: 1 addition & 0 deletions crates/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ pub mod data_dir;
pub mod paths;
pub mod sha256;
pub mod sloth;
pub mod ui;
pub mod url;
4 changes: 3 additions & 1 deletion crates/common/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
use anyhow::{anyhow, Context, Result};
use std::path::{Path, PathBuf};

use crate::ui::quoted_path;

/// The name given to the default manifest file.
pub const DEFAULT_MANIFEST_FILE: &str = "spin.toml";

Expand Down Expand Up @@ -40,7 +42,7 @@ pub fn parent_dir(path: impl AsRef<Path>) -> Result<PathBuf> {
let path = path.as_ref();
let mut parent = path
.parent()
.with_context(|| format!("No parent directory for path {path:?}"))?;
.with_context(|| format!("No parent directory for path {}", quoted_path(path)))?;
if parent == Path::new("") {
parent = Path::new(".");
}
Expand Down
10 changes: 10 additions & 0 deletions crates/common/src/ui.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//! Functions supporting common UI behaviour and standards

use std::path::Path;

/// Renders a Path with double quotes. This is the standard
/// for displaying paths in Spin. It is preferred to the Debug
/// format because the latter doubles up backlashes on Windows.
pub fn quoted_path(path: impl AsRef<Path>) -> impl std::fmt::Display {
format!("\"{}\"", path.as_ref().display())
}
23 changes: 16 additions & 7 deletions crates/doctor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{collections::VecDeque, fmt::Debug, fs, path::PathBuf};

use anyhow::{ensure, Context, Result};
use async_trait::async_trait;
use spin_common::ui::quoted_path;
use toml_edit::Document;

/// Diagnoses for app manifest format problems.
Expand Down Expand Up @@ -88,15 +89,23 @@ impl PatientApp {
let path = manifest_path.into();
ensure!(
path.is_file(),
"No Spin app manifest file found at {path:?}"
"No Spin app manifest file found at {}",
quoted_path(&path),
);

let contents = fs::read_to_string(&path)
.with_context(|| format!("Couldn't read Spin app manifest file at {path:?}"))?;

let manifest_doc: Document = contents
.parse()
.with_context(|| format!("Couldn't parse manifest file at {path:?} as valid TOML"))?;
let contents = fs::read_to_string(&path).with_context(|| {
format!(
"Couldn't read Spin app manifest file at {}",
quoted_path(&path)
)
})?;

let manifest_doc: Document = contents.parse().with_context(|| {
format!(
"Couldn't parse manifest file at {} as valid TOML",
quoted_path(&path)
)
})?;

Ok(Self {
manifest_path: path,
Expand Down
8 changes: 5 additions & 3 deletions crates/doctor/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::fs;

use anyhow::{Context, Result};
use async_trait::async_trait;
use spin_common::ui::quoted_path;
use toml_edit::Document;

use crate::Treatment;
Expand Down Expand Up @@ -37,8 +38,9 @@ impl<T: ManifestTreatment + Sync> Treatment for T {
let after = after_doc.to_string();
let diff = similar::udiff::unified_diff(Default::default(), &before, &after, 1, None);
Ok(format!(
"Apply the following diff to {:?}:\n{}",
patient.manifest_path, diff
"Apply the following diff to {}:\n{}",
quoted_path(&patient.manifest_path),
diff
))
}

Expand All @@ -47,6 +49,6 @@ impl<T: ManifestTreatment + Sync> Treatment for T {
self.treat_manifest(doc).await?;
let path = &patient.manifest_path;
fs::write(path, doc.to_string())
.with_context(|| format!("failed to write fixed manifest to {path:?}"))
.with_context(|| format!("failed to write fixed manifest to {}", quoted_path(path)))
}
}
6 changes: 5 additions & 1 deletion crates/doctor/src/manifest/upgrade.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::{anyhow, Context, Result};
use async_trait::async_trait;
use spin_common::ui::quoted_path;
use spin_manifest::{compat::v1_to_v2_app, schema::v1::AppManifestV1, ManifestVersion};
use toml_edit::{de::from_document, ser::to_document, Item, Table};

Expand Down Expand Up @@ -97,7 +98,10 @@ impl Treatment for UpgradeDiagnosis {
let v1_backup_path = patient.manifest_path.with_extension("toml.v1_backup");
std::fs::rename(&patient.manifest_path, &v1_backup_path)
.context("failed to back up existing manifest")?;
println!("Version 1 manifest backed up to {v1_backup_path:?}.");
println!(
"Version 1 manifest backed up to {}.",
quoted_path(&v1_backup_path)
);

// Write new V2 manifest
std::fs::write(&patient.manifest_path, v2_doc.to_string())
Expand Down
6 changes: 5 additions & 1 deletion crates/doctor/src/wasm/missing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::process::Command;

use anyhow::{ensure, Context, Result};
use async_trait::async_trait;
use spin_common::ui::quoted_path;

use crate::{Diagnosis, PatientApp, Treatment};

Expand Down Expand Up @@ -52,7 +53,10 @@ impl Diagnosis for WasmMissing {
let Some(rel_path) = self.0.source_path() else {
unreachable!("unsupported source");
};
format!("Component {id:?} source {rel_path:?} is missing")
format!(
"Component {id:?} source {} is missing",
quoted_path(rel_path)
)
}

fn treatment(&self) -> Option<&dyn Treatment> {
Expand Down
1 change: 1 addition & 0 deletions crates/llm-local/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ num_cpus = "1"
rand = "0.8.5"
safetensors = "0.3.3"
serde = { version = "1.0.150", features = ["derive"] }
spin-common = { path = "../common" }
spin-core = { path = "../core" }
spin-llm = { path = "../llm" }
spin-world = { path = "../world" }
Expand Down
11 changes: 8 additions & 3 deletions crates/llm-local/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use llm::{
ModelArchitecture, ModelKVMemoryType, ModelParameters,
};
use rand::SeedableRng;
use spin_common::ui::quoted_path;
use spin_core::async_trait;
use spin_llm::{LlmEngine, MODEL_ALL_MINILM_L6_V2};
use spin_world::v2::llm::{self as wasi_llm};
Expand Down Expand Up @@ -376,14 +377,18 @@ async fn generate_embeddings(
}

fn load_tokenizer(tokenizer_file: &Path) -> anyhow::Result<tokenizers::Tokenizer> {
let tokenizer = tokenizers::Tokenizer::from_file(tokenizer_file)
.map_err(|e| anyhow::anyhow!("Failed to read tokenizer file {tokenizer_file:?}: {e}"))?;
let tokenizer = tokenizers::Tokenizer::from_file(tokenizer_file).map_err(|e| {
anyhow::anyhow!(
"Failed to read tokenizer file {}: {e}",
quoted_path(tokenizer_file)
)
})?;
Ok(tokenizer)
}

fn load_model(model_file: &Path) -> anyhow::Result<BertModel> {
let buffer = std::fs::read(model_file)
.with_context(|| format!("Failed to read model file {model_file:?}"))?;
.with_context(|| format!("Failed to read model file {}", quoted_path(model_file)))?;
let weights = safetensors::SafeTensors::deserialize(&buffer)?;
let vb = VarBuilder::from_safetensors(vec![weights], DType::F32, &candle::Device::Cpu);
let model = BertModel::load(vb, &Config::default()).context("error loading bert model")?;
Expand Down
33 changes: 22 additions & 11 deletions crates/loader/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use anyhow::{bail, ensure, Context, Result};
use futures::future::try_join_all;
use reqwest::Url;
use spin_common::paths::parent_dir;
use spin_common::{paths::parent_dir, ui::quoted_path};
use spin_locked_app::{
locked::{
self, ContentPath, ContentRef, LockedApp, LockedComponent, LockedComponentSource,
Expand Down Expand Up @@ -47,12 +47,16 @@ impl LocalLoader {
pub async fn load_file(&self, path: impl AsRef<Path>) -> Result<LockedApp> {
// Parse manifest
let path = path.as_ref();
let manifest = spin_manifest::manifest_from_file(path)
.with_context(|| format!("Failed to read Spin app manifest from {path:?}"))?;
let manifest = spin_manifest::manifest_from_file(path).with_context(|| {
format!(
"Failed to read Spin app manifest from {}",
quoted_path(path)
)
})?;
let mut locked = self
.load_manifest(manifest)
.await
.with_context(|| format!("Failed to load Spin app from {path:?}"))?;
.with_context(|| format!("Failed to load Spin app from {}", quoted_path(path)))?;

// Set origin metadata
locked
Expand Down Expand Up @@ -286,7 +290,7 @@ impl LocalLoader {
let src_path = self.app_root.join(src);
let meta = fs::metadata(&src_path)
.await
.with_context(|| format!("invalid file mount source {src:?}"))?;
.with_context(|| format!("invalid file mount source {}", quoted_path(src)))?;
if meta.is_dir() {
// { source = "host/dir", destination = "guest/dir" }
let pattern = src_path.join("**/*");
Expand Down Expand Up @@ -359,12 +363,19 @@ impl LocalLoader {

let _loading_permit = self.file_loading_permits.acquire().await?;
let dest_parent = parent_dir(dest)?;
fs::create_dir_all(&dest_parent)
.await
.with_context(|| format!("Failed to create parent directory {dest_parent:?}"))?;
fs::copy(src, dest)
.await
.with_context(|| format!("Failed to copy {src:?} to {dest:?}"))?;
fs::create_dir_all(&dest_parent).await.with_context(|| {
format!(
"Failed to create parent directory {}",
quoted_path(&dest_parent)
)
})?;
fs::copy(src, dest).await.with_context(|| {
format!(
"Failed to copy {} to {}",
quoted_path(src),
quoted_path(dest)
)
})?;
tracing::debug!("Copied {src:?} to {dest:?}");
Ok(())
}
Expand Down
5 changes: 3 additions & 2 deletions crates/oci/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
use anyhow::{bail, Context, Result};
use oci_distribution::secrets::RegistryAuth;
use serde::{Deserialize, Serialize};
use spin_common::ui::quoted_path;

#[derive(Serialize, Deserialize)]
pub struct AuthConfig {
Expand Down Expand Up @@ -84,7 +85,7 @@ impl AuthConfig {
async fn load(p: &Path) -> Result<Self> {
let contents = tokio::fs::read_to_string(&p).await?;
serde_json::from_str(&contents)
.with_context(|| format!("cannot load authentication file {:?}", p))
.with_context(|| format!("cannot load authentication file {}", quoted_path(p)))
}

async fn save(&self, p: &Path) -> Result<()> {
Expand All @@ -95,6 +96,6 @@ impl AuthConfig {
}
tokio::fs::write(&p, &serde_json::to_vec_pretty(&self)?)
.await
.with_context(|| format!("cannot save authentication file {:?}", p))
.with_context(|| format!("cannot save authentication file {}", quoted_path(p)))
}
}
11 changes: 9 additions & 2 deletions crates/oci/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use oci_distribution::{
};
use reqwest::Url;
use spin_common::sha256;
use spin_common::ui::quoted_path;
use spin_common::url::parse_file_url;
use spin_loader::cache::Cache;
use spin_loader::FilesMountStrategy;
Expand Down Expand Up @@ -148,11 +149,17 @@ impl Client {
if archive_layers {
self.push_archive_layer(&source, &mut files, &mut layers)
.await
.context(format!("cannot push archive layer for source {:?}", source))?;
.context(format!(
"cannot push archive layer for source {}",
quoted_path(&source)
))?;
} else {
self.push_file_layers(&source, &mut files, &mut layers)
.await
.context(format!("cannot push file layers for source {:?}", source))?;
.context(format!(
"cannot push file layers for source {}",
quoted_path(&source)
))?;
}
}
c.files = files;
Expand Down
16 changes: 12 additions & 4 deletions crates/oci/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf};
use anyhow::{anyhow, ensure, Context, Result};
use oci_distribution::Reference;
use reqwest::Url;
use spin_common::ui::quoted_path;
use spin_loader::cache::Cache;
use spin_locked_app::locked::{ContentPath, ContentRef, LockedApp, LockedComponent};

Expand Down Expand Up @@ -46,9 +47,13 @@ impl OciLoader {
) -> std::result::Result<LockedApp, anyhow::Error> {
let locked_content = tokio::fs::read(&lockfile_path)
.await
.with_context(|| format!("failed to read from {lockfile_path:?}"))?;
let mut locked_app = LockedApp::from_json(&locked_content)
.with_context(|| format!("failed to decode locked app from {lockfile_path:?}"))?;
.with_context(|| format!("failed to read from {}", quoted_path(&lockfile_path)))?;
let mut locked_app = LockedApp::from_json(&locked_content).with_context(|| {
format!(
"failed to decode locked app from {}",
quoted_path(&lockfile_path)
)
})?;

// Update origin metadata
let resolved_reference = Reference::try_from(reference).context("invalid reference")?;
Expand Down Expand Up @@ -108,7 +113,10 @@ impl OciLoader {
tokio::fs::copy(&content_path, &mount_path)
.await
.with_context(|| {
format!("failed to copy {content_path:?}->{mount_path:?}")
format!(
"failed to copy {}->{mount_path:?}",
quoted_path(&content_path)
)
})?;
}
}
Expand Down
Loading

0 comments on commit fc417e1

Please sign in to comment.