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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion source/dub/commandline.d
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,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
Expand Down
7 changes: 5 additions & 2 deletions source/dub/dub.d
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,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);
}

Expand Down Expand Up @@ -1933,6 +1933,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);
Expand All @@ -1948,12 +1949,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);
Expand Down
35 changes: 28 additions & 7 deletions source/dub/packagemanager.d
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ class PackageManager {
foreach (ovr; repo.overrides)
if (ovr.package_ == name.toString() && ovr.source.matches(ver)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the overrides should only apply to base packages as a whole, not on a sub-package basis. I.e., ovr.package_ == name.main.toString().

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;
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The breaking API change is in a separate 2nd commit, in case we cannot/don't want to break it.

{
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;
}

Expand Down
15 changes: 5 additions & 10 deletions source/dub/project.d
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 7 additions & 2 deletions source/dub/test/subpackages.d
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Loading