Skip to content

Commit

Permalink
fix(embedded): Don't create an intermediate manifest
Browse files Browse the repository at this point in the history
To parse the manifest, we have to write it out so our regular manifest
loading code could handle it.  This updates the manifest parsing code to
handle it.

This doesn't mean this will work everywhere in all cases though.  For
example, ephemeral workspaces parses a manifest from the SourceId and
these won't have valid SourceIds.

As a consequence, `Cargo.lock` and `CARGO_TARGET_DIR` are changing from being next to
the temp manifest to being next to the script.  This still isn't the
desired behavior but stepping stones.

This also exposes the fact that we didn't disable `autobins` like the
documentation says we should.
  • Loading branch information
epage committed Jun 14, 2023
1 parent 0af8c66 commit ab590b8
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 235 deletions.
7 changes: 5 additions & 2 deletions src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::Path;
use crate::command_prelude::*;
use crate::util::restricted_names::is_glob_pattern;
use cargo::core::Verbosity;
use cargo::core::Workspace;
use cargo::ops::{self, CompileFilter, Packages};
use cargo_util::ProcessError;

Expand Down Expand Up @@ -101,8 +102,10 @@ pub fn exec_manifest_command(config: &Config, cmd: &str, args: &[OsString]) -> C
);
}
let manifest_path = crate::util::try_canonicalize(manifest_path)?;
let script = cargo::util::toml::embedded::parse_from(&manifest_path)?;
let ws = cargo::util::toml::embedded::to_workspace(&script, config)?;
let mut ws = Workspace::new(&manifest_path, config)?;
if config.cli_unstable().avoid_dev_deps {
ws.set_require_optional_deps(false);
}

let mut compile_opts =
cargo::ops::CompileOptions::new(config, cargo::core::compiler::CompileMode::Build)?;
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub struct Manifest {
metabuild: Option<Vec<String>>,
resolve_behavior: Option<ResolveBehavior>,
lint_rustflags: Vec<String>,
embedded: bool,
}

/// When parsing `Cargo.toml`, some warnings should silenced
Expand Down Expand Up @@ -407,6 +408,7 @@ impl Manifest {
metabuild: Option<Vec<String>>,
resolve_behavior: Option<ResolveBehavior>,
lint_rustflags: Vec<String>,
embedded: bool,
) -> Manifest {
Manifest {
summary,
Expand All @@ -433,6 +435,7 @@ impl Manifest {
metabuild,
resolve_behavior,
lint_rustflags,
embedded,
}
}

Expand Down Expand Up @@ -500,6 +503,9 @@ impl Manifest {
pub fn links(&self) -> Option<&str> {
self.links.as_deref()
}
pub fn is_embedded(&self) -> bool {
self.embedded
}

pub fn workspace_config(&self) -> &WorkspaceConfig {
&self.workspace
Expand Down
11 changes: 11 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,10 @@ impl<'cfg> Workspace<'cfg> {
if self.members.contains(&manifest_path) {
return Ok(());
}
if is_path_dep && self.root_maybe().is_embedded() {
// Embedded manifests cannot have workspace members
return Ok(());
}
if is_path_dep
&& !manifest_path.parent().unwrap().starts_with(self.root())
&& self.find_root(&manifest_path)? != self.root_manifest
Expand Down Expand Up @@ -1580,6 +1584,13 @@ impl MaybePackage {
MaybePackage::Virtual(ref vm) => vm.workspace_config(),
}
}

fn is_embedded(&self) -> bool {
match self {
MaybePackage::Package(p) => p.manifest().is_embedded(),
MaybePackage::Virtual(_) => false,
}
}
}

impl WorkspaceRootConfig {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
let package_root = orig_pkg.root();
let source_id = orig_pkg.package_id().source_id();
let (manifest, _nested_paths) =
TomlManifest::to_real_manifest(&toml_manifest, source_id, package_root, config)?;
TomlManifest::to_real_manifest(&toml_manifest, false, source_id, package_root, config)?;
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());

// Regenerate Cargo.lock using the old one as a guide.
Expand Down
213 changes: 27 additions & 186 deletions src/cargo/util/toml/embedded.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use anyhow::Context as _;

use crate::core::Workspace;
use crate::CargoResult;
use crate::Config;

Expand All @@ -9,21 +8,13 @@ const DEFAULT_EDITION: crate::core::features::Edition =
const DEFAULT_VERSION: &str = "0.0.0";
const DEFAULT_PUBLISH: bool = false;

pub struct RawScript {
manifest: String,
body: String,
path: std::path::PathBuf,
}

pub fn parse_from(path: &std::path::Path) -> CargoResult<RawScript> {
let body = std::fs::read_to_string(path)
.with_context(|| format!("failed to script at {}", path.display()))?;
parse(&body, path)
}

fn parse(body: &str, path: &std::path::Path) -> CargoResult<RawScript> {
let comment = match extract_comment(body) {
Ok(manifest) => Some(manifest),
pub fn expand_manifest(
content: &str,
path: &std::path::Path,
config: &Config,
) -> CargoResult<String> {
let comment = match extract_comment(content) {
Ok(comment) => Some(comment),
Err(err) => {
log::trace!("failed to extract doc comment: {err}");
None
Expand All @@ -38,83 +29,19 @@ fn parse(body: &str, path: &std::path::Path) -> CargoResult<RawScript> {
}
}
.unwrap_or_default();
let body = body.to_owned();
let path = path.to_owned();
Ok(RawScript {
manifest,
body,
path,
})
}

pub fn to_workspace<'cfg>(
script: &RawScript,
config: &'cfg Config,
) -> CargoResult<Workspace<'cfg>> {
let target_dir = config
.target_dir()
.transpose()
.unwrap_or_else(|| default_target_dir().map(crate::util::Filesystem::new))?;
// HACK: without cargo knowing about embedded manifests, the only way to create a
// `Workspace` is either
// - Create a temporary one on disk
// - Create an "ephemeral" workspace **but** compilation re-loads ephemeral workspaces
// from the registry rather than what we already have on memory, causing it to fail
// because the registry doesn't know about embedded manifests.
let manifest_path = write(script, config, target_dir.as_path_unlocked())?;
let workspace = Workspace::new(&manifest_path, config)?;
Ok(workspace)
}

fn write(
script: &RawScript,
config: &Config,
target_dir: &std::path::Path,
) -> CargoResult<std::path::PathBuf> {
let hash = hash(script).to_string();
assert_eq!(hash.len(), 64);

let file_name = script
.path
.file_stem()
.ok_or_else(|| anyhow::format_err!("no file name"))?
.to_string_lossy();
let separator = '_';
let name = sanitize_package_name(file_name.as_ref(), separator);

let mut workspace_root = target_dir.to_owned();
workspace_root.push("eval");
workspace_root.push(&hash[0..2]);
workspace_root.push(&hash[2..4]);
workspace_root.push(&hash[4..]);
workspace_root.push(name);
std::fs::create_dir_all(&workspace_root).with_context(|| {
format!(
"failed to create temporary workspace at {}",
workspace_root.display()
)
})?;
let manifest_path = workspace_root.join("Cargo.toml");
let manifest = expand_manifest(script, config)?;
write_if_changed(&manifest_path, &manifest)?;
Ok(manifest_path)
}

fn expand_manifest(script: &RawScript, config: &Config) -> CargoResult<String> {
let manifest = expand_manifest_(script, config)
.with_context(|| format!("failed to parse manifest at {}", script.path.display()))?;
let manifest = remap_paths(
manifest,
script.path.parent().ok_or_else(|| {
anyhow::format_err!("no parent directory for {}", script.path.display())
})?,
)?;
let manifest = expand_manifest_(content, &manifest, path, config)
.with_context(|| format!("failed to parse manifest at {}", path.display()))?;
let manifest = toml::to_string_pretty(&manifest)?;
Ok(manifest)
}

fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult<toml::Table> {
let mut manifest: toml::Table = toml::from_str(&script.manifest)?;
fn expand_manifest_(
content: &str,
manifest: &str,
path: &std::path::Path,
config: &Config,
) -> CargoResult<toml::Table> {
let mut manifest: toml::Table = toml::from_str(&manifest)?;

for key in ["workspace", "lib", "bin", "example", "test", "bench"] {
if manifest.contains_key(key) {
Expand All @@ -135,14 +62,13 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult<toml::Ta
anyhow::bail!("`package.{key}` is not allowed in embedded manifests")
}
}
let file_name = script
.path
let file_name = path
.file_stem()
.ok_or_else(|| anyhow::format_err!("no file name"))?
.to_string_lossy();
let separator = '_';
let name = sanitize_package_name(file_name.as_ref(), separator);
let hash = hash(script);
let hash = hash(content);
let bin_name = format!("{name}{separator}{hash}");
package
.entry("name".to_owned())
Expand All @@ -166,9 +92,7 @@ fn expand_manifest_(script: &RawScript, config: &Config) -> CargoResult<toml::Ta
bin.insert(
"path".to_owned(),
toml::Value::String(
script
.path
.to_str()
path.to_str()
.ok_or_else(|| anyhow::format_err!("path is not valid UTF-8"))?
.into(),
),
Expand Down Expand Up @@ -217,26 +141,8 @@ fn sanitize_package_name(name: &str, placeholder: char) -> String {
slug
}

fn hash(script: &RawScript) -> blake3::Hash {
blake3::hash(script.body.as_bytes())
}

fn default_target_dir() -> CargoResult<std::path::PathBuf> {
let mut cargo_home = home::cargo_home()?;
cargo_home.push("eval");
cargo_home.push("target");
Ok(cargo_home)
}

fn write_if_changed(path: &std::path::Path, new: &str) -> CargoResult<()> {
let write_needed = match std::fs::read_to_string(path) {
Ok(current) => current != new,
Err(_) => true,
};
if write_needed {
std::fs::write(path, new).with_context(|| format!("failed to write {}", path.display()))?;
}
Ok(())
fn hash(content: &str) -> blake3::Hash {
blake3::hash(content.as_bytes())
}

/// Locates a "code block manifest" in Rust source.
Expand Down Expand Up @@ -438,10 +344,12 @@ mod test_expand {

macro_rules! si {
($i:expr) => {{
let script = parse($i, std::path::Path::new("/home/me/test.rs"))
.unwrap_or_else(|err| panic!("{}", err));
expand_manifest(&script, &Config::default().unwrap())
.unwrap_or_else(|err| panic!("{}", err))
expand_manifest(
$i,
std::path::Path::new("/home/me/test.rs"),
&Config::default().unwrap(),
)
.unwrap_or_else(|err| panic!("{}", err))
}};
}

Expand Down Expand Up @@ -693,73 +601,6 @@ fn main() {}
}
}

/// Given a Cargo manifest, attempts to rewrite relative file paths to absolute ones, allowing the manifest to be relocated.
fn remap_paths(
mani: toml::Table,
package_root: &std::path::Path,
) -> anyhow::Result<toml::value::Table> {
// Values that need to be rewritten:
let paths: &[&[&str]] = &[
&["build-dependencies", "*", "path"],
&["dependencies", "*", "path"],
&["dev-dependencies", "*", "path"],
&["package", "build"],
&["target", "*", "dependencies", "*", "path"],
];

let mut mani = toml::Value::Table(mani);

for path in paths {
iterate_toml_mut_path(&mut mani, path, &mut |v| {
if let toml::Value::String(s) = v {
if std::path::Path::new(s).is_relative() {
let p = package_root.join(&*s);
if let Some(p) = p.to_str() {
*s = p.into()
}
}
}
Ok(())
})?
}

match mani {
toml::Value::Table(mani) => Ok(mani),
_ => unreachable!(),
}
}

/// Iterates over the specified TOML values via a path specification.
fn iterate_toml_mut_path<F>(
base: &mut toml::Value,
path: &[&str],
on_each: &mut F,
) -> anyhow::Result<()>
where
F: FnMut(&mut toml::Value) -> anyhow::Result<()>,
{
if path.is_empty() {
return on_each(base);
}

let cur = path[0];
let tail = &path[1..];

if cur == "*" {
if let toml::Value::Table(tab) = base {
for (_, v) in tab {
iterate_toml_mut_path(v, tail, on_each)?;
}
}
} else if let toml::Value::Table(tab) = base {
if let Some(v) = tab.get_mut(cur) {
iterate_toml_mut_path(v, tail, on_each)?;
}
}

Ok(())
}

#[cfg(test)]
mod test_manifest {
use super::*;
Expand Down
Loading

0 comments on commit ab590b8

Please sign in to comment.