From ae674a8eb3964c17b40ec4eafe05ebaabc2b1bc4 Mon Sep 17 00:00:00 2001 From: Joseph Ryan Date: Fri, 24 Mar 2023 16:10:46 -0700 Subject: [PATCH] Working patching MVP Co-authored-by: Dan Johnson --- crate_universe/src/context/crate_context.rs | 204 +++++++++++--------- crate_universe/src/rendering.rs | 35 ++-- crate_universe/src/utils/starlark.rs | 6 +- crate_universe/src/utils/starlark/glob.rs | 51 ++++- 4 files changed, 188 insertions(+), 108 deletions(-) diff --git a/crate_universe/src/context/crate_context.rs b/crate_universe/src/context/crate_context.rs index cd5722f0d7..eb1989cb44 100644 --- a/crate_universe/src/context/crate_context.rs +++ b/crate_universe/src/context/crate_context.rs @@ -1,9 +1,9 @@ //! Crate specific information embedded into [crate::context::Context] objects. use std::collections::{BTreeMap, BTreeSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; -use cargo_metadata::{Node, Package, PackageId}; +use cargo_metadata::{Node, Package, PackageId, Target}; use serde::{Deserialize, Serialize}; use crate::config::{CrateId, GenBinaries}; @@ -12,7 +12,9 @@ use crate::metadata::{ }; use crate::splicing::WorkspaceMetadata; use crate::utils::sanitize_module_name; -use crate::utils::starlark::{Glob, SelectList, SelectMap, SelectStringDict, SelectStringList}; +use crate::utils::starlark::{ + Glob, GlobOrLabels, Label, SelectList, SelectMap, SelectStringDict, SelectStringList, +}; #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone)] pub struct CrateDependency { @@ -43,8 +45,12 @@ pub struct TargetAttributes { /// The path to the crate's root source file, relative to the manifest. pub crate_root: Option, - /// A glob pattern of all source files required by the target - pub srcs: Glob, + /// A glob pattern of all source files required by the target or a label + /// pointing to a filegroup containing said glob (used for patching) + pub srcs: GlobOrLabels, + + /// A label for overriding compile_data, used for patching + pub compile_data: Option, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone)] @@ -656,95 +662,100 @@ impl CrateContext { .targets .iter() .flat_map(|target| { + let attrs = get_attributes(target, package, workspace, package_root); target.kind.iter().filter_map(move |kind| { - // Unfortunately, The package graph and resolve graph of cargo metadata have different representations - // for the crate names (resolve graph sanitizes names to match module names) so to get the rest of this - // content to align when rendering, the package target names are always sanitized. - let crate_name = sanitize_module_name(&target.name); - - // Locate the crate's root source file relative to the package root normalized for unix - let crate_root = pathdiff::diff_paths(&target.src_path, package_root).map( - // Normalize the path so that it always renders the same regardless of platform - |root| root.to_string_lossy().replace('\\', "/"), - ); - println!("{}", package.id); - if package.id.repr.contains("(path+file://") { - println!(""); - println!("{crate_name} {kind} {:?}", &crate_root); - println!("src: {:?}", &target.src_path); - println!("pkg: {:?}", package_root); - println!("root: {:?}", &crate_root); - println!("workspace: {:?}", workspace.workspace_prefix); - let temp_components = std::env::temp_dir().components().count() + 1; - println!("temp dir components to drop: {temp_components}"); - let real_root: PathBuf = - package_root.components().skip(temp_components).collect(); - println!("ACTUAL FOR REAL ROOT: {}", real_root.to_string_lossy()); - println!(""); - } - let crate_root = crate_root.map(|r| { - if package.id.repr.contains("(path+file://") { - let temp_components = std::env::temp_dir().components().count() + 1; - package_root - .components() - .skip(temp_components) - .collect::() - .join(r) - .to_string_lossy() - .to_string() - } else { - r - } - }); - - // Conditionally check to see if the dependencies is a build-script target - if include_build_scripts && kind == "custom-build" { - return Some(Rule::BuildScript(TargetAttributes { - crate_name, - crate_root, - srcs: Glob::new_rust_srcs(), - })); - } - - // Check to see if the dependencies is a proc-macro target + let attrs = attrs.clone(); if kind == "proc-macro" { - return Some(Rule::ProcMacro(TargetAttributes { - crate_name, - crate_root, - srcs: Glob::new_rust_srcs(), - })); - } - - // Check to see if the dependencies is a library target - if ["lib", "rlib"].contains(&kind.as_str()) { - return Some(Rule::Library(TargetAttributes { - crate_name, - crate_root, - srcs: Glob::new_rust_srcs(), - })); - } - - // Check if the target kind is binary and is one of the ones included in gen_binaries - if kind == "bin" - && match gen_binaries { + Some(Rule::ProcMacro(attrs)) + } else if ["lib", "rlib"].contains(&kind.as_str()) { + Some(Rule::Library(attrs)) + } else if include_build_scripts && kind == "custom-build" { + let build_script_crate_root = attrs + .crate_root + .map(|s| s.replace(":crate_root", ":build_script_crate_root")); + Some(Rule::BuildScript(TargetAttributes { + crate_root: build_script_crate_root, + ..attrs + })) + } else if kind == "bin" { + match gen_binaries { GenBinaries::All => true, GenBinaries::Some(set) => set.contains(&target.name), } - { - return Some(Rule::Binary(TargetAttributes { - crate_name: target.name.clone(), - crate_root, - srcs: Glob::new_rust_srcs(), - })); + .then(|| { + Rule::Binary(TargetAttributes { + crate_name: target.name.clone(), + ..attrs + }) + }) + } else { + None } - - None }) }) .collect() } } +fn get_attributes( + target: &Target, + package: &Package, + workspace: &WorkspaceMetadata, + package_root: &Path, +) -> TargetAttributes { + // Unfortunately, The package graph and resolve graph of cargo metadata have + // different representations for the crate names (resolve graph sanitizes + // names to match module names) so to get the rest of this content to align + // when rendering, the package target names are always sanitized. + let crate_name = sanitize_module_name(&target.name); + + // Locate the crate's root source file relative to the package root normalized + // for unix + let crate_root = pathdiff::diff_paths(&target.src_path, package_root).map( + // Normalize the path so that it always renders the same regardless of platform + |root| root.to_string_lossy().replace('\\', "/"), + ); + let local_patch = package.id.repr.contains("(path+file://"); + let temp_components = std::env::temp_dir().components().count() + 1; + let real_root: PathBuf = package_root.components().skip(temp_components).collect(); + if !local_patch || real_root.as_os_str().is_empty() { + TargetAttributes { + crate_name, + crate_root, + srcs: Glob::new_rust_srcs().into(), + compile_data: None, + } + } else { + let root = real_root.display(); + let pkg = if let Some(workspace) = &workspace.workspace_prefix { + format!("{workspace}/{}", root) + } else { + root.to_string() + }; + // TODO: remove once added to help-docs + println!("\nThere's a patch crate at '//{pkg}'."); + println!("Make sure that '//{pkg}/BUILD.bazel' exposes the following filegroups:"); + println!("'crate_root', 'srcs', 'compile_data', and (if necessary) 'build_script'"); + let srcs = GlobOrLabels::Labels(vec![Label { + repository: None, + package: Some(pkg.clone()), + target: "srcs".to_string(), + }]); + let compile_data = Some(GlobOrLabels::Labels(vec![Label { + repository: None, + package: Some(pkg.clone()), + target: "compile_data".to_string(), + }])); + + TargetAttributes { + crate_name, + crate_root: Some(format!("//{pkg}:crate_root")), + srcs, + compile_data, + } + } +} + #[cfg(test)] mod test { use super::*; @@ -787,7 +798,8 @@ mod test { BTreeSet::from([Rule::Library(TargetAttributes { crate_name: "common".to_owned(), crate_root: Some("lib.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, })]), ); } @@ -819,7 +831,7 @@ mod test { let include_build_scripts = false; let context = CrateContext::new( crate_annotation, - &annotations.metadata.packages, + &annotations.metadata, &annotations.lockfile.crates, &pairred_extras, &annotations.features, @@ -834,12 +846,14 @@ mod test { Rule::Library(TargetAttributes { crate_name: "common".to_owned(), crate_root: Some("lib.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, }), Rule::Binary(TargetAttributes { crate_name: "common-bin".to_owned(), crate_root: Some("main.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, }), ]), ); @@ -882,7 +896,7 @@ mod test { let include_build_scripts = true; let context = CrateContext::new( crate_annotation, - &annotations.metadata.packages, + &annotations.metadata, &annotations.lockfile.crates, &annotations.pairred_extras, &annotations.features, @@ -898,12 +912,14 @@ mod test { Rule::Library(TargetAttributes { crate_name: "openssl_sys".to_owned(), crate_root: Some("src/lib.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, }), Rule::BuildScript(TargetAttributes { crate_name: "build_script_main".to_owned(), crate_root: Some("build/main.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, }) ]), ); @@ -927,7 +943,7 @@ mod test { let include_build_scripts = false; let context = CrateContext::new( crate_annotation, - &annotations.metadata.packages, + &annotations.metadata, &annotations.lockfile.crates, &annotations.pairred_extras, &annotations.features, @@ -942,7 +958,8 @@ mod test { BTreeSet::from([Rule::Library(TargetAttributes { crate_name: "openssl_sys".to_owned(), crate_root: Some("src/lib.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, })]), ); } @@ -962,7 +979,7 @@ mod test { let include_build_scripts = false; let context = CrateContext::new( crate_annotation, - &annotations.metadata.packages, + &annotations.metadata, &annotations.lockfile.crates, &annotations.pairred_extras, &annotations.features, @@ -977,7 +994,8 @@ mod test { BTreeSet::from([Rule::Library(TargetAttributes { crate_name: "sysinfo".to_owned(), crate_root: Some("src/lib.rs".to_owned()), - srcs: Glob::new_rust_srcs(), + srcs: Glob::new_rust_srcs().into(), + compile_data: None, })]), ); } diff --git a/crate_universe/src/rendering.rs b/crate_universe/src/rendering.rs index d44a1d67e2..b10a5940ab 100644 --- a/crate_universe/src/rendering.rs +++ b/crate_universe/src/rendering.rs @@ -328,11 +328,17 @@ impl Renderer { build_script_env: attrs .map_or_else(SelectDict::default, |attrs| attrs.build_script_env.clone()) .remap_configurations(platforms), - compile_data: make_data( - platforms, - &empty_set, - attrs.map_or(&empty_list, |attrs| &attrs.compile_data), - ), + compile_data: { + let mut data = make_data( + platforms, + &empty_set, + attrs.map_or(&empty_list, |attrs| &attrs.compile_data), + ); + if let Some(cd) = &target.compile_data { + data.glob = cd.clone(); + } + data + }, crate_features: SelectList::from(&krate.common_attrs.crate_features) .map_configuration_names(|triple| { render_platform_constraint_label(&self.config.platforms_template, &triple) @@ -479,11 +485,17 @@ impl Renderer { target: &TargetAttributes, ) -> Result { Ok(CommonAttrs { - compile_data: make_data( - platforms, - &krate.common_attrs.compile_data_glob, - &krate.common_attrs.compile_data, - ), + compile_data: { + let mut data = make_data( + platforms, + &krate.common_attrs.compile_data_glob, + &krate.common_attrs.compile_data, + ); + if let Some(cd) = &target.compile_data { + data.glob = cd.clone(); + } + data + }, crate_features: SelectList::from(&krate.common_attrs.crate_features) .map_configuration_names(|triple| { render_platform_constraint_label(&self.config.platforms_template, &triple) @@ -719,7 +731,8 @@ fn make_data(platforms: &Platforms, glob: &BTreeSet, select: &SelectList .iter() .map(|&glob| glob.to_owned()) .collect(), - }, + } + .into(), select: select.clone().remap_configurations(platforms), } } diff --git a/crate_universe/src/utils/starlark.rs b/crate_universe/src/utils/starlark.rs index 64e260f54d..47aa63bf66 100644 --- a/crate_universe/src/utils/starlark.rs +++ b/crate_universe/src/utils/starlark.rs @@ -122,7 +122,7 @@ pub struct CargoBuildScript { serialize_with = "SelectList::serialize_starlark" )] pub rustc_flags: SelectList>, - pub srcs: Glob, + pub srcs: GlobOrLabels, #[serde(skip_serializing_if = "Set::is_empty")] pub tags: Set, #[serde( @@ -232,14 +232,14 @@ pub struct CommonAttrs { pub rustc_env_files: SelectList>, #[serde(skip_serializing_if = "Vec::is_empty")] pub rustc_flags: Vec, - pub srcs: Glob, + pub srcs: GlobOrLabels, #[serde(skip_serializing_if = "Set::is_empty")] pub tags: Set, pub version: String, } pub struct Data { - pub glob: Glob, + pub glob: GlobOrLabels, pub select: SelectList>, } diff --git a/crate_universe/src/utils/starlark/glob.rs b/crate_universe/src/utils/starlark/glob.rs index a7bcebbbad..bbe352a5c7 100644 --- a/crate_universe/src/utils/starlark/glob.rs +++ b/crate_universe/src/utils/starlark/glob.rs @@ -2,8 +2,11 @@ use std::collections::BTreeSet; use std::fmt; use serde::de::value::{MapAccessDeserializer, SeqAccessDeserializer}; -use serde::de::{Deserialize, Deserializer, MapAccess, SeqAccess, Visitor}; +use serde::de::{Deserializer, MapAccess, SeqAccess, Visitor}; use serde::ser::{Serialize, SerializeStruct, Serializer}; +use serde::Deserialize; + +use super::Label; #[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct Glob { @@ -91,3 +94,49 @@ impl<'de> Visitor<'de> for GlobVisitor { }) } } + +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Deserialize)] +#[serde(untagged)] +pub enum GlobOrLabels { + Glob(Glob), + Labels(Vec