Skip to content

Commit

Permalink
Eliminate all use of hash-based collections from crate_universe (#1737)
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay authored Jan 1, 2023
1 parent 31073ff commit 52e02c2
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 31 deletions.
8 changes: 4 additions & 4 deletions crate_universe/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use serde::de::Visitor;
use serde::{Deserialize, Serialize, Serializer};

/// Representations of different kinds of crate vendoring into workspaces.
#[derive(Debug, Serialize, Deserialize, Hash, Clone)]
#[derive(Debug, Serialize, Deserialize, Clone)]
#[serde(rename_all = "lowercase")]
pub enum VendorMode {
/// Crates having full source being vendored into a workspace
Expand All @@ -37,7 +37,7 @@ impl std::fmt::Display for VendorMode {
}
}

#[derive(Debug, Default, Hash, Serialize, Deserialize, Clone)]
#[derive(Debug, Default, Serialize, Deserialize, Clone)]
#[serde(deny_unknown_fields)]
pub struct RenderConfig {
/// The name of the repository being rendered
Expand Down Expand Up @@ -141,7 +141,7 @@ pub enum Checksumish {
},
}

#[derive(Debug, Default, Hash, Deserialize, Serialize, Clone)]
#[derive(Debug, Default, Deserialize, Serialize, Clone)]
pub struct CrateAnnotations {
/// Determins whether or not Cargo build scripts should be generated for the current package
pub gen_build_script: Option<bool>,
Expand Down Expand Up @@ -328,7 +328,7 @@ impl Sum for CrateAnnotations {
}

/// A unique identifier for Crates
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone)]
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
pub struct CrateId {
/// The name of the crate
pub name: String,
Expand Down
6 changes: 3 additions & 3 deletions crate_universe/src/context/platforms.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::collections::{BTreeMap, BTreeSet};

use anyhow::{anyhow, Context, Result};
use cfg_expr::targets::{get_builtin_target_by_triple, TargetInfo};
Expand Down Expand Up @@ -57,7 +57,7 @@ pub fn resolve_cfg_platforms(
// in order to parse configurations, the text is renamed for the check but the
// original is retained for comaptibility with the manifest.
let rename = |cfg: &str| -> String { format!("cfg(target = \"{cfg}\")") };
let original_cfgs: HashMap<String, String> = configurations
let original_cfgs: BTreeMap<String, String> = configurations
.iter()
.filter(|cfg| !cfg.starts_with("cfg("))
.map(|cfg| (rename(cfg), cfg.clone()))
Expand Down Expand Up @@ -190,7 +190,7 @@ mod test {

#[test]
fn resolve_targeted() {
let data = HashMap::from([
let data = BTreeMap::from([
(
r#"cfg(target = "x86_64-unknown-linux-gnu")"#.to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
Expand Down
4 changes: 2 additions & 2 deletions crate_universe/src/lockfile.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Utility module for interracting with different kinds of lock files
use std::collections::HashMap;
use std::collections::BTreeMap;
use std::convert::TryFrom;
use std::ffi::OsStr;
use std::fs;
Expand Down Expand Up @@ -152,7 +152,7 @@ impl Digest {
// computed consistently. If a new binary is released then this
// condition should be removed
// https://github.com/rust-lang/cargo/issues/10547
let corrections = HashMap::from([
let corrections = BTreeMap::from([
(
"cargo 1.60.0 (d1fd9fe 2022-03-01)",
"cargo 1.60.0 (d1fd9fe2c 2022-03-01)",
Expand Down
2 changes: 1 addition & 1 deletion crate_universe/src/rendering/template_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl TemplateEngine {
pub fn render_crate_build_files<'a>(
&self,
ctx: &'a Context,
) -> Result<HashMap<&'a CrateId, String>> {
) -> Result<BTreeMap<&'a CrateId, String>> {
// Create the render context with the global planned context to be
// reused when rendering crates.
let mut context = self.new_tera_ctx();
Expand Down
4 changes: 2 additions & 2 deletions crate_universe/src/splicing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pub(crate) mod cargo_config;
mod splicer;

use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::collections::{BTreeMap, BTreeSet};
use std::convert::TryFrom;
use std::fs;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -193,7 +193,7 @@ impl TryFrom<serde_json::Value> for WorkspaceMetadata {
impl WorkspaceMetadata {
fn new(
splicing_manifest: &SplicingManifest,
member_manifests: HashMap<&PathBuf, String>,
member_manifests: BTreeMap<&PathBuf, String>,
) -> Result<Self> {
let mut package_prefixes: BTreeMap<String, String> = member_manifests
.iter()
Expand Down
31 changes: 15 additions & 16 deletions crate_universe/src/splicing/splicer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Utility for creating valid Cargo workspaces
use std::collections::{BTreeSet, HashMap};
use std::collections::{BTreeMap, BTreeSet};
use std::fs;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -33,7 +33,7 @@ pub enum SplicerKind<'a> {
},
/// Splice a manifest from multiple disjoint Cargo manifests.
MultiPackage {
manifests: &'a HashMap<PathBuf, Manifest>,
manifests: &'a BTreeMap<PathBuf, Manifest>,
splicing_manifest: &'a SplicingManifest,
},
}
Expand All @@ -43,12 +43,12 @@ const IGNORE_LIST: &[&str] = &[".git", "bazel-*", ".svn"];

impl<'a> SplicerKind<'a> {
pub fn new(
manifests: &'a HashMap<PathBuf, Manifest>,
manifests: &'a BTreeMap<PathBuf, Manifest>,
splicing_manifest: &'a SplicingManifest,
cargo: &Path,
) -> Result<Self> {
// First check for any workspaces in the provided manifests
let workspace_owned: HashMap<&PathBuf, &Manifest> = manifests
let workspace_owned: BTreeMap<&PathBuf, &Manifest> = manifests
.iter()
.filter(|(_, manifest)| is_workspace_owned(manifest))
.collect();
Expand All @@ -57,11 +57,10 @@ impl<'a> SplicerKind<'a> {

if !workspace_owned.is_empty() {
// Filter for the root workspace manifest info
let (mut workspace_roots, workspace_packages): (
HashMap<&PathBuf, &Manifest>,
HashMap<&PathBuf, &Manifest>,
let (workspace_roots, workspace_packages): (
BTreeMap<&PathBuf, &Manifest>,
BTreeMap<&PathBuf, &Manifest>,
) = workspace_owned
.clone()
.into_iter()
.partition(|(_, manifest)| is_workspace_root(manifest));

Expand Down Expand Up @@ -96,7 +95,7 @@ impl<'a> SplicerKind<'a> {

// Ensure all workspace owned manifests are members of the one workspace root
// UNWRAP: Safe because we've checked workspace_roots isn't empty.
let (root_manifest_path, root_manifest) = workspace_roots.drain().last().unwrap();
let (root_manifest_path, root_manifest) = workspace_roots.into_iter().next().unwrap();
let external_workspace_members: BTreeSet<String> = workspace_packages
.into_iter()
.filter(|(manifest_path, _)| {
Expand Down Expand Up @@ -229,7 +228,7 @@ impl<'a> SplicerKind<'a> {
}

let root_manifest_path = workspace_dir.join("Cargo.toml");
let member_manifests = HashMap::from([(*path, String::new())]);
let member_manifests = BTreeMap::from([(*path, String::new())]);

// Write the generated metadata to the manifest
let workspace_metadata = WorkspaceMetadata::new(splicing_manifest, member_manifests)?;
Expand Down Expand Up @@ -271,7 +270,7 @@ impl<'a> SplicerKind<'a> {
}

let root_manifest_path = workspace_dir.join("Cargo.toml");
let member_manifests = HashMap::from([(*path, String::new())]);
let member_manifests = BTreeMap::from([(*path, String::new())]);

// Write the generated metadata to the manifest
let workspace_metadata = WorkspaceMetadata::new(splicing_manifest, member_manifests)?;
Expand All @@ -286,7 +285,7 @@ impl<'a> SplicerKind<'a> {
/// Implementation for splicing together multiple Cargo packages/workspaces
fn splice_multi_package(
workspace_dir: &Path,
manifests: &&HashMap<PathBuf, Manifest>,
manifests: &&BTreeMap<PathBuf, Manifest>,
splicing_manifest: &&SplicingManifest,
) -> Result<SplicedManifest> {
let mut manifest = default_cargo_workspace_manifest(&splicing_manifest.resolver_version);
Expand Down Expand Up @@ -403,9 +402,9 @@ impl<'a> SplicerKind<'a> {
/// Cargo workspace members.
fn inject_workspace_members<'b>(
root_manifest: &mut Manifest,
manifests: &'b HashMap<PathBuf, Manifest>,
manifests: &'b BTreeMap<PathBuf, Manifest>,
workspace_dir: &Path,
) -> Result<HashMap<&'b PathBuf, String>> {
) -> Result<BTreeMap<&'b PathBuf, String>> {
manifests
.iter()
.map(|(path, manifest)| {
Expand Down Expand Up @@ -476,7 +475,7 @@ impl<'a> SplicerKind<'a> {

pub struct Splicer {
workspace_dir: PathBuf,
manifests: HashMap<PathBuf, Manifest>,
manifests: BTreeMap<PathBuf, Manifest>,
splicing_manifest: SplicingManifest,
}

Expand All @@ -491,7 +490,7 @@ impl Splicer {
.with_context(|| format!("Failed to read manifest at {}", path.display()))?;
Ok((path.clone(), m))
})
.collect::<Result<HashMap<PathBuf, Manifest>>>()?;
.collect::<Result<BTreeMap<PathBuf, Manifest>>>()?;

Ok(Self {
workspace_dir,
Expand Down
6 changes: 3 additions & 3 deletions crate_universe/tools/urls_generator/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! A helper tool for generating urls and sha256 checksums of cargo-bazel binaries and writing them to a module.
use std::collections::HashMap;
use std::collections::BTreeMap;
use std::io::{BufRead, BufReader};
use std::path::{Path, PathBuf};
use std::process::Command;
Expand Down Expand Up @@ -120,12 +120,12 @@ CARGO_BAZEL_LABEL = Label("@cargo_bazel_bootstrap//:binary")
"#;

fn render_module(artifacts: &[Artifact]) -> String {
let urls: HashMap<&String, &String> = artifacts
let urls: BTreeMap<&String, &String> = artifacts
.iter()
.map(|artifact| (&artifact.triple, &artifact.url))
.collect();

let sha256s: HashMap<&String, &String> = artifacts
let sha256s: BTreeMap<&String, &String> = artifacts
.iter()
.map(|artifact| (&artifact.triple, &artifact.sha256))
.collect();
Expand Down

0 comments on commit 52e02c2

Please sign in to comment.