Fix cache (FileStore) clear to keep .gitkeep. #4452

Merged
merged 1 commit into from May 3, 2012

Conversation

Projects
None yet
4 participants
Contributor

tapajos commented Jan 13, 2012

No description provided.

@drogus drogus commented on the diff Jan 17, 2012

activesupport/lib/active_support/cache/file_store.rb
@@ -23,7 +23,7 @@ def initialize(cache_path, options = nil)
end
def clear(options = nil)
- root_dirs = Dir.entries(cache_path).reject{|f| f.in?(EXCLUDED_DIRS)}
+ root_dirs = Dir.entries(cache_path).reject{|f| f.in?(EXCLUDED_DIRS + [".gitkeep"])}
@drogus

drogus Jan 17, 2012

Member

Why didn't you add it to EXCLUDED_DIRS ?

@tapajos

tapajos Jan 17, 2012

Contributor

It is not a DIR.

On 16/01/2012, at 23:42, Piotr Sarnackireply@reply.github.com wrote:

@@ -23,7 +23,7 @@ def initialize(cache_path, options = nil)
end

  def clear(options = nil)
  •    root_dirs = Dir.entries(cache_path).reject{|f| f.in?(EXCLUDED_DIRS)}
    
  •    root_dirs = Dir.entries(cache_path).reject{|f| f.in?(EXCLUDED_DIRS + [".gitkeep"])}
    

Why didn't you add it to EXCLUDED_DIRS ?


Reply to this email directly or view it on GitHub:
https://github.com/rails/rails/pull/4452/files#r357575

@drogus

drogus Jan 17, 2012

Member

Ha, right :P

Anyways, I'm not sure if this should be hardcoded, maybe there should be some way to configure it and change it to EXCLUDED_FILES or something like that.

Also, why do you want to keep .gitkeep in cache directory? Isn't it recreated if it does not exist?

@tapajos

tapajos Jan 17, 2012

Contributor

I'm supposing that you are talking that the cache directory is recreated and not the .gitkeep. Right?

I created this patch for some reasons.

1 - Sometimes we have some issues when the cache directory is not present. This cache clear keeps the cache directory.
2 - There is a inconsistent behavior between Rails.cache.clear and rake tmp:cache:clear. The last one keep the .gitignore.
3 - The .gitkeep is created by rails new project and I think that if Rails creates it the Rails should keep it.

@tenderlove

tenderlove May 3, 2012

Owner

I hate that this is hardcoded, but if we're generating it on a new app, we probably shouldn't rm it.

@tapajos

tapajos May 3, 2012

Contributor

I can change the EXCLUDED_DIRS to something more generic and add the .gitkeep on that. Is it ok for you?

@tenderlove

tenderlove May 3, 2012

Owner

nah, this is fine. We can refactor later to something more appropriate.

Contributor

isaacsanders commented May 1, 2012

Is this still an issue?

Contributor

tapajos commented May 2, 2012

Yes

Contributor

isaacsanders commented May 3, 2012

@tenderlove Is this something that the core team wants?

tenderlove merged commit 7a3e43c into rails:master May 3, 2012

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