Skip to content

Improve error messages for provide conflicts#4608

Open
Phantomical wants to merge 7 commits into
KSP-CKAN:masterfrom
Phantomical:better-provider-conflict-errors
Open

Improve error messages for provide conflicts#4608
Phantomical wants to merge 7 commits into
KSP-CKAN:masterfrom
Phantomical:better-provider-conflict-errors

Conversation

@Phantomical
Copy link
Copy Markdown

@Phantomical Phantomical commented May 6, 2026

Currently, if you try to install a mod that provides X, but there is some other mod already installed that also provides X, then you get an error message that looks like

Unsatisfied dependency Y needed for: ...

This tells the users nothing about what is wrong or how to fix it. This commit is an attempt to improve the error message here. The new error message is now

X is needed for Y, but cannot be installed because it conflicts with Z which also provides W.

This commit changes:

  • Instead of just getting a dependency chain, DependenciesNotSatisfiedKraken now takes a list of UnsatisfiedRelation. These also optionally store a reason that the relation could not be satisfied, which can then be used for better error messages.
  • ResolvedRelationshipsTree.Candidates now explicitly checks for provides conflicts and records unresolved relations accordingly.
  • In cases where there are no candidates and no unresolved relations then we redo the lookup without filtering in order to find modules that could have satisfied the relation, under different conditions.
  • DependenciesNotSatisfiedKraken now takes the ProviderRejection into account when coming up with the error message.

I have also added some test cases that validate that the correct error message is emitted under a number of different conditions.

I'm not sure that this necessarily the right approach. It feels like I am pulling stuff into ResolvedRelationshipTree that isn't really supposed to be there. On the other hand the "if error then re-resolve to get unsatisfied candidates" bit does actually feel right to me, so I'm not sure what the correct balance is here.


The easiest one to test is probably this:

  • Add the sol repo
  • Install parallax continued
  • Apply
  • Install Sol-Configs

@HebaruSan HebaruSan added Enhancement New features or functionality Core (ckan.dll) Issues affecting the core part of CKAN Relationships Issues affecting depends, recommends, etc. labels May 6, 2026
@coveralls
Copy link
Copy Markdown

coveralls commented May 6, 2026

Coverage Report for CI Build 25575170102

Coverage decreased (-0.09%) to 87.502%

Details

  • Coverage decreased (-0.09%) from the base build.
  • Patch coverage: 15 uncovered changes across 3 files (112 of 127 lines covered, 88.19%).
  • 9 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
Core/Relationships/ResolvedRelationshipsTree.cs 44 36 81.82%
Core/Relationships/ResolvedRelationship.cs 64 59 92.19%
Core/Types/Kraken.cs 19 17 89.47%

Coverage Regressions

9 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
Core/Games/KerbalSpaceProgram/GameVersionProviders/KspBuildMap.cs 9 57.14%

Coverage Stats

Coverage Status
Relevant Lines: 9974
Covered Lines: 8592
Line Coverage: 86.14%
Relevant Branches: 2140
Covered Branches: 2008
Branch Coverage: 93.83%
Branches in Coverage %: Yes
Coverage Strength: 1.8 hits per line

💛 - Coveralls

@Phantomical
Copy link
Copy Markdown
Author

Phantomical commented May 6, 2026

Okay, this should be good to go now. Changes:

  • ResolvedByNew now stores a ResolutionContext that is meant to be a representation of the context it was resolved in. That is what gets used later on to do extra resolves in the error case.
  • Fixed all the issues where a provides relationship was considered to conflict implicitly. That should now only happen if stuff is explicitly marked as conflicting.
  • Updated the test cases to match.

In practice none of the stuff stored in ResolutionContext is modified. The only case where that might change in the future is the Installed list, so if we want to be safe I can update it to make a defensive copy here.

@HebaruSan HebaruSan force-pushed the better-provider-conflict-errors branch 2 times, most recently from 8d2964f to b8c3371 Compare May 8, 2026 16:51
@Phantomical
Copy link
Copy Markdown
Author

I think this covers all the possible error cases now:

  • Conflicts via a provides relationship: A is needed for B, but cannot be installed because it conflicts with C which also provides D
  • General conflict: A is needed for B, but cannot be installed because it conflicts with C
  • Version conflict: Version conflict: A 1.0 is needed for B, and A 2.0 is needed for C, but both cannot be installed at the same time

It still special cases provides conflicts a little bit because I think having the context that "this error is happening through a provides relationship" is useful, but other than that the pathways are pretty much the same.

@HebaruSan HebaruSan force-pushed the better-provider-conflict-errors branch from d65890f to b56b511 Compare May 11, 2026 18:32
Currently, if you try to install a mod that provides X, but there is
some other mod already installed that also provides X, then you get an
error message that looks like

    Unsatisfied dependency Y needed for: ...

This tells the users nothing about what is wrong or how to fix it. This
commit is an attempt to improve the error message here.

This commit changes:
* Instead of just getting a dependency chain,
  DependenciesNotSatisfiedKraken now takes a list of
  UnsatisfiedRelation. These also optionally store a reason that the
  relation could not be satisfied, which can then be used for better
  error messages.
* ResolvedRelationshipsTree.Candidates now explicitly checks for
  provides conflicts and records unresolved relations accordingly.
* In cases where there are no candidates and no unresolved relations
  then we redo the lookup without filtering in order to find modules
  that could have satisfied the relation, under different conditions.
* DependenciesNotSatisfiedKraken now takes the ProviderRejection into
  account when coming up with the error message.

I have also added some test cases that validate that the correct error
message is emitted.
We don't really want to store these fields where in the relationship
tree, by instead capturing them in the relationship itself we can keep
the tree clean, while still having the context we need later on.
@HebaruSan HebaruSan force-pushed the better-provider-conflict-errors branch from b56b511 to 1ba4ce9 Compare May 11, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality Relationships Issues affecting depends, recommends, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants