Newtype CargoTomlPath

When reading code, it is not always clear, what path is Cargo.toml
path, and what path is directory path.

Author: stepancheg

crate_universe/private/srcs.bzl

@@ -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"),

crate_universe/src/cli/generate.rs

@@ -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;
@@ -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,
@@ -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
             }

crate_universe/src/cli/splice.rs

@@ -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")?;
@@ -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")?;
 
@@ -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)

crate_universe/src/cli/vendor.rs

@@ -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,
     )?;
 
@@ -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(
@@ -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")?;
     }
 

crate_universe/src/metadata.rs

@@ -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;
@@ -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::*;
@@ -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 {
@@ -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

crate_universe/src/metadata/cargo_toml_path.rs

@@ -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()
+    }
+}

crate_universe/src/metadata/cargo_tree_resolver.rs

@@ -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;
@@ -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!(
@@ -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();