Skip to content

Newtype CargoTomlPath #3519

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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 crate_universe/private/srcs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ CARGO_BAZEL_SRCS = [
Label("//crate_universe:src/main.rs"),
Label("//crate_universe:src/metadata.rs"),
Label("//crate_universe:src/metadata/cargo_bin.rs"),
Label("//crate_universe:src/metadata/cargo_toml_path.rs"),
Label("//crate_universe:src/metadata/cargo_tree_resolver.rs"),
Label("//crate_universe:src/metadata/cargo_tree_rustc_wrapper.bat"),
Label("//crate_universe:src/metadata/cargo_tree_rustc_wrapper.sh"),
Expand Down
6 changes: 3 additions & 3 deletions crate_universe/src/cli/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use clap::Parser;
use crate::config::Config;
use crate::context::Context;
use crate::lockfile::{lock_context, write_lockfile};
use crate::metadata::{load_metadata, Annotations, Cargo, SourceAnnotation};
use crate::metadata::{load_metadata, Annotations, Cargo, CargoTomlPath, SourceAnnotation};
use crate::rendering::{write_outputs, Renderer};
use crate::splicing::SplicingManifest;
use crate::utils::normalize_cargo_file_paths;
Expand Down Expand Up @@ -236,7 +236,7 @@ fn update_cargo_lockfile(path: &Path, cargo_lockfile: Lockfile) -> Result<()> {
fn write_paths_to_track<
'a,
SourceAnnotations: Iterator<Item = &'a SourceAnnotation>,
Paths: Iterator<Item = Utf8PathBuf>,
Paths: Iterator<Item = CargoTomlPath>,
UnusedPatches: Iterator<Item = &'a cargo_lock::Dependency>,
>(
output_file: &Path,
Expand All @@ -248,7 +248,7 @@ fn write_paths_to_track<
let source_annotation_manifests: BTreeSet<_> = source_annotations
.filter_map(|v| {
if let SourceAnnotation::Path { path } = v {
Some(path.join("Cargo.toml"))
Some(CargoTomlPath::for_dir(path))
} else {
None
}
Expand Down
19 changes: 5 additions & 14 deletions crate_universe/src/cli/splice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {

let resolver_data = TreeResolver::new(cargo.clone())
.generate(
manifest_path.as_path_buf(),
manifest_path.cargo_toml_path(),
&config.supported_platform_triples,
)
.context("Failed to generate features")?;
Expand All @@ -108,8 +108,8 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
&cargo,
&cargo_lockfile,
resolver_data,
manifest_path.as_path_buf(),
manifest_path.as_path_buf(),
manifest_path.cargo_toml_path(),
manifest_path.cargo_toml_path(),
)
.context("Failed to write registry URLs and feature map")?;

Expand All @@ -119,19 +119,10 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
let (cargo_metadata, _) = Generator::new()
.with_cargo(cargo)
.with_rustc(opt.rustc)
.generate(manifest_path.as_path_buf())
.generate(manifest_path.cargo_toml_path())
.context("Failed to generate cargo metadata")?;

let cargo_lockfile_path = manifest_path
.as_path_buf()
.parent()
.with_context(|| {
format!(
"The path {} is expected to have a parent directory",
manifest_path.as_path_buf()
)
})?
.join("Cargo.lock");
let cargo_lockfile_path = manifest_path.cargo_toml_path().parent().join("Cargo.lock");

// Generate the consumable outputs of the splicing process
std::fs::create_dir_all(&output_dir)
Expand Down
10 changes: 5 additions & 5 deletions crate_universe/src/cli/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
let config = Config::try_from_path(&opt.config)?;

let resolver_data = TreeResolver::new(cargo.clone()).generate(
manifest_path.as_path_buf(),
manifest_path.cargo_toml_path(),
&config.supported_platform_triples,
)?;

Expand All @@ -236,15 +236,15 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
&cargo,
&cargo_lockfile,
resolver_data,
manifest_path.as_path_buf(),
manifest_path.as_path_buf(),
manifest_path.cargo_toml_path(),
manifest_path.cargo_toml_path(),
)?;

// Write metadata to the workspace for future reuse
let (cargo_metadata, cargo_lockfile) = Generator::new()
.with_cargo(cargo.clone())
.with_rustc(opt.rustc.clone())
.generate(manifest_path.as_path_buf())?;
.generate(manifest_path.cargo_toml_path())?;

// Annotate metadata
let annotations = Annotations::new(
Expand Down Expand Up @@ -281,7 +281,7 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {

if matches!(config.rendering.vendor_mode, Some(VendorMode::Local)) {
VendorGenerator::new(cargo, opt.rustc.clone())
.generate(manifest_path.as_path_buf(), &vendor_dir)
.generate(manifest_path.cargo_toml_path(), &vendor_dir)
.context("Failed to vendor dependencies")?;
}

Expand Down
11 changes: 6 additions & 5 deletions crate_universe/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Tools for gathering various kinds of metadata (Cargo.lock, Cargo metadata, Crate Index info).

mod cargo_bin;
mod cargo_toml_path;
mod cargo_tree_resolver;
mod dependency;
mod metadata_annotation;
Expand All @@ -12,12 +13,12 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;

use anyhow::{bail, Context, Result};
use camino::Utf8Path;
use cargo_lock::Lockfile as CargoLockfile;
use cargo_metadata::Metadata as CargoMetadata;
use tracing::debug;

pub(crate) use self::cargo_bin::*;
pub(crate) use self::cargo_toml_path::*;
pub(crate) use self::cargo_tree_resolver::*;
pub(crate) use self::dependency::*;
pub(crate) use self::metadata_annotation::*;
Expand Down Expand Up @@ -190,13 +191,13 @@ impl LockGenerator {
#[tracing::instrument(name = "LockGenerator::generate", skip_all)]
pub(crate) fn generate(
&self,
manifest_path: &Utf8Path,
manifest_path: &CargoTomlPath,
existing_lock: &Option<PathBuf>,
update_request: &Option<CargoUpdateRequest>,
) -> Result<cargo_lock::Lockfile> {
debug!("Generating Cargo Lockfile for {}", manifest_path);

let manifest_dir = manifest_path.parent().unwrap();
let manifest_dir = manifest_path.parent();
let generated_lockfile_path = manifest_dir.join("Cargo.lock");

if let Some(lock) = existing_lock {
Expand Down Expand Up @@ -304,9 +305,9 @@ impl VendorGenerator {
}
}
#[tracing::instrument(name = "VendorGenerator::generate", skip_all)]
pub(crate) fn generate(&self, manifest_path: &Utf8Path, output_dir: &Path) -> Result<()> {
pub(crate) fn generate(&self, manifest_path: &CargoTomlPath, output_dir: &Path) -> Result<()> {
debug!("Vendoring {} to {}", manifest_path, output_dir.display());
let manifest_dir = manifest_path.parent().unwrap();
let manifest_dir = manifest_path.parent();

// Simply invoke `cargo generate-lockfile`
let output = self
Expand Down
60 changes: 60 additions & 0 deletions crate_universe/src/metadata/cargo_toml_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use anyhow::{anyhow, Result};
use camino::{Utf8Path, Utf8PathBuf};
use serde::{Deserialize, Serialize};
use std::fmt;
use std::fmt::Display;
use std::path::Path;

/// Path that is know to end with `/Cargo.toml`.
#[derive(Eq, PartialEq, Debug, Clone, Hash, Ord, PartialOrd, Serialize, Deserialize)]
#[serde(transparent)]
pub(crate) struct CargoTomlPath(Utf8PathBuf);

impl Display for CargoTomlPath {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Display::fmt(&self.0, f)
}
}

impl CargoTomlPath {
pub(crate) fn unchecked_new(path: impl Into<Utf8PathBuf>) -> Self {
CargoTomlPath(path.into())
}

pub(crate) fn new(path: impl Into<Utf8PathBuf>) -> Result<Self> {
let path = path.into();
if !path.ends_with("Cargo.toml") {
return Err(anyhow!("Path does not end with Cargo.toml: {path}"));
}
Ok(Self::unchecked_new(path))
}

pub(crate) fn for_dir(dir: impl Into<Utf8PathBuf>) -> Self {
let mut path = dir.into();
path.push("Cargo.toml");
CargoTomlPath::unchecked_new(path)
}

/// Directory of `Cargo.toml` path.
pub(crate) fn parent(&self) -> &Utf8Path {
self.0.parent().expect("Validated in constructor")
}

pub(crate) fn ancestors(&self) -> impl Iterator<Item = &Utf8Path> {
self.0.ancestors()
}

pub(crate) fn as_path(&self) -> &Utf8Path {
self.0.as_path()
}

pub(crate) fn as_std_path(&self) -> &Path {
self.0.as_std_path()
}
}

impl AsRef<Path> for CargoTomlPath {
fn as_ref(&self) -> &Path {
self.0.as_ref()
}
}
8 changes: 4 additions & 4 deletions crate_universe/src/metadata/cargo_tree_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use std::path::{Path, PathBuf};
use std::process::Child;

use anyhow::{anyhow, bail, Context, Result};
use camino::Utf8Path;
use semver::Version;
use serde::{Deserialize, Serialize};
use tracing::{debug, trace};
use url::Url;

use crate::config::CrateId;
use crate::metadata::cargo_bin::Cargo;
use crate::metadata::CargoTomlPath;
use crate::select::{Select, SelectableScalar};
use crate::utils::symlink::symlink;
use crate::utils::target_triple::TargetTriple;
Expand Down Expand Up @@ -307,7 +307,7 @@ impl TreeResolver {
#[tracing::instrument(name = "TreeResolver::generate", skip_all)]
pub(crate) fn generate(
&self,
pristine_manifest_path: &Utf8Path,
pristine_manifest_path: &CargoTomlPath,
target_triples: &BTreeSet<TargetTriple>,
) -> Result<TreeResolverMetadata> {
debug!(
Expand Down Expand Up @@ -450,14 +450,14 @@ impl TreeResolver {
// and if we don't have this fake root injection, cross-compiling from Darwin to Linux won't work because features don't get correctly resolved for the exec=darwin case.
fn copy_project_with_explicit_deps_on_all_transitive_proc_macros(
&self,
pristine_manifest_path: &Utf8Path,
pristine_manifest_path: &CargoTomlPath,
output_dir: &Path,
) -> Result<PathBuf> {
if !output_dir.exists() {
std::fs::create_dir_all(output_dir)?;
}

let pristine_root = pristine_manifest_path.parent().unwrap();
let pristine_root = pristine_manifest_path.parent();
for file in std::fs::read_dir(pristine_root).context("Failed to read dir")? {
let source_path = file?.path();
let file_name = source_path.file_name().unwrap();
Expand Down
Loading