diff --git a/fixtures/workspace/inside-outside/external/Cargo.toml b/fixtures/workspace/inside-outside/external/Cargo.toml index 913eacf44f1..5409e44d629 100644 --- a/fixtures/workspace/inside-outside/external/Cargo.toml +++ b/fixtures/workspace/inside-outside/external/Cargo.toml @@ -4,6 +4,15 @@ version = "0.1.0" authors = ["Fake Author "] edition = "2018" +[lib] +name = "external2" +crate-type = ["rlib"] + +# Create an example with a crate-type that is also rlib. +[[example]] +name = "external3" +crate-type = ["rlib"] + [dependencies] transitive = { path = "../transitive" } bytes = { version = "0.5", optional = true } diff --git a/fixtures/workspace/inside-outside/external/examples/external3.rs b/fixtures/workspace/inside-outside/external/examples/external3.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/guppy/src/graph/build.rs b/guppy/src/graph/build.rs index db16b5092cb..8ef136a8bab 100644 --- a/guppy/src/graph/build.rs +++ b/guppy/src/graph/build.rs @@ -24,14 +24,15 @@ use semver::{Version, VersionReq}; use smallvec::SmallVec; use std::{ borrow::Cow, + cell::RefCell, collections::{BTreeMap, HashSet}, - sync::Arc, + rc::Rc, }; use target_spec::TargetSpec; impl PackageGraph { /// Constructs a new `PackageGraph` instances from the given metadata. - pub(crate) fn build(metadata: Metadata) -> Result> { + pub(crate) fn build(mut metadata: Metadata) -> Result> { // resolve_nodes is missing if the metadata was generated with --no-deps. let resolve_nodes = metadata.resolve.map(|r| r.nodes).unwrap_or_default(); @@ -44,11 +45,11 @@ impl PackageGraph { let workspace_root = metadata.workspace_root; let mut build_state = GraphBuildState::new( - &metadata.packages, + &mut metadata.packages, resolve_nodes, &workspace_root, &workspace_members, - ); + )?; let packages: AHashMap<_, _> = metadata .packages @@ -143,9 +144,9 @@ impl WorkspaceImpl { /// Helper struct for building up dependency graph. struct GraphBuildState<'a> { dep_graph: Graph, - package_data: AHashMap>, + package_data: AHashMap>, // The above, except by package name. - by_package_name: AHashMap>>, + by_package_name: AHashMap>>, // The values of resolve_data are the resolved dependencies. This is mutated so it is stored // separately from package_data. @@ -155,12 +156,13 @@ struct GraphBuildState<'a> { } impl<'a> GraphBuildState<'a> { + /// This method drains the list of targets from the package. fn new( - packages: &[Package], + packages: &mut [Package], resolve_nodes: Vec, workspace_root: &'a Utf8Path, workspace_members: &'a HashSet, - ) -> Self { + ) -> Result> { // Precomputing the edge count is a roughly 5% performance improvement. let edge_count = resolve_nodes .iter() @@ -168,13 +170,14 @@ impl<'a> GraphBuildState<'a> { .sum::(); let mut dep_graph = Graph::with_capacity(packages.len(), edge_count); - let all_package_data = packages - .iter() + let all_package_data: AHashMap<_, _> = packages + .iter_mut() .map(|package| PackageDataValue::new(package, &mut dep_graph)) - .collect::>(); + .collect::>()?; + // While it is possible to have duplicate names so the hash map is smaller, just make this // as big as package_data. - let mut by_package_name: AHashMap>> = + let mut by_package_name: AHashMap>> = AHashMap::with_capacity(all_package_data.len()); for package_data in all_package_data.values() { by_package_name @@ -195,14 +198,14 @@ impl<'a> GraphBuildState<'a> { }) .collect(); - Self { + Ok(Self { dep_graph, package_data: all_package_data, by_package_name, resolve_data, workspace_root, workspace_members, - } + }) } fn process_package( @@ -210,7 +213,8 @@ impl<'a> GraphBuildState<'a> { package: Package, ) -> Result<(PackageId, PackageMetadataImpl), Box> { let package_id = PackageId::from_metadata(package.id); - let package_data = self.package_data(&package_id)?.clone(); + let (package_data, build_targets) = + self.package_data_and_remove_build_targets(&package_id)?; let source = if self.workspace_members.contains(&package_id) { PackageSourceImpl::Workspace(self.workspace_path(&package_id, &package.manifest_path)?) @@ -235,12 +239,6 @@ impl<'a> GraphBuildState<'a> { PackageSourceImpl::create_path(dirname, self.workspace_root) }; - let mut build_targets = BuildTargets::new(&package_id); - for build_target in package.targets { - build_targets.add(build_target)?; - } - let build_targets = build_targets.finish(); - // resolved_deps is missing if the metadata was generated with --no-deps. let resolved_deps = self.resolve_data.remove(&package_id).unwrap_or_default(); @@ -382,11 +380,16 @@ impl<'a> GraphBuildState<'a> { )) } - fn package_data(&self, id: &PackageId) -> Result<&Arc, Box> { - self.package_data.get(id).ok_or_else(|| { + fn package_data_and_remove_build_targets( + &self, + id: &PackageId, + ) -> Result<(Rc, BuildTargetMap), Box> { + let package_data = self.package_data.get(id).ok_or_else(|| { Error::PackageGraphConstructError(format!("no package data found for package '{}'", id)) - .into() - }) + })?; + let package_data = package_data.clone(); + let build_targets = std::mem::take(&mut *package_data.build_targets.borrow_mut()); + Ok((package_data, build_targets)) } /// Computes the workspace path for this package. Errors if this package is not in the @@ -425,36 +428,38 @@ struct PackageDataValue { package_ix: NodeIndex, name: String, resolved_name: ResolvedName, + // build_targets is used in two spots: in the constructor here, and removed from this field in + // package_data_and_remove_build_targets. + build_targets: RefCell, version: Version, } impl PackageDataValue { fn new( - package: &Package, + package: &mut Package, dep_graph: &mut Graph, - ) -> (PackageId, Arc) { + ) -> Result<(PackageId, Rc), Box> { let package_id = PackageId::from_metadata(package.id.clone()); let package_ix = dep_graph.add_node(package_id.clone()); - // The resolved name is determined by lib.name or proc-macro.name (only one should exist). - // So look for a lib target if it exists. - fn lib_target(package: &Package) -> Option<&Target> { - package.targets.iter().find(|target| { - target - .kind - .iter() - .any(|kind| kind == "lib" || kind == "proc-macro") - }) + // Build up the list of build targets -- this will be used to construct the resolved_name. + let mut build_targets = BuildTargets::new(&package_id); + for build_target in package.targets.drain(..) { + build_targets.add(build_target)?; } + let build_targets = build_targets.finish(); - let resolved_name = match lib_target(package) { - Some(lib_target) => { - if lib_target.name != package.name { - // This means that `lib.name` was specified. - ResolvedName::LibNameSpecified(lib_target.name.clone()) + let resolved_name = match build_targets.get(&OwnedBuildTargetId::Library) { + Some(target) => { + let lib_name = target + .lib_name + .as_deref() + .expect("lib_name is always specified for library targets"); + if lib_name != package.name { + ResolvedName::LibNameSpecified(lib_name.to_string()) } else { // The resolved name is the same as the package name. - ResolvedName::LibNameNotSpecified(lib_target.name.replace('-', "_")) + ResolvedName::LibNameNotSpecified(lib_name.replace('-', "_")) } } None => { @@ -467,10 +472,11 @@ impl PackageDataValue { package_ix, name: package.name.clone(), resolved_name, + build_targets: RefCell::new(build_targets), version: package.version.clone(), }; - (package_id, Arc::new(value)) + Ok((package_id, Rc::new(value))) } } @@ -543,9 +549,11 @@ impl NamedFeatureDep { } } +type BuildTargetMap = BTreeMap; + struct BuildTargets<'a> { package_id: &'a PackageId, - targets: BTreeMap, + targets: BuildTargetMap, } impl<'a> BuildTargets<'a> { @@ -661,7 +669,7 @@ impl<'a> BuildTargets<'a> { list.iter().any(|kind| kind.as_str() == "proc-macro") } - fn finish(self) -> BTreeMap { + fn finish(self) -> BuildTargetMap { self.targets } } @@ -670,7 +678,7 @@ struct DependencyResolver<'g> { from_id: &'g PackageId, /// The package data, inherited from the graph build state. - package_data: &'g AHashMap>, + package_data: &'g AHashMap>, /// This is a list of dependency requirements. We don't know the package ID yet so we don't have /// a great key to work with. This could be improved in the future by matching on requirements @@ -682,8 +690,8 @@ impl<'g> DependencyResolver<'g> { /// Constructs a new resolver using the provided package data and dependencies. fn new( from_id: &'g PackageId, - package_data: &'g AHashMap>, - by_package_name: &'g AHashMap>>, + package_data: &'g AHashMap>, + by_package_name: &'g AHashMap>>, package_deps: impl IntoIterator, ) -> Self { let mut dep_reqs = DependencyReqs::default(); @@ -728,7 +736,7 @@ impl<'g> DependencyResolver<'g> { dep_kinds: &'a [DepKindInfo], ) -> Result< ( - &'g Arc, + &'g Rc, impl Iterator + 'a, ), Error,