Skip to content

Commit

Permalink
fix Issue #528 - fetch doesn't work for recently updated package
Browse files Browse the repository at this point in the history
Revert "store metadata cache on disk"

This reverts commit 161aad0.

We only ask the package suppliers for metadata when upgrading or
searching a specific package version. In particular building an already
fetched package does not ask the package suppliers (only the package
manager responsible for locally installed packages).

In all cases where we get data from a package supplier we want to get
the latest available information. Therefor it doesn't make sense to
cache any metadata from the package suppliers on disk.

Not sure whether we still need the in memory metadata cache (e.g. for
multiple queries during dependency resolution).
  • Loading branch information
MartinNowak committed Jan 27, 2016
1 parent 54b6922 commit 0ff3594
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 87 deletions.
14 changes: 3 additions & 11 deletions source/dub/commandline.d
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ int runDubCommandLine(string[] args)
}

// execute the command
int rc;
try {
rc = cmd.execute(dub, remaining_args, app_args);
}
try return cmd.execute(dub, remaining_args, app_args);
catch (UsageException e) {
logError("%s", e.msg);
logDebug("Full exception: %s", e.toString().sanitize);
Expand All @@ -222,10 +219,6 @@ int runDubCommandLine(string[] args)
logDebug("Full exception: %s", e.toString().sanitize);
return 2;
}

if (!cmd.skipDubInitialization)
dub.shutdown();
return rc;
}

struct CommonOptions {
Expand Down Expand Up @@ -413,7 +406,7 @@ class InitCommand : Command {

void depCallback(ref PackageRecipe p, ref PackageFormat fmt) {
if (m_nonInteractive) return;

while (true) {
string rawfmt = input("Package recipe format (sdl/json)", fmt.to!string);
if (!rawfmt.length) break;
Expand Down Expand Up @@ -1482,7 +1475,6 @@ class CleanCachesCommand : Command {

override int execute(Dub dub, string[] free_args, string[] app_args)
{
dub.cleanCaches();
return 0;
}
}
Expand Down Expand Up @@ -1630,7 +1622,7 @@ class DustmiteCommand : PackageBuildCommand {
auto testcmd = appender!string();
testcmd.formattedWrite("%s dustmite --vquiet --test-package=%s --build=%s --config=%s",
thisExePath, prj.name, m_buildType, m_buildConfig);

if (m_compilerName.length) testcmd.formattedWrite(" \"--compiler=%s\"", m_compilerName);
if (m_arch.length) testcmd.formattedWrite(" --arch=%s", m_arch);
if (m_compilerStatusCode != int.min) testcmd.formattedWrite(" --compiler-status=%s", m_compilerStatusCode);
Expand Down
22 changes: 1 addition & 21 deletions source/dub/dub.d
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ class Dub {
if (skip_registry < SkipRegistry.standard)
ps ~= defaultPackageSuppliers();

auto cacheDir = m_userDubPath ~ "cache/";
foreach (p; ps)
p.cacheOp(cacheDir, CacheOp.load);

m_packageSuppliers = ps;
m_packageManager = new PackageManager(m_userDubPath, m_systemDubPath);
updatePackageSearchPath();
Expand Down Expand Up @@ -153,22 +149,6 @@ class Dub {
m_systemConfig = jsonFromFile(m_systemDubPath ~ "settings.json", true);
}

/// Perform cleanup and persist caches to disk
void shutdown()
{
auto cacheDir = m_userDubPath ~ "cache/";
foreach (p; m_packageSuppliers)
p.cacheOp(cacheDir, CacheOp.store);
}

/// cleans all metadata caches
void cleanCaches()
{
auto cacheDir = m_userDubPath ~ "cache/";
foreach (p; m_packageSuppliers)
p.cacheOp(cacheDir, CacheOp.clean);
}

@property void dryRun(bool v) { m_dryRun = v; }

/** Returns the root path (usually the current working directory).
Expand Down Expand Up @@ -796,7 +776,7 @@ class Dub {
logInfo("Package format is already %s.", destination_file_ext);
return;
}

writePackageRecipe(srcfile[0 .. $-1] ~ ("dub."~destination_file_ext), m_project.rootPackage.info);
removeFile(srcfile);
}
Expand Down
56 changes: 1 addition & 55 deletions source/dub/packagesupplier.d
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,10 @@ interface PackageSupplier {
/// returns the metadata for the package
Json getPackageDescription(string packageId, Dependency dep, bool pre_release);

/// perform cache operation
void cacheOp(Path cacheDir, CacheOp op);

static struct SearchResult { string name, description, version_; }
SearchResult[] searchPackages(string query);
}

/// operations on package supplier cache
enum CacheOp {
load,
store,
clean,
}

class FileSystemPackageSupplier : PackageSupplier {
private {
Path m_path;
Expand Down Expand Up @@ -92,9 +82,6 @@ class FileSystemPackageSupplier : PackageSupplier {
return jsonFromZip(filename, "dub.json");
}

void cacheOp(Path cacheDir, CacheOp op) {
}

SearchResult[] searchPackages(string query) {
return null;
}
Expand Down Expand Up @@ -122,10 +109,9 @@ class RegistryPackageSupplier : PackageSupplier {
struct CacheEntry { Json data; SysTime cacheTime; }
CacheEntry[string] m_metadataCache;
Duration m_maxCacheTime;
bool m_metadataCacheDirty;
}

this(URL registry)
this(URL registry)
{
m_registryUrl = registry;
m_maxCacheTime = 24.hours();
Expand Down Expand Up @@ -160,52 +146,13 @@ class RegistryPackageSupplier : PackageSupplier {
return getBestPackage(packageId, dep, pre_release);
}

void cacheOp(Path cacheDir, CacheOp op)
{
auto path = cacheDir ~ cacheFileName;
final switch (op)
{
case CacheOp.store:
if (!m_metadataCacheDirty) return;
if (!cacheDir.existsFile())
mkdirRecurse(cacheDir.toNativeString());
// TODO: method is slow due to Json escaping
atomicWriteJsonFile(path, m_metadataCache.serializeToJson());
break;

case CacheOp.load:
if (!path.existsFile()) return;
try deserializeJson(m_metadataCache, jsonFromFile(path));
catch (Exception e) {
import std.encoding;
logWarn("Error loading package cache file %s: %s", path.toNativeString(), e.msg);
logDebug("Full error: %s", e.toString().sanitize());
}
break;

case CacheOp.clean:
if (path.existsFile()) removeFile(path);
m_metadataCache.destroy();
break;
}
m_metadataCacheDirty = false;
}

private @property string cacheFileName()
{
import std.digest.md;
auto hash = m_registryUrl.toString.md5Of();
return m_registryUrl.host ~ hash[0 .. $/2].toHexString().idup ~ ".json";
}

private Json getMetadata(string packageId)
{
auto now = Clock.currTime(UTC());
if (auto pentry = packageId in m_metadataCache) {
if (pentry.cacheTime + m_maxCacheTime > now)
return pentry.data;
m_metadataCache.remove(packageId);
m_metadataCacheDirty = true;
}

auto url = m_registryUrl ~ Path(PackagesPath ~ "/" ~ packageId ~ ".json");
Expand All @@ -219,7 +166,6 @@ class RegistryPackageSupplier : PackageSupplier {
foreach (ref v; json["versions"])
v.remove("readme");
m_metadataCache[packageId] = CacheEntry(json, now);
m_metadataCacheDirty = true;
return json;
}

Expand Down

0 comments on commit 0ff3594

Please sign in to comment.