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

Jump to bottom

Simplify mocking IO in PackageManager, add a test for add-path #2881

Merged
merged 8 commits into from
Feb 26, 2024
Merged

Simplify mocking IO in PackageManager, add a test for add-path #2881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2fae039
Move Package.findPackageFile to PackageManager, mock
Geod24 Feb 23, 2024
9309a3b
PackageManager: Mock readText, do not mock load
Geod24 Feb 23, 2024
7a01053
Remove direct IO from Location.scanLocalPackages
Geod24 Feb 23, 2024
e8dd304
Remove the need to mock PackageManager.refresh()
Geod24 Feb 23, 2024
1ba6ee6
Mock IO in writeLocalPackageList / writeOverrides
Geod24 Feb 23, 2024
08110bb
Add the ability to re-invoke Dub with the same vfs
Geod24 Feb 24, 2024
9309b09
Test: Make NativePath import public
Geod24 Feb 24, 2024
1082f7f
Test that `add-path` packages take priority
Geod24 Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/dub/commandline.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ class Command {

private bool loadCwdPackage(Dub dub, bool warn_missing_package)
{
auto filePath = Package.findPackageFile(dub.rootPath);
auto filePath = dub.packageManager.findPackageFile(dub.rootPath);

if (filePath.empty) {
if (warn_missing_package) {
Expand Down
1 change: 1 addition & 0 deletions source/dub/package_.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class Package {
Returns the full path to the package file, if any was found.
Otherwise returns an empty path.
*/
deprecated("Use `PackageManager.findPackageFile`")
static NativePath findPackageFile(NativePath directory)
{
foreach (file; packageInfoFiles) {
Expand Down
100 changes: 67 additions & 33 deletions source/dub/packagemanager.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ class PackageManager {
StrictMode mode = StrictMode.Ignore)
{
if (recipe.empty)
recipe = Package.findPackageFile(path);
recipe = this.findPackageFile(path);

enforce(!recipe.empty,
"No package file found in %s, expected one of %s"
Expand All @@ -390,12 +390,32 @@ class PackageManager {

const PackageName pname = parent
? PackageName(parent.name) : PackageName.init;
auto content = readPackageRecipe(recipe, pname, mode);
string text = this.readText(recipe);
auto content = parsePackageRecipe(
text, recipe.toNativeString(), pname, null, mode);
auto ret = new Package(content, path, parent, version_);
ret.m_infoFile = recipe;
return ret;
}

/** Searches the given directory for package recipe files.
*
* Params:
* directory = The directory to search
*
* Returns:
* Returns the full path to the package file, if any was found.
* Otherwise returns an empty path.
*/
public NativePath findPackageFile(NativePath directory)
{
foreach (file; packageInfoFiles) {
auto filename = directory ~ file.filename;
if (this.existsFile(filename)) return filename;
}
return NativePath.init;
}

/** 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 @@ -669,14 +689,14 @@ class PackageManager {
void addOverride(PlacementLocation scope_, string package_, Dependency version_spec, Version target)
{
m_repositories[scope_].overrides ~= PackageOverride(package_, version_spec, target);
m_repositories[scope_].writeOverrides();
m_repositories[scope_].writeOverrides(this);
}
/// ditto
deprecated("Use the overload that accepts a `VersionRange` as 3rd argument")
void addOverride(PlacementLocation scope_, string package_, Dependency version_spec, NativePath target)
{
m_repositories[scope_].overrides ~= PackageOverride(package_, version_spec, target);
m_repositories[scope_].writeOverrides();
m_repositories[scope_].writeOverrides(this);
}

/// Ditto
Expand All @@ -696,13 +716,13 @@ class PackageManager {
package(dub) void addOverride_(PlacementLocation scope_, string package_, VersionRange source, Version target)
{
m_repositories[scope_].overrides ~= PackageOverride_(package_, source, target);
m_repositories[scope_].writeOverrides();
m_repositories[scope_].writeOverrides(this);
}
// Non deprecated version that is used by `commandline`. Do not use!
package(dub) void addOverride_(PlacementLocation scope_, string package_, VersionRange source, NativePath target)
{
m_repositories[scope_].overrides ~= PackageOverride_(package_, source, target);
m_repositories[scope_].writeOverrides();
m_repositories[scope_].writeOverrides(this);
}

/** Removes an existing package override.
Expand All @@ -729,7 +749,7 @@ class PackageManager {
if (ovr.package_ != package_ || ovr.source != src)
continue;
rep.overrides = rep.overrides[0 .. i] ~ rep.overrides[i+1 .. $];
(*rep).writeOverrides();
(*rep).writeOverrides(this);
return;
}
throw new Exception(format("No override exists for %s %s", package_, src));
Expand Down Expand Up @@ -970,7 +990,7 @@ symlink_exit:

addPackages(*packs, pack);

this.m_repositories[type].writeLocalPackageList();
this.m_repositories[type].writeLocalPackageList(this);

logInfo("Registered package: %s (version: %s)", pack.name, pack.version_);
return pack;
Expand Down Expand Up @@ -1000,7 +1020,7 @@ symlink_exit:
.filter!(en => !to_remove.canFind(en.index))
.map!(en => en.value).array;

this.m_repositories[type].writeLocalPackageList();
this.m_repositories[type].writeLocalPackageList(this);

foreach(ver, name; removed)
logInfo("Deregistered package: %s (version: %s)", name, ver);
Expand All @@ -1010,14 +1030,14 @@ symlink_exit:
void addSearchPath(NativePath path, PlacementLocation type)
{
m_repositories[type].searchPath ~= path;
this.m_repositories[type].writeLocalPackageList();
this.m_repositories[type].writeLocalPackageList(this);
}

/// Removes a search path from the given type.
void removeSearchPath(NativePath path, PlacementLocation type)
{
m_repositories[type].searchPath = m_repositories[type].searchPath.filter!(p => p != path)().array();
this.m_repositories[type].writeLocalPackageList();
this.m_repositories[type].writeLocalPackageList(this);
}

deprecated("Use `refresh()` without boolean argument(same as `refresh(false)`")
Expand Down Expand Up @@ -1046,7 +1066,7 @@ symlink_exit:
repository.scan(this, refresh);

foreach (ref repository; this.m_repositories)
repository.loadOverrides();
repository.loadOverrides(this);
this.m_initialized = true;
}

Expand Down Expand Up @@ -1182,6 +1202,13 @@ symlink_exit:
return dub.internal.vibecompat.core.file.existsDirectory(path);
}

/// Ditto
protected void ensureDirectory(NativePath path)
{
static import dub.internal.vibecompat.core.file;
return dub.internal.vibecompat.core.file.ensureDirectory(path);
}

/// Ditto
protected bool existsFile(NativePath path)
{
Expand All @@ -1203,6 +1230,13 @@ symlink_exit:
return dub.internal.vibecompat.core.file.writeFile(path, data);
}

/// Ditto
protected string readText(NativePath path)
{
static import dub.internal.vibecompat.core.file;
return dub.internal.vibecompat.core.file.readText(path);
}

/// Ditto
protected alias IterateDirDg = int delegate(scope int delegate(ref FileInfo));

Expand Down Expand Up @@ -1351,17 +1385,17 @@ package struct Location {
this.packagePath = path;
}

void loadOverrides()
void loadOverrides(PackageManager mgr)
{
import dub.internal.vibecompat.core.file;

this.overrides = null;
auto ovrfilepath = this.packagePath ~ LocalOverridesFilename;
if (existsFile(ovrfilepath)) {
if (mgr.existsFile(ovrfilepath)) {
logWarn("Found local override file: %s", ovrfilepath);
logWarn(OverrideDepMsg);
logWarn("Replace with a path-based dependency in your project or a custom cache path");
foreach (entry; jsonFromFile(ovrfilepath)) {
const text = mgr.readText(ovrfilepath);
auto json = parseJsonString(text, ovrfilepath.toNativeString());
foreach (entry; json) {
PackageOverride_ ovr;
ovr.package_ = entry["name"].get!string;
ovr.source = VersionRange.fromString(entry["version"].get!string);
Expand All @@ -1372,10 +1406,8 @@ package struct Location {
}
}

private void writeOverrides()
private void writeOverrides(PackageManager mgr)
{
import dub.internal.vibecompat.core.file;

Json[] newlist;
foreach (ovr; this.overrides) {
auto jovr = Json.emptyObject;
Expand All @@ -1388,14 +1420,14 @@ package struct Location {
newlist ~= jovr;
}
auto path = this.packagePath;
ensureDirectory(path);
writeJsonFile(path ~ LocalOverridesFilename, Json(newlist));
mgr.ensureDirectory(path);
auto app = appender!string();
app.writePrettyJsonString(Json(newlist));
mgr.writeFile(path ~ LocalOverridesFilename, app.data);
}

private void writeLocalPackageList()
private void writeLocalPackageList(PackageManager mgr)
{
import dub.internal.vibecompat.core.file;

Json[] newlist;
foreach (p; this.searchPath) {
auto entry = Json.emptyObject;
Expand All @@ -1414,15 +1446,15 @@ package struct Location {
}

NativePath path = this.packagePath;
ensureDirectory(path);
writeJsonFile(path ~ LocalPackagesFilename, Json(newlist));
mgr.ensureDirectory(path);
auto app = appender!string();
app.writePrettyJsonString(Json(newlist));
mgr.writeFile(path ~ LocalPackagesFilename, app.data);
}

// load locally defined packages
void scanLocalPackages(bool refresh, PackageManager manager)
{
import dub.internal.vibecompat.core.file;

NativePath list_path = this.packagePath;
Package[] packs;
NativePath[] paths;
Expand All @@ -1431,7 +1463,9 @@ package struct Location {
if (!manager.existsFile(local_package_file)) return;

logDiagnostic("Loading local package map at %s", local_package_file.toNativeString());
auto packlist = jsonFromFile(local_package_file);
const text = manager.readText(local_package_file);
auto packlist = parseJsonString(
text, local_package_file.toNativeString());
enforce(packlist.type == Json.Type.array, LocalPackagesFilename ~ " must contain an array.");
foreach (pentry; packlist) {
try {
Expand All @@ -1452,7 +1486,7 @@ package struct Location {
}

if (!pp) {
auto infoFile = Package.findPackageFile(path);
auto infoFile = manager.findPackageFile(path);
if (!infoFile.empty) pp = manager.load(path, infoFile);
else {
logWarn("Locally registered package %s %s was not found. Please run 'dub remove-local \"%s\"'.",
Expand Down Expand Up @@ -1533,7 +1567,7 @@ package struct Location {
if (!pdir.isDirectory) continue;

const pack_path = path ~ (pdir.name ~ "/");
auto packageFile = Package.findPackageFile(pack_path);
auto packageFile = mgr.findPackageFile(pack_path);

if (isManaged(path)) {
// Old / flat directory structure, used in non-standard path
Expand All @@ -1554,7 +1588,7 @@ package struct Location {
if (!versdir.isDirectory) continue;
auto vers_path = pack_path ~ versdir.name ~ (pdir.name ~ "/");
if (!mgr.existsDirectory(vers_path)) continue;
packageFile = Package.findPackageFile(vers_path);
packageFile = mgr.findPackageFile(vers_path);
loadInternal(vers_path, packageFile);
}
}
Expand Down