Wrap Package/Target in Rc.

This is intended to make it easier to deal with Unit lifetimes.

Author: ehuss

src/cargo/core/compiler/build_context/mod.rs

@@ -32,7 +32,7 @@ pub struct BuildContext<'a, 'cfg> {
     pub packages: &'a PackageSet<'cfg>,
 
     /// Source of interning new units as they're created.
-    pub units: &'a UnitInterner<'a>,
+    pub units: &'a UnitInterner,
 
     /// Information about rustc and the target platform.
     pub target_data: RustcTargetData,
@@ -45,7 +45,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         config: &'cfg Config,
         build_config: &'a BuildConfig,
         profiles: Profiles,
-        units: &'a UnitInterner<'a>,
+        units: &'a UnitInterner,
         extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
         target_data: RustcTargetData,
     ) -> CargoResult<BuildContext<'a, 'cfg>> {

src/cargo/core/compiler/compilation.rs

@@ -2,6 +2,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
+use std::rc::Rc;
 
 use cargo_platform::CfgExpr;
 use semver::Version;
@@ -14,9 +15,9 @@ use crate::util::{self, config, join_paths, process, CargoResult, Config, Proces
 /// Structure with enough information to run `rustdoc --test`.
 pub struct Doctest {
     /// The package being doc-tested.
-    pub package: Package,
+    pub package: Rc<Package>,
     /// The target being tested (currently always the package's lib).
-    pub target: Target,
+    pub target: Rc<Target>,
     /// Arguments needed to pass to rustdoc to run this test.
     pub args: Vec<OsString>,
     /// Whether or not -Zunstable-options is needed.
@@ -27,7 +28,7 @@ pub struct Doctest {
 pub struct Compilation<'cfg> {
     /// An array of all tests created during this compilation.
     /// `(package, target, path_to_test_exe)`
-    pub tests: Vec<(Package, Target, PathBuf)>,
+    pub tests: Vec<(Rc<Package>, Rc<Target>, PathBuf)>,
 
     /// An array of all binaries created.
     pub binaries: Vec<PathBuf>,

src/cargo/core/compiler/context/compilation_files.rs

@@ -631,7 +631,7 @@ fn compute_metadata<'a, 'cfg>(
 
     hash_rustc_version(bcx, &mut hasher);
 
-    if cx.bcx.ws.is_member(unit.pkg) {
+    if cx.bcx.ws.is_member(&unit.pkg) {
         // This is primarily here for clippy. This ensures that the clippy
         // artifacts are separate from the `check` ones.
         if let Some(path) = &cx.bcx.rustc().workspace_wrapper {

src/cargo/core/compiler/context/mod.rs

@@ -1,6 +1,7 @@
 #![allow(deprecated)]
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::path::PathBuf;
+use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 
 use filetime::FileTime;
@@ -179,8 +180,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
                 if unit.mode == CompileMode::Test {
                     self.compilation.tests.push((
-                        unit.pkg.clone(),
-                        unit.target.clone(),
+                        Rc::clone(&unit.pkg),
+                        Rc::clone(&unit.target),
                         output.path.clone(),
                     ));
                 } else if unit.target.is_executable() {
@@ -212,8 +213,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                 let mut unstable_opts = false;
                 let args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
                 self.compilation.to_doc_test.push(compilation::Doctest {
-                    package: unit.pkg.clone(),
-                    target: unit.target.clone(),
+                    package: Rc::clone(&unit.pkg),
+                    target: Rc::clone(&unit.target),
                     args,
                     unstable_opts,
                 });

src/cargo/core/compiler/custom_build.rs

@@ -177,7 +177,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     // `Profiles::get_profile_run_custom_build` so that those flags get
     // carried over.
     let to_exec = to_exec.into_os_string();
-    let mut cmd = cx.compilation.host_process(to_exec, unit.pkg)?;
+    let mut cmd = cx.compilation.host_process(to_exec, &unit.pkg)?;
     let debug = unit.profile.debuginfo.unwrap_or(0) != 0;
     cmd.env("OUT_DIR", &script_out_dir)
         .env("CARGO_MANIFEST_DIR", unit.pkg.root())

src/cargo/core/compiler/fingerprint.rs

@@ -1286,7 +1286,7 @@ fn calculate_run_custom_build<'a, 'cfg>(
     // the whole crate.
     let (gen_local, overridden) = build_script_local_fingerprints(cx, unit);
     let deps = &cx.build_explicit_deps[unit];
-    let local = (gen_local)(deps, Some(&|| pkg_fingerprint(cx.bcx, unit.pkg)))?.unwrap();
+    let local = (gen_local)(deps, Some(&|| pkg_fingerprint(cx.bcx, &unit.pkg)))?.unwrap();
     let output = deps.build_script_output.clone();
 
     // Include any dependencies of our execution, which is typically just the

src/cargo/core/compiler/job_queue.rs

@@ -932,15 +932,15 @@ impl<'a, 'cfg> DrainState<'a, 'cfg> {
             Dirty => {
                 if unit.mode.is_doc() {
                     self.documented.insert(unit.pkg.package_id());
-                    config.shell().status("Documenting", unit.pkg)?;
+                    config.shell().status("Documenting", &unit.pkg)?;
                 } else if unit.mode.is_doc_test() {
                     // Skip doc test.
                 } else {
                     self.compiled.insert(unit.pkg.package_id());
                     if unit.mode.is_check() {
-                        config.shell().status("Checking", unit.pkg)?;
+                        config.shell().status("Checking", &unit.pkg)?;
                     } else {
-                        config.shell().status("Compiling", unit.pkg)?;
+                        config.shell().status("Compiling", &unit.pkg)?;
                     }
                 }
             }
@@ -950,7 +950,7 @@ impl<'a, 'cfg> DrainState<'a, 'cfg> {
                     && !(unit.mode.is_doc_test() && self.compiled.contains(&unit.pkg.package_id()))
                 {
                     self.compiled.insert(unit.pkg.package_id());
-                    config.shell().verbose(|c| c.status("Fresh", unit.pkg))?;
+                    config.shell().verbose(|c| c.status("Fresh", &unit.pkg))?;
                 }
             }
         }

src/cargo/core/compiler/mod.rs

@@ -139,7 +139,7 @@ fn compile<'a, 'cfg: 'a>(
             let work = if cx.bcx.show_warnings(unit.pkg.package_id()) {
                 replay_output_cache(
                     unit.pkg.package_id(),
-                    unit.target,
+                    &unit.target,
                     cx.files().message_cache_path(unit),
                     cx.bcx.build_config.message_format,
                     cx.bcx.config.shell().supports_color(),
@@ -212,7 +212,7 @@ fn rustc<'a, 'cfg>(
     }
     let mut output_options = OutputOptions::new(cx, unit);
     let package_id = unit.pkg.package_id();
-    let target = unit.target.clone();
+    let target = Target::clone(&unit.target);
     let mode = unit.mode;
 
     exec.init(cx, unit);
@@ -408,7 +408,7 @@ fn link_targets<'a, 'cfg>(
     let features = unit.features.iter().map(|s| s.to_string()).collect();
     let json_messages = bcx.build_config.emit_json();
     let executable = cx.get_executable(unit)?;
-    let mut target = unit.target.clone();
+    let mut target = Target::clone(&unit.target);
     if let TargetSourcePath::Metabuild = target.src_path() {
         // Give it something to serialize.
         let path = unit.pkg.manifest().metabuild_path(cx.bcx.ws.target_dir());
@@ -541,11 +541,11 @@ fn prepare_rustc<'a, 'cfg>(
     unit: &Unit<'a>,
 ) -> CargoResult<ProcessBuilder> {
     let is_primary = cx.is_primary_package(unit);
-    let is_workspace = cx.bcx.ws.is_member(unit.pkg);
+    let is_workspace = cx.bcx.ws.is_member(&unit.pkg);
 
     let mut base = cx
         .compilation
-        .rustc_process(unit.pkg, is_primary, is_workspace)?;
+        .rustc_process(&unit.pkg, is_primary, is_workspace)?;
     if cx.bcx.config.cli_unstable().jobserver_per_rustc {
         let client = cx.new_jobserver()?;
         base.inherit_jobserver(&client);
@@ -561,7 +561,7 @@ fn prepare_rustc<'a, 'cfg>(
 
 fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<Work> {
     let bcx = cx.bcx;
-    let mut rustdoc = cx.compilation.rustdoc_process(unit.pkg, unit.target)?;
+    let mut rustdoc = cx.compilation.rustdoc_process(&unit.pkg, &unit.target)?;
     rustdoc.inherit_jobserver(&cx.jobserver);
     rustdoc.arg("--crate-name").arg(&unit.target.crate_name());
     add_path_args(bcx, unit, &mut rustdoc);
@@ -599,7 +599,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
     let name = unit.pkg.name().to_string();
     let build_script_outputs = Arc::clone(&cx.build_script_outputs);
     let package_id = unit.pkg.package_id();
-    let target = unit.target.clone();
+    let target = Target::clone(&unit.target);
     let mut output_options = OutputOptions::new(cx, unit);
     let pkg_id = unit.pkg.package_id();
     let script_metadata = cx.find_build_script_metadata(*unit);
@@ -786,7 +786,7 @@ fn build_base_args<'a, 'cfg>(
     }
 
     let prefer_dynamic = (unit.target.for_host() && !unit.target.is_custom_build())
-        || (crate_types.contains(&"dylib") && bcx.ws.members().any(|p| p != unit.pkg));
+        || (crate_types.contains(&"dylib") && bcx.ws.members().any(|p| p != &*unit.pkg));
     if prefer_dynamic {
         cmd.arg("-C").arg("prefer-dynamic");
     }

src/cargo/core/compiler/timings.rs

@@ -225,7 +225,7 @@ impl<'a, 'cfg> Timings<'a, 'cfg> {
         if self.report_json {
             let msg = machine_message::TimingInfo {
                 package_id: unit_time.unit.pkg.package_id(),
-                target: unit_time.unit.target,
+                target: &unit_time.unit.target,
                 mode: unit_time.unit.mode,
                 duration: unit_time.duration,
                 rmeta_time: unit_time.rmeta_time,

src/cargo/core/compiler/unit.rs

@@ -1,11 +1,13 @@
 use crate::core::compiler::{CompileKind, CompileMode};
-use crate::core::{profiles::Profile, InternedString, Package, Target};
+use crate::core::manifest::{LibKind, Target, TargetKind};
+use crate::core::{profiles::Profile, InternedString, Package};
 use crate::util::hex::short_hash;
 use std::cell::RefCell;
 use std::collections::HashSet;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::ops::Deref;
+use std::rc::Rc;
 
 /// All information needed to define a unit.
 ///
@@ -23,19 +25,19 @@ use std::ops::Deref;
 /// all that out.
 #[derive(Clone, Copy, PartialOrd, Ord)]
 pub struct Unit<'a> {
-    inner: &'a UnitInner<'a>,
+    inner: &'a UnitInner,
 }
 
 /// Internal fields of `Unit` which `Unit` will dereference to.
 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct UnitInner<'a> {
+pub struct UnitInner {
     /// Information about available targets, which files to include/exclude, etc. Basically stuff in
     /// `Cargo.toml`.
-    pub pkg: &'a Package,
+    pub pkg: Rc<Package>,
     /// Information about the specific target to build, out of the possible targets in `pkg`. Not
     /// to be confused with *target-triple* (or *target architecture* ...), the target arch for a
     /// build.
-    pub target: &'a Target,
+    pub target: Rc<Target>,
     /// The profile contains information about *how* the build should be run, including debug
     /// level, etc.
     pub profile: Profile,
@@ -55,7 +57,7 @@ pub struct UnitInner<'a> {
     pub is_std: bool,
 }
 
-impl UnitInner<'_> {
+impl UnitInner {
     /// Returns whether compilation of this unit requires all upstream artifacts
     /// to be available.
     ///
@@ -76,23 +78,23 @@ impl<'a> Unit<'a> {
 // Just hash the pointer for fast hashing
 impl<'a> Hash for Unit<'a> {
     fn hash<H: Hasher>(&self, hasher: &mut H) {
-        (self.inner as *const UnitInner<'a>).hash(hasher)
+        (self.inner as *const UnitInner).hash(hasher)
     }
 }
 
 // Just equate the pointer since these are interned
 impl<'a> PartialEq for Unit<'a> {
     fn eq(&self, other: &Unit<'a>) -> bool {
-        self.inner as *const UnitInner<'a> == other.inner as *const UnitInner<'a>
+        self.inner as *const UnitInner == other.inner as *const UnitInner
     }
 }
 
 impl<'a> Eq for Unit<'a> {}
 
 impl<'a> Deref for Unit<'a> {
-    type Target = UnitInner<'a>;
+    type Target = UnitInner;
 
-    fn deref(&self) -> &UnitInner<'a> {
+    fn deref(&self) -> &UnitInner {
         self.inner
     }
 }
@@ -117,17 +119,17 @@ impl<'a> fmt::Debug for Unit<'a> {
 /// efficient hash/equality implementation for `Unit`. All units are
 /// manufactured through an interner which guarantees that each equivalent value
 /// is only produced once.
-pub struct UnitInterner<'a> {
-    state: RefCell<InternerState<'a>>,
+pub struct UnitInterner {
+    state: RefCell<InternerState>,
 }
 
-struct InternerState<'a> {
-    cache: HashSet<Box<UnitInner<'a>>>,
+struct InternerState {
+    cache: HashSet<Box<UnitInner>>,
 }
 
-impl<'a> UnitInterner<'a> {
+impl UnitInterner {
     /// Creates a new blank interner
-    pub fn new() -> UnitInterner<'a> {
+    pub fn new() -> UnitInterner {
         UnitInterner {
             state: RefCell::new(InternerState {
                 cache: HashSet::new(),
@@ -139,17 +141,38 @@ impl<'a> UnitInterner<'a> {
     /// will all be equivalent to the provided arguments, although they may not
     /// be the exact same instance.
     pub fn intern(
-        &'a self,
-        pkg: &'a Package,
-        target: &'a Target,
+        &self,
+        pkg: &Rc<Package>,
+        target: &Rc<Target>,
         profile: Profile,
         kind: CompileKind,
         mode: CompileMode,
         features: Vec<InternedString>,
         is_std: bool,
-    ) -> Unit<'a> {
+    ) -> Unit<'_> {
+        let target = match (is_std, target.kind()) {
+            // This is a horrible hack to support build-std. `libstd` declares
+            // itself with both rlib and dylib. We don't want the dylib for a
+            // few reasons:
+            //
+            // - dylibs don't have a hash in the filename. If you do something
+            //   (like switch rustc versions), it will stomp on the dylib
+            //   file, invalidating the entire cache (because std is a dep of
+            //   everything).
+            // - We don't want to publicize the presence of dylib for the
+            //   standard library.
+            //
+            // At some point in the future, it would be nice to have a
+            // first-class way of overriding or specifying crate-types.
+            (true, TargetKind::Lib(crate_types)) if crate_types.contains(&LibKind::Dylib) => {
+                let mut new_target = Target::clone(target);
+                new_target.set_kind(TargetKind::Lib(vec![LibKind::Rlib]));
+                Rc::new(new_target)
+            }
+            _ => Rc::clone(target),
+        };
         let inner = self.intern_inner(&UnitInner {
-            pkg,
+            pkg: Rc::clone(pkg),
             target,
             profile,
             kind,
@@ -178,16 +201,16 @@ impl<'a> UnitInterner<'a> {
     // Ideally we'd use an off-the-shelf interner from crates.io which avoids a
     // small amount of unsafety here, but at the time this was written one
     // wasn't obviously available.
-    fn intern_inner(&'a self, item: &UnitInner<'a>) -> &'a UnitInner<'a> {
+    fn intern_inner(&self, item: &UnitInner) -> &UnitInner {
         let mut me = self.state.borrow_mut();
         if let Some(item) = me.cache.get(item) {
-            // note that `item` has type `&Box<UnitInner<'a>`. Use `&**` to
-            // convert that to `&UnitInner<'a>`, then do some trickery to extend
+            // note that `item` has type `&Box<UnitInner>`. Use `&**` to
+            // convert that to `&UnitInner`, then do some trickery to extend
             // the lifetime to the `'a` on the function here.
-            return unsafe { &*(&**item as *const UnitInner<'a>) };
+            return unsafe { &*(&**item as *const UnitInner) };
         }
         me.cache.insert(Box::new(item.clone()));
         let item = me.cache.get(item).unwrap();
-        unsafe { &*(&**item as *const UnitInner<'a>) }
+        unsafe { &*(&**item as *const UnitInner) }
     }
 }