Skip to content

Commit

Permalink
Auto merge of #12268 - epage:direct, r=weihanglo
Browse files Browse the repository at this point in the history
fix(embedded): Don't create an intermediate manifest

### What does this PR try to resolve?

More immediately, this is to unblock rust-lang/rust#112601

More generally, this gets us away from hackily writing out an out-of-line manifest from an embedded manifest.  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.

### How should we test and review this PR?

A Commit at a time

### Additional information

In production code, this does not conflict with #12255 (due to #12262) but in test code, it does.
  • Loading branch information
bors committed Jun 17, 2023
2 parents a581ca2 + 224d2e6 commit 0d5370a
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 279 deletions.
35 changes: 1 addition & 34 deletions Cargo.lock

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

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ exclude = [
[workspace.dependencies]
anyhow = "1.0.47"
base64 = "0.21.0"
blake3 = "1.3.3"
bytesize = "1.0"
cargo = { path = "" }
cargo-credential = { version = "0.2.0", path = "credential/cargo-credential" }
Expand Down Expand Up @@ -114,7 +113,6 @@ path = "src/cargo/lib.rs"
[dependencies]
anyhow.workspace = true
base64.workspace = true
blake3.workspace = true
bytesize.workspace = true
cargo-platform.workspace = true
cargo-util.workspace = true
Expand Down
1 change: 0 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ allow = [
"MIT-0",
"Apache-2.0",
"BSD-3-Clause",
"BSD-2-Clause",
"MPL-2.0",
"Unicode-DFS-2016",
"CC0-1.0",
Expand Down
18 changes: 12 additions & 6 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 @@ -95,14 +96,19 @@ pub fn exec_manifest_command(config: &Config, cmd: &str, args: &[OsString]) -> C
}

let manifest_path = Path::new(cmd);
let manifest_path = config.cwd().join(manifest_path);
let manifest_path = cargo_util::paths::normalize_path(&manifest_path);
if !manifest_path.exists() {
return Err(
anyhow::anyhow!("manifest `{}` does not exist", manifest_path.display()).into(),
);
return Err(anyhow::format_err!(
"manifest path `{}` does not exist",
manifest_path.display()
)
.into());
}
let mut ws = Workspace::new(&manifest_path, config)?;
if config.cli_unstable().avoid_dev_deps {
ws.set_require_optional_deps(false);
}
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 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
Loading

0 comments on commit 0d5370a

Please sign in to comment.