PackageManager: Support directly loading a sub-package via path-based getOrLoadPackage() (#2977)

PackageManager: Support directly loading a sub-package via path-based `getOrLoadPackage()`

Signed-off-by: Mathias LANG <[email protected]>
Merged-on-behalf-of: Martin Kinkelin <[email protected]>

Author: kinke

source/dub/commandline.d

@@ -888,7 +888,7 @@ class Command {
 		dub.mainRecipePath = options.recipeFile;
 		// make the CWD package available so that for example sub packages can reference their
 		// parent package.
-		try dub.packageManager.getOrLoadPackage(NativePath(options.root_path), NativePath(options.recipeFile), false, StrictMode.Warn);
+		try dub.packageManager.getOrLoadPackage(NativePath(options.root_path), NativePath(options.recipeFile), PackageName.init, StrictMode.Warn);
 		catch (Exception e) {
 			// by default we ignore CWD package load fails in prepareDUB, since
 			// they will fail again later when they are actually requested. This

source/dub/dub.d

@@ -516,7 +516,7 @@ class Dub {
 	void loadPackage(NativePath path)
 	{
 		auto pack = this.m_packageManager.getOrLoadPackage(
-			path, NativePath.init, false, StrictMode.Warn);
+			path, NativePath.init, PackageName.init, StrictMode.Warn);
 		this.loadPackage(pack);
 	}
 
@@ -1970,6 +1970,7 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
 		import dub.recipe.json;
 
 		// for sub packages, first try to get them from the base package
+		// FIXME: avoid this, PackageManager.getSubPackage() is costly
 		if (name.main != name) {
 			auto subname = name.sub;
 			auto basepack = getPackage(name.main, dep);
@@ -1985,12 +1986,14 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
 			return m_rootPackage.basePackage;
 
 		if (!dep.repository.empty) {
+			// note: would handle sub-packages directly
 			auto ret = m_dub.packageManager.loadSCMPackage(name, dep.repository);
 			return ret !is null && dep.matches(ret.version_) ? ret : null;
 		}
 		if (!dep.path.empty) {
 			try {
-				return m_dub.packageManager.getOrLoadPackage(dep.path);
+				// note: would handle sub-packages directly
+				return m_dub.packageManager.getOrLoadPackage(dep.path, NativePath.init, name);
 			} catch (Exception e) {
 				logDiagnostic("Failed to load path based dependency %s: %s", name, e.msg);
 				logDebug("Full error: %s", e.toString().sanitize);

source/dub/packagemanager.d

@@ -271,7 +271,7 @@ class PackageManager {
 				foreach (ovr; repo.overrides)
 					if (ovr.package_ == name.toString() && ovr.source.matches(ver)) {
 						Package pack = ovr.target.match!(
-							(NativePath path) => getOrLoadPackage(path),
+							(NativePath path) => getOrLoadPackage(path, NativePath.init, name),
 							(Version	vers) => getPackage(name, vers, false),
 						);
 						if (pack) return pack;
@@ -377,21 +377,42 @@ class PackageManager {
 		Params:
 			path = NativePath to the root directory of the package
 			recipe_path = Optional path to the recipe file of the package
-			allow_sub_packages = Also return a sub package if it resides in the given folder
+			name = Optional (sub-)package name if known in advance. Required if a sub-package is to be returned.
 			mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
 
 		Returns: The packages loaded from the given path
 		Throws: Throws an exception if no package can be loaded
 	*/
 	Package getOrLoadPackage(NativePath path, NativePath recipe_path = NativePath.init,
-		bool allow_sub_packages = false, StrictMode mode = StrictMode.Ignore)
+		PackageName name = PackageName.init, StrictMode mode = StrictMode.Ignore)
 	{
 		path.endsWithSlash = true;
-		foreach (p; this.m_internal.fromPath)
-			if (p.path == path && (!p.parentPackage || (allow_sub_packages && p.parentPackage.path != p.path)))
-				return p;
+		const nameString = name.toString();
+
+		foreach (p; this.m_internal.fromPath) {
+			if (!nameString.empty) {
+				if (p.name == nameString && (p.path == path || p.basePackage.path == path))
+					return p;
+			} else {
+				if (p.path == path && !p.parentPackage)
+					return p;
+			}
+		}
+
 		auto pack = this.load(path, recipe_path, null, null, mode);
-		addPackages(this.m_internal.fromPath, pack);
+		auto nameToResolve = PackageName(pack.name);
+
+		if (!nameString.empty) {
+			nameToResolve = PackageName(nameString);
+			const loadedName = PackageName(pack.name);
+			enforce(loadedName == nameToResolve || loadedName == nameToResolve.main,
+				"Package %s loaded from '%s' does not match expected name %s".format(
+					loadedName, path.toNativeString(), nameToResolve));
+		}
+
+		pack = addPackagesAndResolveSubPackage(this.m_internal.fromPath, pack, nameToResolve);
+		enforce(pack !is null, "No sub-package %s in parent package loaded from '%s'".format(
+			nameToResolve, path.toNativeString()));
 		return pack;
 	}
 

source/dub/project.d

@@ -67,7 +67,7 @@ class Project {
 			logWarn("There was no package description found for the application in '%s'.", project_path.toNativeString());
 			pack = new Package(PackageRecipe.init, project_path);
 		} else {
-			pack = package_manager.getOrLoadPackage(project_path, packageFile, false, StrictMode.Warn);
+			pack = package_manager.getOrLoadPackage(project_path, packageFile, PackageName.init, StrictMode.Warn);
 		}
 
 		this(package_manager, pack);
@@ -553,8 +553,7 @@ class Project {
 				p = vspec.visit!(
 					(NativePath path_) {
 						auto path = path_.absolute ? path_ : m_rootPackage.path ~ path_;
-						auto tmp = m_packageManager.getOrLoadPackage(path, NativePath.init, true);
-						return resolveSubPackage(tmp, subname, true);
+						return m_packageManager.getOrLoadPackage(path, NativePath.init, dep.name);
 					},
 					(Repository repo) {
 						return m_packageManager.loadSCMPackage(dep.name, repo);
@@ -587,15 +586,11 @@ class Project {
 					NativePath path = vspec.path;
 					if (!path.absolute) path = pack.path ~ path;
 					logDiagnostic("%sAdding local %s in %s", indent, dep.name, path);
-					p = m_packageManager.getOrLoadPackage(path, NativePath.init, true);
-					if (p.parentPackage !is null) {
+					p = m_packageManager.getOrLoadPackage(path, NativePath.init, dep.name);
+					path.endsWithSlash = true;
+					if (path != p.basePackage.path) {
 						logWarn("%sSub package %s must be referenced using the path to it's parent package.", indent, dep.name);
-						p = p.parentPackage;
 					}
-					p = resolveSubPackage(p, subname, false);
-					enforce(p.name == dep.name.toString(),
-						format("Path based dependency %s is referenced with a wrong name: %s vs. %s",
-							path.toNativeString(), dep.name, p.name));
 				} else {
 					logDiagnostic("%sMissing dependency %s %s of %s", indent, dep.name, vspec, pack.name);
 					if (is_desired) m_missingDependencies ~= dep.name.toString();

source/dub/test/subpackages.d

@@ -44,14 +44,19 @@ unittest
 {
     scope dub = new TestDub((scope Filesystem root) {
         root.writeFile(TestDub.ProjectPath ~ "dub.json",
-            `{ "name": "a", "dependencies": { "b:a": "~>1.0" } }`);
+            `{ "name": "a", "dependencies": { "b:a": "~>1.0", "c:a": "~>1.0" } }`);
         root.writeFile(TestDub.ProjectPath ~ "dub.selections.json",
-            `{ "fileVersion": 1, "versions": { "b": "1.0.0" } }`);
+            `{ "fileVersion": 1, "versions": { "b": "1.0.0", "c": { "path": "c" } } }`);
         root.writePackageFile("b", "1.0.0",
             `{ "name": "b", "version": "1.0.0", "subPackages": [ { "name": "a" } ] }`);
+        const cDir = TestDub.ProjectPath ~ "c";
+        root.mkdir(cDir);
+        root.writeFile(cDir ~ "dub.json",
+            `{ "name": "c", "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");
+    assert(dub.project.getDependency("c:a", true), "Missing 'c:a' dependency");
 }