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.

Sign up for GitHub

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

Deprecate Package.load, use PackageManager.load instead #2769

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Deprecate Package.load, use PackageManager.load instead #2769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion source/dub/package_.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ static immutable string[] builtinBuildTypes = [
/** Represents a package, including its sub packages.
*/
class Package {
// `package` visibility as it is set from the PackageManager
package NativePath m_infoFile;
private {
NativePath m_path;
NativePath m_infoFile;
PackageRecipe m_info;
PackageRecipe m_rawRecipe;
Package m_parentPackage;
Expand Down Expand Up @@ -173,6 +174,7 @@ class Package {
determined by invoking the VCS (GIT currently).
mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
*/
deprecated("Use `PackageManager.getOrLoadPackage` instead of loading packages directly")
static Package load(NativePath root, NativePath recipe_file = NativePath.init,
Package parent = null, string version_override = "",
StrictMode mode = StrictMode.Ignore)
Expand Down
59 changes: 51 additions & 8 deletions source/dub/packagemanager.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,54 @@ class PackageManager {
foreach (p; this.m_internal.fromPath)
if (p.path == path && (!p.parentPackage || (allow_sub_packages && p.parentPackage.path != p.path)))
return p;
auto pack = Package.load(path, recipe_path, null, null, mode);
auto pack = this.load(path, recipe_path, null, null, mode);
addPackages(this.m_internal.fromPath, pack);
return pack;
}

/**
* Loads a `Package` from the filesystem
*
* This is called when a `Package` needs to be loaded from the path.
* This does not change the internal state of the `PackageManager`,
* it simply loads the `Package` and returns it - it is up to the caller
* to call `addPackages`.
*
* Throws:
* If no package can be found at the `path` / with the `recipe`.
*
* Params:
* path = The directory in which the package resides.
* recipe = Optional path to the package recipe file. If left empty,
* the `path` directory will be searched for a recipe file.
* parent = Reference to the parent package, if the new package is a
* sub package.
* version_ = Optional version to associate to the package instead of
* the one declared in the package recipe, or the one
* determined by invoking the VCS (GIT currently).
* mode = Whether to issue errors, warning, or ignore unknown keys in
* dub.json
*
* Returns: A populated `Package`.
*/
protected Package load(NativePath path, NativePath recipe = NativePath.init,
Package parent = null, string version_ = null,
StrictMode mode = StrictMode.Ignore) const
{
if (recipe.empty)
recipe = Package.findPackageFile(path);

enforce(!recipe.empty,
"No package file found in %s, expected one of %s"
.format(path.toNativeString(),
packageInfoFiles.map!(f => cast(string)f.filename).join("/")));

auto content = readPackageRecipe(recipe, parent ? parent.name : null, mode);
auto ret = new Package(content, path, parent, version_);
ret.m_infoFile = recipe;
return ret;
}

/** For a given SCM repository, returns the corresponding package.

An SCM repository is provided as its remote URL, the repository is cloned
Expand Down Expand Up @@ -399,7 +442,7 @@ class PackageManager {
return null;
}

return Package.load(destination);
return this.load(destination);
}

/**
Expand Down Expand Up @@ -787,7 +830,7 @@ symlink_exit:
logDebug("%s file(s) copied.", to!string(countFiles));

// overwrite dub.json (this one includes a version field)
auto pack = Package.load(destination, NativePath.init, null, vers.toString());
auto pack = this.load(destination, NativePath.init, null, vers.toString());

if (pack.recipePath.head != defaultPackageFilename)
// Storeinfo saved a default file, this could be different to the file from the zip.
Expand Down Expand Up @@ -850,7 +893,7 @@ symlink_exit:
this.refresh();

path.endsWithSlash = true;
auto pack = Package.load(path);
auto pack = this.load(path);
enforce(pack.name.length, "The package has no name, defined in: " ~ path.toString());
if (verName.length)
pack.version_ = Version(verName);
Expand Down Expand Up @@ -995,7 +1038,7 @@ symlink_exit:
p.normalize();
enforce(!p.absolute, "Sub package paths must be sub paths of the parent package.");
auto path = pack.path ~ p;
sp = Package.load(path, NativePath.init, pack);
sp = this.load(path, NativePath.init, pack);
} else sp = new Package(spr.recipe, pack.path, pack);

// Add the sub-package.
Expand Down Expand Up @@ -1241,7 +1284,7 @@ package struct Location {

if (!pp) {
auto infoFile = Package.findPackageFile(path);
if (!infoFile.empty) pp = Package.load(path, infoFile);
if (!infoFile.empty) pp = manager.load(path, infoFile);
else {
logWarn("Locally registered package %s %s was not found. Please run 'dub remove-local \"%s\"'.",
name, ver, path.toNativeString());
Expand Down Expand Up @@ -1304,7 +1347,7 @@ package struct Location {
p = pp;
break;
}
if (!p) p = Package.load(pack_path, packageFile);
if (!p) p = mgr.load(pack_path, packageFile);
mgr.addPackages(this.fromPath, p);
} catch (ConfigException exc) {
// Configy error message already include the path
Expand Down Expand Up @@ -1403,7 +1446,7 @@ package struct Location {
return null;

logDiagnostic("Lazily loading package %s:%s from %s", lookupName, vers, path);
auto p = Package.load(path);
auto p = mgr.load(path);
enforce(
p.version_ == vers,
format("Package %s located in %s has a different version than its path: Got %s, expected %s",
Expand Down
30 changes: 10 additions & 20 deletions source/dub/test/base.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,27 +289,17 @@ package class TestPackageManager : PackageManager
// Do nothing
}

/**
* Looks up a specific package
*
* Unlike its parent class, no lazy loading is performed.
* Additionally, as they are already deprecated, overrides are
* disabled and not available.
*/
public override Package getPackage(string name, Version vers, bool enable_overrides = false)
/**
* Loads a `Package`
*
* This is currently not implemented, and any call to it will trigger
* an assert, as that would otherwise be an access to the filesystem.
*/
protected override Package load(NativePath path, NativePath recipe = NativePath.init,
Package parent = null, string version_ = null,
StrictMode mode = StrictMode.Ignore) const
{
//assert(!enable_overrides, "Overrides are not implemented for TestPackageManager");

// Implementation inspired from `PackageManager.lookup`,
// except we replaced `load` with `lookup`.
if (auto pkg = this.m_internal.lookup(name, vers, this))
return pkg;

foreach (ref location; this.m_repositories)
if (auto p = location.lookup(name, vers, this))
return p;

return null;
assert(0, "`TestPackageManager.load` is not implemented");
}

/**
Expand Down
Loading