Clean up build script stuff and documentation.

Author: ehuss

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

@@ -16,6 +16,11 @@ mod target_info;
 pub use self::target_info::{FileFlavor, TargetInfo};
 
 /// The build context, containing all information about a build task.
+///
+/// It is intended that this is mostly static information. Stuff that mutates
+/// during the build can be found in the parent `Context`. (I say mostly,
+/// because this has internal caching, but nothing that should be observable
+/// or require &mut.)
 pub struct BuildContext<'a, 'cfg> {
     /// The workspace the build is for.
     pub ws: &'a Workspace<'cfg>,
@@ -183,6 +188,17 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
     pub fn extra_args_for(&self, unit: &Unit<'a>) -> Option<&Vec<String>> {
         self.extra_compiler_args.get(unit)
     }
+
+    /// If a build script is overridden, this returns the `BuildOutput` to use.
+    ///
+    /// `lib_name` is the `links` library name and `kind` is whether it is for
+    /// Host or Target.
+    pub fn script_override(&self, lib_name: &str, kind: Kind) -> Option<&BuildOutput> {
+        match kind {
+            Kind::Host => self.host_config.overrides.get(lib_name),
+            Kind::Target => self.target_config.overrides.get(lib_name),
+        }
+    }
 }
 
 /// Information required to build for a target.
@@ -192,7 +208,11 @@ pub struct TargetConfig {
     pub ar: Option<PathBuf>,
     /// The path of the linker for this target.
     pub linker: Option<PathBuf>,
-    /// Special build options for any necessary input files (filename -> options).
+    /// Build script override for the given library name.
+    ///
+    /// Any package with a `links` value for the given library name will skip
+    /// running its build script and instead use the given output from the
+    /// config file.
     pub overrides: HashMap<String, BuildOutput>,
 }
 

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

@@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet};
 use std::ffi::OsStr;
 use std::fmt::Write;
 use std::path::PathBuf;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 
 use filetime::FileTime;
 use jobserver::Client;
@@ -15,7 +15,7 @@ use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::{internal, profile, Config};
 
 use super::build_plan::BuildPlan;
-use super::custom_build::{self, BuildDeps, BuildScripts, BuildState};
+use super::custom_build::{self, BuildDeps, BuildScriptOutputs, BuildScripts};
 use super::fingerprint::Fingerprint;
 use super::job_queue::JobQueue;
 use super::layout::Layout;
@@ -28,21 +28,45 @@ mod compilation_files;
 use self::compilation_files::CompilationFiles;
 pub use self::compilation_files::{Metadata, OutputFile};
 
+/// Collection of all the stuff that is needed to perform a build.
 pub struct Context<'a, 'cfg> {
+    /// Mostly static information about the build task.
     pub bcx: &'a BuildContext<'a, 'cfg>,
+    /// A large collection of information about the result of the entire compilation.
     pub compilation: Compilation<'cfg>,
-    pub build_state: Arc<BuildState>,
-    pub build_script_overridden: HashSet<(PackageId, Kind)>,
+    /// Output from build scripts, updated after each build script runs.
+    pub build_script_outputs: Arc<Mutex<BuildScriptOutputs>>,
+    /// Dependencies (like rerun-if-changed) declared by a build script.
+    /// This is *only* populated from the output from previous runs.
+    /// If the build script hasn't ever been run, then it must be run.
     pub build_explicit_deps: HashMap<Unit<'a>, BuildDeps>,
+    /// Fingerprints used to detect if a unit is out-of-date.
     pub fingerprints: HashMap<Unit<'a>, Arc<Fingerprint>>,
+    /// Cache of file mtimes to reduce filesystem hits.
     pub mtime_cache: HashMap<PathBuf, FileTime>,
+    /// A set used to track which units have been compiled.
+    /// A unit may appear in the job graph multiple times as a dependency of
+    /// multiple packages, but it only needs to run once.
     pub compiled: HashSet<Unit<'a>>,
+    /// Linking information for each `Unit`.
+    /// See `build_map` for details.
     pub build_scripts: HashMap<Unit<'a>, Arc<BuildScripts>>,
+    /// Used to check the `links` field in the manifest is not duplicated and
+    /// is used correctly.
     pub links: Links,
+    /// Job server client to manage concurrency with other processes.
     pub jobserver: Client,
+    /// "Primary" packages are the ones the user selected on the command-line
+    /// with `-p` flags. If no flags are specified, then it is the defaults
+    /// based on the current directory and the default workspace members.
     primary_packages: HashSet<PackageId>,
+    /// The dependency graph of units to compile.
     unit_dependencies: HashMap<Unit<'a>, Vec<Unit<'a>>>,
+    /// An abstraction of the files and directories that will be generated by
+    /// the compilation. This is `None` until after `unit_dependencies` has
+    /// been computed.
     files: Option<CompilationFiles<'a, 'cfg>>,
+    /// Cache of packages, populated when they are downloaded.
     package_cache: HashMap<PackageId, &'a Package>,
 
     /// A flag indicating whether pipelining is enabled for this compilation
@@ -82,16 +106,14 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         Ok(Self {
             bcx,
             compilation: Compilation::new(bcx)?,
-            build_state: Arc::new(BuildState::new(&bcx.host_config, &bcx.target_config)),
+            build_script_outputs: Arc::new(Mutex::new(BuildScriptOutputs::default())),
             fingerprints: HashMap::new(),
             mtime_cache: HashMap::new(),
             compiled: HashSet::new(),
             build_scripts: HashMap::new(),
             build_explicit_deps: HashMap::new(),
             links: Links::new(),
             jobserver,
-            build_script_overridden: HashSet::new(),
-
             primary_packages: HashSet::new(),
             unit_dependencies: HashMap::new(),
             files: None,
@@ -228,7 +250,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             super::output_depinfo(&mut self, unit)?;
         }
 
-        for (&(ref pkg, _), output) in self.build_state.outputs.lock().unwrap().iter() {
+        for (&(ref pkg, _), output) in self.build_script_outputs.lock().unwrap().iter() {
             self.compilation
                 .cfgs
                 .entry(pkg.clone())
@@ -338,22 +360,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     //
     // TODO: this ideally should be `-> &[Unit<'a>]`.
     pub fn dep_targets(&self, unit: &Unit<'a>) -> Vec<Unit<'a>> {
-        // If this build script's execution has been overridden then we don't
-        // actually depend on anything, we've reached the end of the dependency
-        // chain as we've got all the info we're gonna get.
-        //
-        // Note there's a subtlety about this piece of code! The
-        // `build_script_overridden` map here is populated in
-        // `custom_build::build_map` which you need to call before inspecting
-        // dependencies. However, that code itself calls this method and
-        // gets a full pre-filtered set of dependencies. This is not super
-        // obvious, and clear, but it does work at the moment.
-        if unit.target.is_custom_build() {
-            let key = (unit.pkg.package_id(), unit.kind);
-            if self.build_script_overridden.contains(&key) {
-                return Vec::new();
-            }
-        }
         self.unit_dependencies[unit].clone()
     }
 

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

@@ -258,6 +258,13 @@ fn compute_deps_custom_build<'a, 'cfg>(
     unit: &Unit<'a>,
     bcx: &BuildContext<'a, 'cfg>,
 ) -> CargoResult<Vec<(Unit<'a>, UnitFor)>> {
+    if let Some(links) = unit.pkg.manifest().links() {
+        if bcx.script_override(links, unit.kind).is_some() {
+            // Overridden build scripts don't have any dependencies.
+            return Ok(Vec::new());
+        }
+    }
+
     // When not overridden, then the dependencies to run a build script are:
     //
     // 1. Compiling the build script itself.