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

Ignore dependency self conflicts #2747

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

HebaruSan
Copy link
Member

Problem

  • On a completely fresh install, AstronomersVisualPack shows as compatible
  • If you install Scatterer, AstronomersVisualPack (which depends on Scatterer) shows as incompatible

That doesn't make sense. Installing a dependency shouldn't ever mark the depending module as incompatible.

Cause

Scatterer's self-conflict relationship trips this check:

if (module.conflicts != null)
{
foreach (RelationshipDescriptor rel in module.conflicts)
{
// If any of the conflicts are present, fail
if (rel.MatchesAny(others, null, null))
{
return false;
}
}
}

... but only when it's installed, via this block:

if (installed != null)
{
modules = modules.Where(m => DependsAndConflictsOK(m, installed));
}

... passed from here:

.Select(am => am.Latest(
ksp_version,
relationship_descriptor,
InstalledModules.Select(im => im.Module),
toInstall
))

... while checking dependencies for compatibility here:

if (!dependency.MatchesAny(null, InstalledDlls.ToHashSet(), InstalledDlc)
&& !dependency.LatestAvailableWithProvides(this, ksp_version).Any())
{
return false;
}

Changes

Now when checking that conflicts are OK for an available module, we filter any copies of itself (i.e., with matching identifier) out of the list of potential conflicting modules. This prevents the Scatterer self conflict from breaking modules that depend on Scatterer.

We could try ignoring the self conflict relationships instead, but that would miss other modules that provide the conflicting identifier, which presumably would be part of why such conflicts would be set up in the first place.

Fixes #2719 (the part of it that can't be explained as expected behavior).

@HebaruSan HebaruSan added Bug Core (ckan.dll) Issues affecting the core part of CKAN Pull request Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc. labels Apr 28, 2019
@Olympic1 Olympic1 removed the Bug label Apr 28, 2019
@HebaruSan HebaruSan added the Bug label Apr 28, 2019
@HebaruSan HebaruSan merged commit aba78b7 into KSP-CKAN:master Apr 29, 2019
HebaruSan added a commit that referenced this pull request Apr 29, 2019
@HebaruSan HebaruSan deleted the fix/dep-self-conf branch April 29, 2019 16:26
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 Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mod compatibility state changed after updating CKAN
3 participants