Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double backslashes in printed Windows paths #2162

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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