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 path-based dependencies in sub-packages from path-based dependencies #2588

Merged
merged 3 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/path-dep-fix.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Path dependencies of path-based sub-packages have been fixed

Path-based dependencies in path-based sub-packages in DUB v1.30.0 and 1.31.0
have had regressed and didn't resolve properly. (dub.selections.json could be
used to workaround this issue before)

From this release, these resolve properly again.
5 changes: 4 additions & 1 deletion source/dub/dependency.d
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ struct Dependency {
return this.m_value.match!(
(NativePath v) {
if (v.empty || v.absolute) return this;
return Dependency(path ~ v);
auto ret = Dependency(path ~ v);
ret.m_default = m_default;
ret.m_optional = m_optional;
return ret;
},
(Repository v) => this,
(VersionRange v) => this,
Expand Down
14 changes: 7 additions & 7 deletions source/dub/dub.d
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

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

version_ crashes with non-version-range dependency specifications, so for logging we just dump the Dependency directly

text("dub upgrade ", packageId).color(Mode.bold));
return null;
}
Expand Down Expand Up @@ -983,7 +983,7 @@ class Dub {
try {
remove(pack);
} catch (Exception e) {
logError("Failed to remove %s %s: %s", package_id, pack.version_, e.msg);
logError("Failed to remove %s %s: %s", package_id, pack, e.msg);
logInfo("Continuing with other packages (if any).");
}
}
Expand Down Expand Up @@ -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];
Copy link
Member

Choose a reason for hiding this comment

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

Duh, glad to see it was a dumb typo in the end.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

else return null;
}
else return null;
Expand Down Expand Up @@ -1676,7 +1676,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend

if (!d.spec.path.empty && absdeppath != desireddeppath)
logWarn("Sub package %s, referenced by %s %s must be referenced using the path to its base package",
subpack.name, pack.name, pack.version_);
subpack.name, pack.name, pack);

enforce(d.spec.path.empty || absdeppath == desireddeppath || absdeppath == altdeppath,
format("Dependency from %s to %s uses wrong path: %s vs. %s",
Expand Down Expand Up @@ -1746,11 +1746,11 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
m_remotePackages[sp.name] = sp;
return sp;
} else {
logDiagnostic("Sub package %s doesn't exist in %s %s.", name, basename, dep.version_);
logDiagnostic("Sub package %s doesn't exist in %s %s.", name, basename, dep);
return null;
}
} else {
logDiagnostic("External sub package %s %s not found.", name, dep.version_);
logDiagnostic("External sub package %s %s not found.", name, dep);
return null;
}
}
Expand Down Expand Up @@ -1805,7 +1805,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
m_dub.fetch(rootpack, vers, m_dub.defaultPlacementLocation, fetchOpts, "need sub package description");
auto ret = m_dub.m_packageManager.getBestPackage(name, vers);
if (!ret) {
logWarn("Package %s %s doesn't have a sub package %s", rootpack, dep.version_, name);
logWarn("Package %s %s doesn't have a sub package %s", rootpack, dep, name);
return null;
}
m_remotePackages[key] = ret;
Expand Down
6 changes: 4 additions & 2 deletions source/dub/recipe/packagerecipe.d
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ string[] getSubPackagePath(string package_name) @safe pure
}

/**
Returns the name of the top level package for a given (sub) package name.
Returns the name of the top level package for a given (sub) package name of
format `"basePackageName"` or `"basePackageName:subPackageName"`.

In case of a top level package, the qualified name is returned unmodified.
*/
Expand All @@ -48,7 +49,8 @@ string getBasePackageName(string package_name) @safe pure
}

/**
Returns the qualified sub package part of the given package name.
Returns the qualified sub package part of the given package name of format
`"basePackageName:subPackageName"`, or empty string if none.

This is the part of the package name excluding the base package
name. See also $(D getBasePackageName).
Expand Down
13 changes: 13 additions & 0 deletions test/issue2587-subpackage-dependency-resolution.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash

set -e

. $(dirname "${BASH_SOURCE[0]}")/common.sh
cd "${CURR_DIR}/issue2587-subpackage-dependency-resolution/a"

rm -f dub.selections.json
$DUB upgrade -v
$DUB run

rm -f dub.selections.json
$DUB run
Empty file.
Empty file.
Empty file.
16 changes: 16 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/a/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.dub
docs.json
__dummy.html
docs/
/a
a.so
a.dylib
a.dll
a.a
a.lib
a-test-*
*.exe
*.pdb
*.o
*.obj
*.lst
6 changes: 6 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/a/dub.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "a",
"dependencies": {
"b": {"path":"../b"}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import b, c;

void main()
{
doB();
doC();
}
16 changes: 16 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/b/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.dub
docs.json
__dummy.html
docs/
/b
b.so
b.dylib
b.dll
b.a
b.lib
b-test-*
*.exe
*.pdb
*.o
*.obj
*.lst
14 changes: 14 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/b/dub.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "b",
"dependencies": {
"b:sub":"*"
},
"subPackages": [
{
"name": "sub",
"dependencies": {
"c": {"path":"../c"}
}
}
]
}
5 changes: 5 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/b/source/b.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module b;

void doB()
{
}
16 changes: 16 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/c/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.dub
docs.json
__dummy.html
docs/
/c
c.so
c.dylib
c.dll
c.a
c.lib
c-test-*
*.exe
*.pdb
*.o
*.obj
*.lst
3 changes: 3 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/c/dub.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "c"
}
5 changes: 5 additions & 0 deletions test/issue2587-subpackage-dependency-resolution/c/source/c.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module c;

void doC()
{
}