Skip to content

Commit

Permalink
Rework planning to use resolve tree and not packages (updated) (#425)
Browse files Browse the repository at this point in the history
There are a range of subtle and irritating bugs that stem from
attempting to work with renames from the package set provided by cargo
metadata.

Fortunately, recent versions of cargo metadata provide a limited version
of the resolve nodes, both as the original node ids and as a newer set
providing both rename and targeting information.

As such we can (hopefully) simplify crate -> dep node resolution, remove
a bunch of subtle aliasing bugs.

Fixes #241, fixes #269, fixes #270, resolves #144, resolves #187
  • Loading branch information
sayrer committed Aug 9, 2021
1 parent ee2effe commit 78617c1
Show file tree
Hide file tree
Showing 12 changed files with 463 additions and 582 deletions.
13 changes: 8 additions & 5 deletions impl/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::path::PathBuf;
use std::{
collections::BTreeMap,
path::PathBuf,
};

use crate::settings::CrateSettings;
use semver::Version;
Expand All @@ -29,7 +32,7 @@ pub struct BuildableDependency {
pub is_proc_macro: bool,
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)]
#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord, Serialize)]
pub struct DependencyAlias {
pub target: String,
pub alias: String,
Expand Down Expand Up @@ -81,7 +84,7 @@ pub struct SourceDetails {
pub git_data: Option<GitRepo>,
}

#[derive(Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Default, Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct CrateDependencyContext {
pub dependencies: Vec<BuildableDependency>,
pub proc_macro_dependencies: Vec<BuildableDependency>,
Expand All @@ -92,7 +95,7 @@ pub struct CrateDependencyContext {
// build_data_dependencies can only be set when using cargo-raze as a library at the moment.
pub build_data_dependencies: Vec<BuildableDependency>,
pub dev_dependencies: Vec<BuildableDependency>,
pub aliased_dependencies: Vec<DependencyAlias>,
pub aliased_dependencies: BTreeMap<String, DependencyAlias>,
}

impl CrateDependencyContext {
Expand All @@ -110,7 +113,7 @@ impl CrateDependencyContext {
pub struct CrateTargetedDepContext {
pub target: String,
pub deps: CrateDependencyContext,
pub conditions: Vec<String>,
pub platform_targets: Vec<String>,
}

#[derive(Debug, Clone, Serialize)]
Expand Down
2 changes: 1 addition & 1 deletion impl/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ impl RazeMetadataFetcher {
let version = info.req();
let src_dir = self.fetch_crate_src(cargo_dir.as_ref(), &name, version)?;
checksums.insert(
package_ident(name, version),
package_ident(name, &version),
self.fetch_crate_checksum(name, version)?,
);
if let Some(dirname) = src_dir.file_name() {
Expand Down
34 changes: 20 additions & 14 deletions impl/src/planning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use anyhow::Result;
use cargo_lock::Lockfile;

use crate::{
context::{CrateContext, WorkspaceContext},
context::{CrateContext, DependencyAlias, WorkspaceContext},
metadata::RazeMetadata,
settings::RazeSettings,
util::PlatformDetails,
Expand All @@ -32,8 +32,13 @@ use subplanners::WorkspaceSubplanner;
/// A ready-to-be-rendered build, containing renderable context for each crate.
#[derive(Debug)]
pub struct PlannedBuild {
/// The overall context for this workspace
pub workspace_context: WorkspaceContext,
/// The creates to build for
pub crate_contexts: Vec<CrateContext>,
/// And aliases that are defined at the workspace root
pub workspace_aliases: Vec<DependencyAlias>,
/// The version lock used if present
pub lockfile: Option<Lockfile>,
}

Expand Down Expand Up @@ -89,6 +94,7 @@ mod tests {
use super::*;
use cargo_metadata::PackageId;
use indoc::indoc;
use itertools::Itertools;
use semver::{Version, VersionReq};

fn dummy_resolve_dropping_metadata() -> RazeMetadata {
Expand Down Expand Up @@ -245,22 +251,23 @@ mod tests {
assert!(krate_position.is_some());

// Get crate context using computed position
let krate_context = crates_with_aliased_deps[krate_position.unwrap()].clone();
let crate_ctx = crates_with_aliased_deps[krate_position.unwrap()].clone();

// There are two default dependencies for cargo-raze-alias-test, log^0.4 and log^0.3
// However, log^0.3 is aliased to old_log_ while log^0.4 isn't aliased. Therefore, we
// should only see one aliased dependency (log^0.3 -> old_log_) which shows that the
// However, log^0.3 is aliased to old_log while log^0.4 isn't aliased. Therefore, we
// should only see one aliased dependency (log^0.3 -> old_log) which shows that the
// name and semver matching for aliased dependencies is working correctly
assert!(krate_context.default_deps.aliased_dependencies.len() == 1);
assert_eq!(
krate_context.default_deps.aliased_dependencies[0].target,
"@raze_test__log__0_3_9//:log"
);
assert_eq!(
krate_context.default_deps.aliased_dependencies[0].alias,
"old_log_"
);
let (name, dep) = crate_ctx
.default_deps
.aliased_dependencies
.iter()
.exactly_one()
.unwrap();
assert_eq!(name, &dep.target);
assert_eq!(dep.target, "@raze_test__log__0_3_9//:log");
assert_eq!(dep.alias, "old_log");
}

#[test]
fn test_plan_build_produces_proc_macro_dependencies() {
let mut settings = dummy_raze_settings();
Expand Down Expand Up @@ -423,7 +430,6 @@ mod tests {
let context = planned_build
.crate_contexts
.iter()
.inspect(|x| println!("{}{}", x.pkg_name, x.pkg_version))
.find(|ctx| ctx.pkg_name == "some-binary-crate" && ctx.pkg_version == version)
.unwrap();

Expand Down
Loading

0 comments on commit 78617c1

Please sign in to comment.