Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor BuildContext #8068

Merged
merged 8 commits into from
Apr 19, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Hide Rc<Package> under Package
  • Loading branch information
alexcrichton authored and ehuss committed Apr 19, 2020
commit fbd34a69973509afe6916fe0f2b622ce3612d3ad
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ 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: Rc<Package>,
pub package: Package,
/// The target being tested (currently always the package's lib).
pub target: Rc<Target>,
/// Arguments needed to pass to rustdoc to run this test.
Expand All @@ -28,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<(Rc<Package>, Rc<Target>, PathBuf)>,
pub tests: Vec<(Package, Rc<Target>, PathBuf)>,

/// An array of all binaries created.
pub binaries: Vec<PathBuf>,
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

if unit.mode == CompileMode::Test {
self.compilation.tests.push((
Rc::clone(&unit.pkg),
unit.pkg.clone(),
Rc::clone(&unit.target),
output.path.clone(),
));
Expand Down Expand Up @@ -200,7 +200,7 @@ 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: Rc::clone(&unit.pkg),
package: unit.pkg.clone(),
target: Rc::clone(&unit.target),
args,
unstable_opts,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/compiler/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub struct Unit<'a> {
pub struct UnitInner {
/// Information about available targets, which files to include/exclude, etc. Basically stuff in
/// `Cargo.toml`.
pub pkg: Rc<Package>,
pub pkg: 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.
Expand Down Expand Up @@ -142,7 +142,7 @@ impl UnitInterner {
/// be the exact same instance.
pub fn intern(
&self,
pkg: &Rc<Package>,
pkg: &Package,
target: &Rc<Target>,
profile: Profile,
kind: CompileKind,
Expand Down Expand Up @@ -172,7 +172,7 @@ impl UnitInterner {
_ => Rc::clone(target),
};
let inner = self.intern_inner(&UnitInner {
pkg: Rc::clone(pkg),
pkg: pkg.clone(),
target,
profile,
kind,
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ fn check_or_build_mode(mode: CompileMode, target: &Target) -> CompileMode {
fn new_unit_dep<'unit>(
state: &State<'_, 'unit, '_>,
parent: &Unit<'unit>,
pkg: &Rc<Package>,
pkg: &Package,
target: &Rc<Target>,
unit_for: UnitFor,
kind: CompileKind,
Expand All @@ -597,7 +597,7 @@ fn new_unit_dep<'unit>(
fn new_unit_dep_with_profile<'unit>(
state: &State<'_, 'unit, '_>,
parent: &Unit<'unit>,
pkg: &Rc<Package>,
pkg: &Package,
target: &Rc<Target>,
unit_for: UnitFor,
kind: CompileKind,
Expand Down Expand Up @@ -735,7 +735,7 @@ impl<'a, 'unit, 'cfg> State<'a, 'unit, 'cfg> {
features.activated_features(pkg_id, features_for)
}

fn get(&self, id: PackageId) -> &'a Rc<Package> {
fn get(&self, id: PackageId) -> &'a Package {
self.package_set
.get_one(id)
.unwrap_or_else(|_| panic!("expected {} to be downloaded", id))
Expand Down
69 changes: 39 additions & 30 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ use crate::util::{self, internal, Config, Progress, ProgressStyle};
// TODO: is `manifest_path` a relic?
#[derive(Clone)]
pub struct Package {
inner: Rc<PackageInner>,
}

#[derive(Clone)]
struct PackageInner {
/// The package's manifest.
manifest: Manifest,
/// The root of the package.
Expand Down Expand Up @@ -88,9 +93,9 @@ impl ser::Serialize for Package {
where
S: ser::Serializer,
{
let summary = self.manifest.summary();
let summary = self.manifest().summary();
let package_id = summary.package_id();
let manmeta = self.manifest.metadata();
let manmeta = self.manifest().metadata();
let license = manmeta.license.as_deref();
let license_file = manmeta.license_file.as_deref();
let description = manmeta.description.as_deref();
Expand All @@ -103,7 +108,7 @@ impl ser::Serialize for Package {
// detail that is probably not relevant externally. There's also not a
// real path to show in `src_path`, and this avoids changing the format.
let targets: Vec<&Target> = self
.manifest
.manifest()
.targets()
.iter()
.filter(|t| t.src_path().is_path())
Expand All @@ -121,16 +126,16 @@ impl ser::Serialize for Package {
dependencies: summary.dependencies(),
targets,
features: summary.features(),
manifest_path: &self.manifest_path,
metadata: self.manifest.custom_metadata(),
manifest_path: self.manifest_path(),
metadata: self.manifest().custom_metadata(),
authors,
categories,
keywords,
readme,
repository,
edition: &self.manifest.edition().to_string(),
links: self.manifest.links(),
metabuild: self.manifest.metabuild(),
edition: &self.manifest().edition().to_string(),
links: self.manifest().links(),
metabuild: self.manifest().metabuild(),
publish: self.publish().as_ref(),
}
.serialize(s)
Expand All @@ -141,58 +146,60 @@ impl Package {
/// Creates a package from a manifest and its location.
pub fn new(manifest: Manifest, manifest_path: &Path) -> Package {
Package {
manifest,
manifest_path: manifest_path.to_path_buf(),
inner: Rc::new(PackageInner {
manifest,
manifest_path: manifest_path.to_path_buf(),
}),
}
}

/// Gets the manifest dependencies.
pub fn dependencies(&self) -> &[Dependency] {
self.manifest.dependencies()
self.manifest().dependencies()
}
/// Gets the manifest.
pub fn manifest(&self) -> &Manifest {
&self.manifest
&self.inner.manifest
}
/// Gets the manifest.
pub fn manifest_mut(&mut self) -> &mut Manifest {
&mut self.manifest
&mut Rc::make_mut(&mut self.inner).manifest
}
/// Gets the path to the manifest.
pub fn manifest_path(&self) -> &Path {
&self.manifest_path
&self.inner.manifest_path
}
/// Gets the name of the package.
pub fn name(&self) -> InternedString {
self.package_id().name()
}
/// Gets the `PackageId` object for the package (fully defines a package).
pub fn package_id(&self) -> PackageId {
self.manifest.package_id()
self.manifest().package_id()
}
/// Gets the root folder of the package.
pub fn root(&self) -> &Path {
self.manifest_path.parent().unwrap()
self.manifest_path().parent().unwrap()
}
/// Gets the summary for the package.
pub fn summary(&self) -> &Summary {
self.manifest.summary()
self.manifest().summary()
}
/// Gets the targets specified in the manifest.
pub fn targets(&self) -> &[Rc<Target>] {
self.manifest.targets()
self.manifest().targets()
}
/// Gets the current package version.
pub fn version(&self) -> &Version {
self.package_id().version()
}
/// Gets the package authors.
pub fn authors(&self) -> &Vec<String> {
&self.manifest.metadata().authors
&self.manifest().metadata().authors
}
/// Returns `true` if the package is set to publish.
pub fn publish(&self) -> &Option<Vec<String>> {
self.manifest.publish()
self.manifest().publish()
}
/// Returns `true` if this package is a proc-macro.
pub fn proc_macro(&self) -> bool {
Expand All @@ -206,8 +213,10 @@ impl Package {

pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Package {
Package {
manifest: self.manifest.map_source(to_replace, replace_with),
manifest_path: self.manifest_path,
inner: Rc::new(PackageInner {
manifest: self.manifest().clone().map_source(to_replace, replace_with),
manifest_path: self.manifest_path().to_owned(),
}),
}
}

Expand Down Expand Up @@ -276,7 +285,7 @@ impl hash::Hash for Package {
/// This is primarily used to convert a set of `PackageId`s to `Package`s. It
/// will download as needed, or used the cached download if available.
pub struct PackageSet<'cfg> {
packages: HashMap<PackageId, LazyCell<Rc<Package>>>,
packages: HashMap<PackageId, LazyCell<Package>>,
sources: RefCell<SourceMap<'cfg>>,
config: &'cfg Config,
multi: Multi,
Expand Down Expand Up @@ -440,7 +449,7 @@ impl<'cfg> PackageSet<'cfg> {
})
}

pub fn get_one(&self, id: PackageId) -> CargoResult<&Rc<Package>> {
pub fn get_one(&self, id: PackageId) -> CargoResult<&Package> {
if let Some(pkg) = self.packages.get(&id).and_then(|slot| slot.borrow()) {
return Ok(pkg);
}
Expand All @@ -450,7 +459,7 @@ impl<'cfg> PackageSet<'cfg> {
pub fn get_many(
&self,
ids: impl IntoIterator<Item = PackageId>,
) -> CargoResult<Vec<&Rc<Package>>> {
) -> CargoResult<Vec<&Package>> {
let mut pkgs = Vec::new();
let mut downloads = self.enable_download()?;
for id in ids {
Expand Down Expand Up @@ -576,13 +585,13 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
/// Returns `None` if the package is queued up for download and will
/// eventually be returned from `wait_for_download`. Returns `Some(pkg)` if
/// the package is ready and doesn't need to be downloaded.
pub fn start(&mut self, id: PackageId) -> CargoResult<Option<&'a Rc<Package>>> {
pub fn start(&mut self, id: PackageId) -> CargoResult<Option<&'a Package>> {
Ok(self
.start_inner(id)
.chain_err(|| format!("failed to download `{}`", id))?)
}

fn start_inner(&mut self, id: PackageId) -> CargoResult<Option<&'a Rc<Package>>> {
fn start_inner(&mut self, id: PackageId) -> CargoResult<Option<&'a Package>> {
// First up see if we've already cached this package, in which case
// there's nothing to do.
let slot = self
Expand All @@ -607,7 +616,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
let (url, descriptor) = match pkg {
MaybePackage::Ready(pkg) => {
debug!("{} doesn't need a download", id);
assert!(slot.fill(Rc::new(pkg)).is_ok());
assert!(slot.fill(pkg).is_ok());
return Ok(Some(slot.borrow().unwrap()));
}
MaybePackage::Download { url, descriptor } => (url, descriptor),
Expand Down Expand Up @@ -720,7 +729,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
/// # Panics
///
/// This function will panic if there are no remaining downloads.
pub fn wait(&mut self) -> CargoResult<&'a Rc<Package>> {
pub fn wait(&mut self) -> CargoResult<&'a Package> {
let (dl, data) = loop {
assert_eq!(self.pending.len(), self.pending_ids.len());
let (token, result) = self.wait_for_curl()?;
Expand Down Expand Up @@ -833,7 +842,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
.set(self.next_speed_check.get() + finish_dur);

let slot = &self.set.packages[&dl.id];
assert!(slot.fill(Rc::new(pkg)).is_ok());
assert!(slot.fill(pkg).is_ok());
Ok(slot.borrow().unwrap())
}

Expand Down
12 changes: 6 additions & 6 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ impl CompileFilter {
/// not the target requires its features to be present.
#[derive(Debug)]
struct Proposal<'a> {
pkg: &'a Rc<Package>,
pkg: &'a Package,
target: &'a Rc<Target>,
/// Indicates whether or not all required features *must* be present. If
/// false, and the features are not available, then it will be silently
Expand All @@ -693,7 +693,7 @@ struct Proposal<'a> {
/// compile. Dependencies for these targets are computed later in `unit_dependencies`.
fn generate_targets<'unit>(
ws: &Workspace<'_>,
packages: &[&Rc<Package>],
packages: &[&Package],
filter: &CompileFilter,
default_arch_kind: CompileKind,
mode: CompileMode,
Expand All @@ -705,7 +705,7 @@ fn generate_targets<'unit>(
) -> CargoResult<Vec<Unit<'unit>>> {
let config = ws.config();
// Helper for creating a `Unit` struct.
let new_unit = |pkg: &Rc<Package>, target: &Rc<Target>, target_mode: CompileMode| {
let new_unit = |pkg: &Package, target: &Rc<Target>, target_mode: CompileMode| {
let unit_for = if target_mode.is_any_test() {
// NOTE: the `UnitFor` here is subtle. If you have a profile
// with `panic` set, the `panic` flag is cleared for
Expand Down Expand Up @@ -1009,7 +1009,7 @@ fn filter_default_targets(targets: &[Rc<Target>], mode: CompileMode) -> Vec<&Rc<

/// Returns a list of proposed targets based on command-line target selection flags.
fn list_rule_targets<'a>(
packages: &[&'a Rc<Package>],
packages: &[&'a Package],
rule: &FilterRule,
target_desc: &'static str,
is_expected_kind: fn(&Target) -> bool,
Expand Down Expand Up @@ -1037,7 +1037,7 @@ fn list_rule_targets<'a>(

/// Finds the targets for a specifically named target.
fn find_named_targets<'a>(
packages: &[&'a Rc<Package>],
packages: &[&'a Package],
target_name: &str,
target_desc: &'static str,
is_expected_kind: fn(&Target) -> bool,
Expand All @@ -1063,7 +1063,7 @@ fn find_named_targets<'a>(
}

fn filter_targets<'a>(
packages: &[&'a Rc<Package>],
packages: &[&'a Package],
predicate: impl Fn(&Target) -> bool,
requires_features: bool,
mode: CompileMode,
Expand Down