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

Fix a couple circular dependencies between crates #803

Merged
merged 2 commits into from
Jul 29, 2023
Merged

Conversation

jrray
Copy link
Collaborator

@jrray jrray commented Jul 19, 2023

Fix "cyclic deps" as can be seen being complained about in the rust-analyzer server log:

[ERROR][2023-07-18 19:26:40] .../vim/lsp/rpc.lua:734    "rpc"   "/home/jrray/.cargo/bin/rust-analyzer"  "stderr"        "[ERROR project_model::workspace] cyclic deps: spk_schema_validators(CrateId(344)) -> spk_schema(CrateId(340)), alternative path: spk_schema(CrateId(340)) -> spk_schema_validators(CrateId(344))\n"

@jrray jrray added the maintenance Cleanup, upgrades, etc label Jul 19, 2023
@jrray jrray self-assigned this Jul 19, 2023
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>
Relocate a tests file to avoid a cyclic dependency:

    spk_schema -> spk_schema_validators -> spk_schema (dev)

Signed-off-by: J Robert Ray <jrray@jrray.org>
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #803 (db541a3) into main (9ca9d6e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #803      +/-   ##
==========================================
- Coverage   53.97%   53.96%   -0.01%     
==========================================
  Files         248      248              
  Lines       19303    19311       +8     
==========================================
+ Hits        10419    10422       +3     
- Misses       8884     8889       +5     
Impacted Files Coverage Δ
...tes/spk-schema/crates/validators/src/validators.rs 77.50% <ø> (ø)
crates/spk-solve/src/lib.rs 83.33% <ø> (ø)
crates/spk-solve/crates/macros/src/lib.rs 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jrray jrray requested a review from rydrman July 19, 2023 03:31
Comment on lines 13 to 16

#[cfg(test)]
#[path = "./validators_test.rs"]
mod validators_test;
// Tests for this module are in spk-schema/src/v0/validators_test.rs to avoid
// a cyclic crate dependency (the tests need spk_schema::v0).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change that I'm stuck on here - but I guess with the crates all blown apart into small pieces there's not much that can be done here aside from defining a separate spec and package type for use in the validator tests specifically. I suppose that's the right way to do it but it would be a lot of work, probably...

@jrray jrray merged commit 7fe5456 into main Jul 29, 2023
5 of 6 checks passed
@jrray jrray deleted the circular-deps branch July 29, 2023 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Cleanup, upgrades, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants