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

Add local cache to dist downloads #1282

Merged
merged 3 commits into from Nov 11, 2012

Conversation

Projects
None yet
5 participants
@Seldaek
Member

Seldaek commented Nov 1, 2012

No description provided.

@Seldaek Seldaek referenced this pull request Nov 1, 2012

Closed

Package cache #915

@pborreli

View changes

src/Composer/Cache.php Outdated
if ($this->enabled) {
$file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file);
$this->filesystem->ensureDirectoryExists(dirname($this->root . $file));
copy($source, $this->root . $file);

This comment has been minimized.

@pborreli

pborreli Nov 1, 2012

Contributor

maybe return the result of copy() as you do it in copyTo() ?

@@ -44,7 +49,7 @@ public function getRoot()
public function read($file)
{
$file = preg_replace('{[^a-z0-9.]}i', '-', $file);
$file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file);

This comment has been minimized.

@stof

stof Nov 1, 2012

Contributor

shouldn't you apply preg_quote to be safe ?

This comment has been minimized.

@Seldaek

Seldaek Nov 10, 2012

Member

Nope because the whitelist contains - that are escaped by preg_quote, it's a regex whitelist so you should know that and not pass garbage in it. I'll add a docblock to clarify.

@Slamdunk

This comment has been minimized.

Contributor

Slamdunk commented Nov 8, 2012

Any guess on when this feature will be merged?

@Seldaek

This comment has been minimized.

Member

Seldaek commented Nov 10, 2012

Just pushed the GC and other stuff. I'd consider this final. Please review and if all is well I'll merge tomorrow. Default cache TTL is 6 months btw, I have no idea how crazy that is, I guess it'll have to be tweaked with time.

@Seldaek Seldaek merged commit 5a9d986 into composer:master Nov 11, 2012

1 check failed

default The Travis build failed
Details
@chEbba

This comment has been minimized.

Contributor

chEbba commented Nov 13, 2012

About GC. May be it is better to have size restriction for cache, and remove old items (by access time) when size more than restriction?

@Seldaek

This comment has been minimized.

Member

Seldaek commented Nov 13, 2012

Yup, I guess a mix of both would be ideal, I'll create an issue so it's not forgotten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment