From 030f2a275a56118046981068a3e2f6096dae361e Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Thu, 31 Oct 2024 15:16:47 +0100 Subject: [PATCH 1/4] Add test case for issue #2973 --- source/dub/test/dependencies.d | 2 +- source/dub/test/subpackages.d | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/source/dub/test/dependencies.d b/source/dub/test/dependencies.d index 4b31f7dc3..39c13faf9 100644 --- a/source/dub/test/dependencies.d +++ b/source/dub/test/dependencies.d @@ -98,7 +98,7 @@ version "1.0.0"`, PackageFormat.sdl); assert(dub.project.hasAllDependencies(), "project has missing dependencies"); assert(dub.project.getDependency("b", true), "Missing 'b' dependency"); assert(dub.project.getDependency("c", true), "Missing 'c' dependency"); - assert(dub.project.getDependency("c", true), "Missing 'd' dependency"); + assert(dub.project.getDependency("d", true), "Missing 'd' dependency"); assert(dub.project.getDependency("no", true) is null, "Returned unexpected dependency"); } diff --git a/source/dub/test/subpackages.d b/source/dub/test/subpackages.d index a56ac0c3e..f1e1104a2 100644 --- a/source/dub/test/subpackages.d +++ b/source/dub/test/subpackages.d @@ -38,3 +38,20 @@ unittest assert(!dub.packageManager().getPackage(PackageName("b:b"), Version("1.1.0"))); } + +// https://github.com/dlang/dub/issues/2973 +unittest +{ + scope dub = new TestDub((scope Filesystem root) { + root.writeFile(TestDub.ProjectPath ~ "dub.json", + `{ "name": "a", "dependencies": { "b:a": "~>1.0" } }`); + root.writeFile(TestDub.ProjectPath ~ "dub.selections.json", + `{ "fileVersion": 1, "versions": { "b": "1.0.0" } }`); + root.writePackageFile("b", "1.0.0", + `{ "name": "b", "version": "1.0.0", "subPackages": [ { "name": "a" } ] }`); + }); + dub.loadPackage(); + + assert(dub.project.hasAllDependencies(), "project has missing dependencies"); + assert(dub.project.getDependency("b:a", true), "Missing 'b:a' dependency"); +} From 6d54e207bde8d740192c5c4887660b51dc119e0e Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 29 Oct 2024 17:09:38 +0100 Subject: [PATCH 2/4] Fix #2973 --- source/dub/project.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/dub/project.d b/source/dub/project.d index 575e8eba3..83e055d83 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -563,7 +563,8 @@ class Project { (VersionRange range) { // See `dub.recipe.selection : SelectedDependency.fromYAML` assert(range.isExactVersion()); - return m_packageManager.getPackage(dep.name, vspec.version_); + auto tmp = m_packageManager.getPackage(basename, vspec.version_); + return resolveSubPackage(tmp, subname, true); }, ); } else if (m_dependencies.canFind!(d => PackageName(d.name).main == basename)) { From cef47ae13c4bd97587d77305c088cba3ff900ffd Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 29 Oct 2024 17:34:07 +0100 Subject: [PATCH 3/4] Tentatively fix similar issue for SCM deps --- source/dub/project.d | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/dub/project.d b/source/dub/project.d index 83e055d83..065c9b8ed 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -582,10 +582,10 @@ class Project { { if (!vspec.repository.empty) { p = m_packageManager.loadSCMPackage(basename, vspec.repository); - resolveSubPackage(p, subname, false); enforce(p !is null, - "Unable to fetch '%s@%s' using git - does the repository and version exists?".format( - dep.name, vspec.repository)); + "Unable to fetch '%s@%s' using git - does the repository and version exist?".format( + basename, vspec.repository)); + p = resolveSubPackage(p, subname, false); } else if (!vspec.path.empty && is_desired) { NativePath path = vspec.path; if (!path.absolute) path = pack.path ~ path; From 41002a4a47dabfeeb045d3cdba2a8871fd340d8b Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Fri, 1 Nov 2024 04:09:48 +0100 Subject: [PATCH 4/4] [take 2 - fix PackageManager instead] --- source/dub/packagemanager.d | 37 ++++++++++++++++++++++++++++++------- source/dub/project.d | 11 ++++------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index 79029cc4b..6fa7f90d5 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -523,10 +523,11 @@ class PackageManager { } else if (!this.gitClone(repo.remote, gitReference, destination)) return null; - Package result = this.load(destination); - if (result !is null) - this.addPackages(this.m_internal.fromPath, result); - return result; + Package p = this.load(destination); + if (p is null) + return null; + + return this.addPackagesAndResolveSubPackage(this.m_internal.fromPath, p, name); } /** @@ -1323,6 +1324,29 @@ symlink_exit: } } } + + /// Adds the package, scans for sub-packages, and returns the added package matching + /// the specified name (of the package itself or a sub-package). + /// Returns null if the sub-package doesn't exist. + private Package addPackagesAndResolveSubPackage(ref Package[] dst_repos, Package pack, + PackageName nameToResolve) + { + enforce(pack.name == nameToResolve.main.toString(), + "nameToResolve must be the added package or one of its sub-packages"); + + this.addPackages(dst_repos, pack); + + if (nameToResolve.sub.length == 0) + return pack; + + // available sub-packages have been appended + foreach_reverse (sp; dst_repos) { + if (sp.parentPackage is pack && sp.name == nameToResolve.toString()) + return sp; + } + + return null; + } } deprecated(OverrideDepMsg) @@ -1742,9 +1766,8 @@ package struct Location { enforce( p.version_ == vers, format("Package %s located in %s has a different version than its path: Got %s, expected %s", - name, path, p.version_, vers)); - mgr.addPackages(this.fromPath, p); - return p; + name.main, path, p.version_, vers)); + return mgr.addPackagesAndResolveSubPackage(this.fromPath, p, name); } /** diff --git a/source/dub/project.d b/source/dub/project.d index 065c9b8ed..f2496378a 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -557,14 +557,12 @@ class Project { return resolveSubPackage(tmp, subname, true); }, (Repository repo) { - auto tmp = m_packageManager.loadSCMPackage(basename, repo); - return resolveSubPackage(tmp, subname, true); + return m_packageManager.loadSCMPackage(dep.name, repo); }, (VersionRange range) { // See `dub.recipe.selection : SelectedDependency.fromYAML` assert(range.isExactVersion()); - auto tmp = m_packageManager.getPackage(basename, vspec.version_); - return resolveSubPackage(tmp, subname, true); + return m_packageManager.getPackage(dep.name, vspec.version_); }, ); } else if (m_dependencies.canFind!(d => PackageName(d.name).main == basename)) { @@ -581,11 +579,10 @@ class Project { if (p is null) { if (!vspec.repository.empty) { - p = m_packageManager.loadSCMPackage(basename, vspec.repository); + p = m_packageManager.loadSCMPackage(dep.name, vspec.repository); enforce(p !is null, "Unable to fetch '%s@%s' using git - does the repository and version exist?".format( - basename, vspec.repository)); - p = resolveSubPackage(p, subname, false); + dep.name, vspec.repository)); } else if (!vspec.path.empty && is_desired) { NativePath path = vspec.path; if (!path.absolute) path = pack.path ~ path;