Skip to content

Commit

Permalink
Fix cyclic crate dependency #1
Browse files Browse the repository at this point in the history
Move the macros in spk-solve to its own crate, to break the cycle of

   spk-solve -> spk-solve-validation -> spk-solve (dev)

The macro code was changed to remove many uses of re-exported symbols
through `$crate`, since adding these dependencies would reintroduce a
cyclic dependency.

Signed-off-by: J Robert Ray <jrray@jrray.org>
  • Loading branch information
jrray committed Jul 19, 2023
1 parent 9ca9d6e commit f1ae853
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 49 deletions.
16 changes: 14 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/spk-cli/group1/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ tracing = { workspace = true }

[dev-dependencies]
rstest = { workspace = true }
spk-solve-macros = { path = '../../spk-solve/crates/macros' }
spk-solve-solution = { path = '../../spk-solve/crates/solution' }
2 changes: 1 addition & 1 deletion crates/spk-cli/group1/src/cmd_deprecate_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use rstest::rstest;
use spk_schema::ident::parse_version_ident;
use spk_schema::Deprecate;
use spk_solve::make_repo;
use spk_solve_macros::make_repo;

use super::{change_deprecation_state, ChangeAction};

Expand Down
2 changes: 1 addition & 1 deletion crates/spk-cli/group1/src/cmd_undeprecate_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use rstest::rstest;
use spk_schema::ident::parse_version_ident;
use spk_schema::Deprecate;
use spk_solve::make_repo;
use spk_solve_macros::make_repo;

use super::{change_deprecation_state, ChangeAction};

Expand Down
1 change: 1 addition & 0 deletions crates/spk-solve/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ tracing = { workspace = true }

[dev-dependencies]
rstest = { workspace = true }
spk-solve-macros = { path = "./crates/macros" }
strip-ansi-escapes = { workspace = true }
13 changes: 13 additions & 0 deletions crates/spk-solve/crates/macros/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
authors = { workspace = true }
edition = { workspace = true }
name = "spk-solve-macros"
version = { workspace = true }

[features]
migration-to-components = []

[dependencies]
serde_json = { workspace = true }
spfs = { path = "../../../spfs" }
spk-schema = { path = "../../../spk-schema" }
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/imageworks/spk

#![deny(unsafe_op_in_unsafe_fn)]
#![warn(clippy::fn_params_excessive_bools)]

pub use spk_schema::recipe;
pub use {serde_json, spfs};

/// Creates a repository containing a set of provided package specs.
/// It will take care of publishing the spec, and creating a build for
/// each provided package so that it can be resolved.
Expand All @@ -14,16 +20,16 @@ macro_rules! make_repo {
make_repo!([ $( $spec ),* ], options={})
}};
( [ $( $spec:tt ),+ $(,)? ], options={ $($k:expr => $v:expr),* } ) => {{
let options = $crate::option_map!{$($k => $v),*};
let options = spk_schema::foundation::option_map!{$($k => $v),*};
make_repo!([ $( $spec ),* ], options=options)
}};
( [ $( $spec:tt ),+ $(,)? ], options=$options:expr ) => {{
tracing::debug!("creating in-memory repository");
let repo = $crate::RepositoryHandle::new_mem();
let repo = spk_storage::RepositoryHandle::new_mem();
let _opts = $options;
$(
let (s, cmpts) = $crate::make_package!(repo, $spec, &_opts);
tracing::trace!(pkg=%$crate::Package::ident(&s), cmpts=?cmpts.keys(), "adding package to repo");
tracing::trace!(pkg=%spk_schema::Package::ident(&s), cmpts=?cmpts.keys(), "adding package to repo");
repo.publish_package(&s, &cmpts).await.unwrap();
)*
repo
Expand All @@ -36,7 +42,7 @@ macro_rules! make_package {
($build_spec, $components)
}};
($repo:ident, $build_spec:ident, $opts:expr) => {{
use $crate::Package;
use spk_schema::Package;
let s = $build_spec.clone();
let cmpts: std::collections::HashMap<_, $crate::spfs::encoding::Digest> = s
.components()
Expand All @@ -47,18 +53,21 @@ macro_rules! make_package {
}};
($repo:ident, $spec:tt, $opts:expr) => {{
let json = $crate::serde_json::json!($spec);
let spec: $crate::v0::Spec<$crate::AnyIdent> =
let spec: spk_schema::v0::Spec<spk_schema::ident::AnyIdent> =
$crate::serde_json::from_value(json).expect("Invalid spec json");
match spec.pkg.build().map(|b| b.clone()) {
None => {
let recipe = spec.clone().map_ident(|i| i.into_base()).into();
$repo.force_publish_recipe(&recipe).await.unwrap();
make_build_and_components!(recipe, [], $opts, [])
}
Some($crate::Build::Source) => {
Some(spk_schema::foundation::ident_build::Build::Source) => {
let recipe = spec.clone().map_ident(|i| i.into_base());
$repo.force_publish_recipe(&recipe.into()).await.unwrap();
let build = spec.map_ident(|i| i.into_base().into_build($crate::Build::Source));
let build = spec.map_ident(|i| {
i.into_base()
.into_build(spk_schema::foundation::ident_build::Build::Source)
});
make_build_and_components!(build, [], $opts, [])
}
Some(b) => {
Expand Down Expand Up @@ -108,31 +117,31 @@ macro_rules! make_build_and_components {
make_build_and_components!($spec, [$($dep),*], $opts, [])
};
($spec:tt, [$($dep:expr),*], { $($k:expr => $v:expr),* }, [$($component:expr),*]) => {{
let opts = $crate::option_map!{$($k => $v),*};
let opts = spk_schema::foundation::option_map!{$($k => $v),*};
make_build_and_components!($spec, [$($dep),*], opts, [$($component),*])
}};
($spec:tt, [$($dep:expr),*], $opts:expr, [$($component:expr),*]) => {{
#[allow(unused_imports)]
use $crate::{Package, Recipe};
use spk_schema::{Package, Recipe};
let json = $crate::serde_json::json!($spec);
let spec: $crate::v0::Spec<$crate::AnyIdent> =
let spec: spk_schema::v0::Spec<spk_schema::ident::AnyIdent> =
$crate::serde_json::from_value(json).expect("Invalid spec json");
let mut components = std::collections::HashMap::<$crate::Component, $crate::spfs::encoding::Digest>::new();
let mut components = std::collections::HashMap::<spk_schema::foundation::ident_component::Component, $crate::spfs::encoding::Digest>::new();
match spec.pkg.build().map(|b| b.clone()) {
None => {
let recipe = spec.clone().map_ident(|i| i.into_base());
let mut build_opts = $opts.clone();
#[allow(unused_mut)]
let mut solution = $crate::Solution::new(build_opts.clone());
let mut solution = spk_solve_solution::Solution::new(build_opts.clone());
$(
let dep = Arc::new($dep.clone());
solution.add(
$crate::PkgRequest::from_ident(
spk_schema::ident::PkgRequest::from_ident(
$dep.ident().to_any(),
$crate::RequestedBy::SpkInternalTest,
spk_schema::ident::RequestedBy::SpkInternalTest,
),
Arc::clone(&dep),
$crate::PackageSource::SpkInternalTest,
spk_solve_solution::PackageSource::SpkInternalTest,
);
)*
let mut resolved_opts = recipe.resolve_options(&build_opts).unwrap().into_iter();
Expand All @@ -145,15 +154,15 @@ macro_rules! make_build_and_components {
names = build.components().iter().map(|c| c.name.to_string()).collect();
}
for name in names {
let name = $crate::Component::parse(name).expect("invalid component name");
let name = spk_schema::foundation::ident_component::Component::parse(name).expect("invalid component name");
components.insert(name, $crate::spfs::encoding::EMPTY_DIGEST.into());
}
($crate::Spec::V0Package(build), components)
(spk_schema::Spec::V0Package(build), components)
}
Some(b @ $crate::Build::Source) => {
Some(b @ spk_schema::foundation::ident_build::Build::Source) => {
let build = spec.map_ident(|i| i.into_base().into_build(b));
components.insert($crate::Component::Source, $crate::spfs::encoding::EMPTY_DIGEST.into());
($crate::Spec::V0Package(build), components)
components.insert(spk_schema::foundation::ident_component::Component::Source, $crate::spfs::encoding::EMPTY_DIGEST.into());
(spk_schema::Spec::V0Package(build), components)
}
Some(b) => {
let build = spec.map_ident(|i| i.into_base().into_build(b));
Expand All @@ -162,10 +171,10 @@ macro_rules! make_build_and_components {
names = build.components().iter().map(|c| c.name.to_string()).collect();
}
for name in names {
let name = $crate::Component::parse(name).expect("invalid component name");
let name = spk_schema::foundation::ident_component::Component::parse(name).expect("invalid component name");
components.insert(name, $crate::spfs::encoding::EMPTY_DIGEST.into());
}
($crate::Spec::V0Package(build), components)
(spk_schema::Spec::V0Package(build), components)
}
}
}}
Expand All @@ -187,20 +196,20 @@ macro_rules! make_spec {
#[macro_export(local_inner_macros)]
macro_rules! request {
($req:literal) => {
$crate::Request::Pkg($crate::PkgRequest::new(
$crate::parse_ident_range($req).unwrap(),
RequestedBy::SpkInternalTest,
spk_schema::ident::Request::Pkg(spk_schema::ident::PkgRequest::new(
spk_schema::ident::parse_ident_range($req).unwrap(),
spk_schema::ident::RequestedBy::SpkInternalTest,
))
};
($req:ident) => {
$crate::Request::Pkg($crate::PkgRequest::new(
$crate::parse_ident_range($req).unwrap(),
RequestedBy::SpkInternalTest,
spk_schema::ident::Request::Pkg(spk_schema::ident::PkgRequest::new(
spk_schema::ident::parse_ident_range($req).unwrap(),
spk_schema::ident::RequestedBy::SpkInternalTest,
))
};
($req:tt) => {{
let value = serde_json::json!($req);
let req: $crate::Request = serde_json::from_value(value).unwrap();
let req: spk_schema::ident::Request = serde_json::from_value(value).unwrap();
req
}};
}
2 changes: 1 addition & 1 deletion crates/spk-solve/crates/package-iterator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ tracing = { workspace = true }
[dev-dependencies]
itertools = "0.10"
rstest = { workspace = true }
spk-solve = { path = "../.." }
spk-solve-macros = { path = "../macros" }
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use spk_schema::foundation::option_map;
use spk_schema::foundation::option_map::OptionMap;
use spk_schema::foundation::version::Compatibility;
use spk_schema::{recipe, spec, BuildIdent, Package, Spec};
use spk_solve::{make_build, make_repo};
use spk_solve_macros::{make_build, make_repo};

use super::{BuildIterator, PackageIterator, RepositoryPackageIterator, SortedBuildIterator};

Expand Down
2 changes: 1 addition & 1 deletion crates/spk-solve/crates/validation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ tracing = "0.1.35"
[dev-dependencies]
rstest = { workspace = true }
serde_yaml = "0.8.17"
spk-solve = { path = "../.." }
spk-solve-macros = { path = "../macros" }
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use spk_schema::foundation::fixtures::*;
use spk_schema::ident::{version_ident, PkgRequest, RequestedBy};
use spk_schema::name::PkgNameBuf;
use spk_schema::spec;
use spk_solve::{make_build, make_repo};
use spk_solve_macros::{make_build, make_repo};

use super::ImpossibleRequestsChecker;

Expand Down
2 changes: 1 addition & 1 deletion crates/spk-solve/crates/validation/src/validation_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use spk_schema::foundation::fixtures::*;
use spk_schema::foundation::opt_name;
use spk_schema::ident::{build_ident, version_ident, PkgRequest, Request, RequestedBy};
use spk_schema::{spec, FromYaml};
use spk_solve::recipe;
use spk_solve_graph::State;
use spk_solve_macros::recipe;
use spk_solve_solution::PackageSource;

use super::{default_validators, OptionsValidator, ValidatorT, VarRequirementsValidator};
Expand Down
1 change: 0 additions & 1 deletion crates/spk-solve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

mod error;
mod io;
mod macros;
#[cfg(feature = "statsd")]
mod metrics;
mod search_space;
Expand Down
12 changes: 2 additions & 10 deletions crates/spk-solve/src/solver_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,13 @@ use spk_schema::ident::{
use spk_schema::ident_build::EmbeddedSource;
use spk_schema::prelude::*;
use spk_schema::{recipe, v0};
use spk_solve_macros::{make_build, make_build_and_components, make_repo, request};
use spk_solve_solution::PackageSource;
use spk_storage::RepositoryHandle;

use super::{ErrorDetails, Solver};
use crate::io::DecisionFormatterBuilder;
use crate::{
make_build,
make_build_and_components,
make_repo,
option_map,
request,
spec,
Error,
Result,
};
use crate::{option_map, spec, Error, Result};

#[fixture]
fn solver() -> Solver {
Expand Down

0 comments on commit f1ae853

Please sign in to comment.