Fix path-based dependencies in sub-packages from path-based dependencies #2588
Conversation
| @@ -864,7 +864,7 @@ class Dub { | |||
| if (options & FetchOptions.printOnly) { | |||
| if (existing && existing.version_ != ver) | |||
| logInfo("A new version for %s is available (%s -> %s). Run \"%s\" to switch.", | |||
| packageId.color(Mode.bold), existing.version_, ver, | |||
| packageId.color(Mode.bold), existing, ver, | |||
There was a problem hiding this comment.
version_ crashes with non-version-range dependency specifications, so for logging we just dump the Dependency directly
WebFreak001
force-pushed
the
fix-regression-2587
branch
from
a6c2326 to
5172c02
Compare
WebFreak001
changed the title
A major issue when dealing with the 'Dependency' type is that it evolved from a simple type, which stored a version range and perhaps one or two boolean, to one that stores 4 kinds of dependencies: version range (including exact version), branches, repository + hash, and path. Instead of using a discrete type (e.g. a tagged union), the type is a mix of style, and repository and branch are piggybacking on the field used for the version range. Switching to std.sumtype makes interaction between fields more obvious. Most importantly, it forces the programmer to answer the hard questions, such as how do we compare completely different dependency (version vs repository). Hopefully this will make the code easier to understand, and in the future, make it easier to add a new kind of dependency.
|
Does windows always fail? |
|
yes (it's a long path bug, that we should really try to make shorter to avoid this kind of thing) fix is here: #2589 |
|
Is anything going to be done about it? I'd hate to promote normalizing the behaviour of ignoring test results. |
|
I'll try starting by first just using a smaller cache string (e.g. replace the long sha string with a truncated base64 one) in a separate PR |
|
Merging for now, so can get on with tagging beta for point release. If there's still any issues, there's a few days to fix them. |
| @@ -1631,7 +1631,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend | |||
| protected override Dependency[] getSpecificConfigs(string pack, TreeNodes nodes) | |||
| { | |||
| if (!nodes.configs.path.empty || !nodes.configs.repository.empty) { | |||
| if (getPackage(pack, nodes.configs)) return [nodes.configs]; | |||
| if (getPackage(nodes.pack, nodes.configs)) return [nodes.configs]; | |||
There was a problem hiding this comment.
Duh, glad to see it was a dumb typo in the end.
There was a problem hiding this comment.
yeah, no idea how the hashOf removal broke this though, can only guess that it was missing checks further inside, because path based dependencies used to always equal to true whenever comparing with any other path based dependencies.
Fix #2587