diff --git a/source/dub/dependencyresolver.d b/source/dub/dependencyresolver.d index f5e9dcb..4d7e1c6 100644 --- a/source/dub/dependencyresolver.d +++ b/source/dub/dependencyresolver.d @@ -11,7 +11,7 @@ import dub.internal.vibecompat.core.log; import std.algorithm : all, canFind, filter, map, sort; -import std.array : appender, array; +import std.array : appender, array, join; import std.conv : to; import std.exception : enforce; import std.typecons : Nullable; @@ -54,6 +54,19 @@ } } + static struct ConfigEntry + { + static struct Depender + { + TreeNode origin; + TreeNodes dependency; + } + + CONFIG[] all_configs; + bool any_config; + Depender[] origins; + } + CONFIG[string] resolve(TreeNode root, bool throw_on_failure = true) { auto root_base_pack = basePackage(root.pack); @@ -61,8 +74,7 @@ // find all possible configurations of each possible dependency size_t[string] package_indices; string[size_t] package_names; - CONFIG[][] all_configs; - bool[] any_config; + ConfigEntry[] configs; bool[string] maybe_optional_deps; bool[TreeNode] visited; @@ -73,38 +85,40 @@ foreach (ch; getChildren(parent)) { auto basepack = basePackage(ch.pack); - auto pidx = all_configs.length; + auto pidx = configs.length; if (ch.depType != DependencyType.required) maybe_optional_deps[ch.pack] = true; - CONFIG[] configs; + ConfigEntry config; if (auto pi = basepack in package_indices) { pidx = *pi; - configs = all_configs[*pi]; + config = configs[*pi]; } else { - if (basepack == root_base_pack) configs = [root.config]; - else configs = getAllConfigs(basepack); - all_configs ~= configs; + if (basepack == root_base_pack) config.all_configs = [root.config]; + else config.all_configs = getAllConfigs(basepack); + configs ~= config; package_indices[basepack] = pidx; package_names[pidx] = basepack; } foreach (c; getSpecificConfigs(basepack, ch)) - if (!configs.canFind(c)) - configs = c ~ configs; + if (!config.all_configs.canFind(c)) + config.all_configs = c ~ config.all_configs; - if (any_config.length <= pidx) any_config.length = pidx+1; - if (configs.length > 0) - any_config[pidx] = true; + if (config.all_configs.length > 0) + config.any_config = true; + + // store package depending on this for better error messages + config.origins ~= ConfigEntry.Depender(parent, ch); // eliminate configurations from which we know that they can't satisfy // the uniquely defined root dependencies (==version or ~branch style dependencies) - if (parent_unique) configs = configs.filter!(c => matches(ch.configs, c)).array; + if (parent_unique) config.all_configs = config.all_configs.filter!(c => matches(ch.configs, c)).array; - all_configs[pidx] = configs; + configs[pidx] = config; - foreach (v; configs) - findConfigsRec(TreeNode(ch.pack, v), parent_unique && configs.length == 1); + foreach (v; config.all_configs) + findConfigsRec(TreeNode(ch.pack, v), parent_unique && config.all_configs.length == 1); } } findConfigsRec(root, true); @@ -113,14 +127,14 @@ // this is used to properly support optional dependencies (when // getChildren() returns no configurations for an optional dependency, // but getAllConfigs() has already provided an existing list of configs) - foreach (i, ref cfgs; all_configs) - if (cfgs.length == 0 || package_names[i] in maybe_optional_deps) - cfgs = cfgs ~ CONFIG.invalid; + foreach (i, ref cfgs; configs) + if (cfgs.all_configs.length == 0 || package_names[i] in maybe_optional_deps) + cfgs.all_configs = cfgs.all_configs ~ CONFIG.invalid; logDebug("Configurations used for dependency resolution:"); - foreach (n, i; package_indices) logDebug(" %s (%s%s): %s", n, i, n in maybe_optional_deps ? ", maybe optional" : ", required", all_configs[i]); + foreach (n, i; package_indices) logDebug(" %s (%s%s): %s", n, i, n in maybe_optional_deps ? ", maybe optional" : ", required", configs[i]); - auto config_indices = new size_t[all_configs.length]; + auto config_indices = new size_t[configs.length]; config_indices[] = 0; visited = null; @@ -142,7 +156,8 @@ // get the current config/version of the current dependency sizediff_t childidx = package_indices[basepack]; - if (all_configs[childidx].length == 1 && all_configs[childidx][0] == CONFIG.invalid) { + auto child = configs[childidx]; + if (child.all_configs == [CONFIG.invalid]) { // ignore invalid optional dependencies if (ch.depType != DependencyType.required) continue; @@ -154,8 +169,10 @@ logError("Dependency \"%s\" of %s contains upper case letters, but must be lower case.", ch.pack, parent.pack); if (getAllConfigs(lp).length) logError("Did you mean \"%s\"?", lp); } - if (any_config[childidx]) - throw new Exception(format("Root package %s reference %s %s cannot be satisfied.", parent.pack, ch.pack, ch.configs)); + if (child.any_config) + throw new Exception(format("Root package %s reference %s %s cannot be satisfied.\nPackages causing the conflict:\n\t%s", + parent.pack, ch.pack, ch.configs, + child.origins.map!(a => a.origin.pack ~ " depends on " ~ a.dependency.configs.to!string).join("\n\t"))); else throw new Exception(format("Root package %s references unknown package %s", parent.pack, ch.pack)); } @@ -166,7 +183,7 @@ maxcpi = parentidx; } } else { - auto config = all_configs[childidx][config_indices[childidx]]; + auto config = child.all_configs[config_indices[childidx]]; auto chnode = TreeNode(ch.pack, config); if (config == CONFIG.invalid || !matches(ch.configs, config)) { @@ -221,10 +238,10 @@ // print out current iteration state logDebug("Interation (ci=%s) %s", conflict_index, { import std.array : join; - auto cs = new string[all_configs.length]; + auto cs = new string[configs.length]; foreach (p, i; package_indices) { - if (all_configs[i].length) - cs[i] = p~" "~all_configs[i][config_indices[i]].to!string~(i >= 0 && i >= conflict_index ? " (C)" : ""); + if (configs[i].all_configs.length) + cs[i] = p~" "~configs[i].all_configs[config_indices[i]].to!string~(i >= 0 && i >= conflict_index ? " (C)" : ""); else cs[i] = p ~ " [no config]"; } return cs.join(", "); @@ -233,8 +250,8 @@ if (conflict_index < 0) { CONFIG[string] ret; foreach (p, i; package_indices) - if (all_configs[i].length) { - auto cfg = all_configs[i][config_indices[i]]; + if (configs[i].all_configs.length) { + auto cfg = configs[i].all_configs[config_indices[i]]; if (cfg != CONFIG.invalid) ret[p] = cfg; } logDebug("Resolved dependencies before optional-purge: %s", ret.byKey.map!(k => k~" "~ret[k].to!string)); @@ -246,7 +263,7 @@ // find the next combination of configurations foreach_reverse (pi, ref i; config_indices) { if (pi > conflict_index) i = 0; - else if (++i >= all_configs[pi].length) i = 0; + else if (++i >= configs[pi].all_configs.length) i = 0; else break; } if (config_indices.all!"a==0") { diff --git a/test/expected-issue1037-output b/test/expected-issue1037-output new file mode 100644 index 0000000..a5d7c55 --- /dev/null +++ b/test/expected-issue1037-output @@ -0,0 +1,4 @@ +Root package issue1037-better-dependency-messages reference gitcompatibledubpackage 1.0.1 cannot be satisfied. +Packages causing the conflict: + issue1037-better-dependency-messages depends on 1.0.1 + b depends on ~>1.0.2 diff --git a/test/issue1037-better-dependency-messages.sh b/test/issue1037-better-dependency-messages.sh new file mode 100755 index 0000000..7893241 --- /dev/null +++ b/test/issue1037-better-dependency-messages.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -e -o pipefail + +cd ${CURR_DIR}/issue1037-better-dependency-messages + +temp_file=$(mktemp $(basename $0).XXXXXX) +expected_file="$CURR_DIR/expected-issue1037-output" + +function cleanup { + rm $temp_file +} + +trap cleanup EXIT + +$DUB upgrade 2>$temp_file && exit 1 # dub upgrade should fail + +if ! diff "$expected_file" "$temp_file"; then + die 'output not containing conflict information' +fi + +exit 0 \ No newline at end of file diff --git a/test/issue1037-better-dependency-messages/.no_build b/test/issue1037-better-dependency-messages/.no_build new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/test/issue1037-better-dependency-messages/.no_build diff --git a/test/issue1037-better-dependency-messages/.no_run b/test/issue1037-better-dependency-messages/.no_run new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/test/issue1037-better-dependency-messages/.no_run diff --git a/test/issue1037-better-dependency-messages/.no_test b/test/issue1037-better-dependency-messages/.no_test new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/test/issue1037-better-dependency-messages/.no_test diff --git a/test/issue1037-better-dependency-messages/b/dub.json b/test/issue1037-better-dependency-messages/b/dub.json new file mode 100644 index 0000000..92443ac --- /dev/null +++ b/test/issue1037-better-dependency-messages/b/dub.json @@ -0,0 +1,6 @@ +{ + "name": "b", + "dependencies": { + "gitcompatibledubpackage": "~>1.0.2" + } +} \ No newline at end of file diff --git a/test/issue1037-better-dependency-messages/dub.json b/test/issue1037-better-dependency-messages/dub.json new file mode 100644 index 0000000..abb7d17 --- /dev/null +++ b/test/issue1037-better-dependency-messages/dub.json @@ -0,0 +1,10 @@ +{ + "name": "issue1037-better-dependency-messages", + "dependencies": { + "gitcompatibledubpackage": "1.0.1", + "b": { + "path": "b", + "version": "*" + } + } +} \ No newline at end of file