Use -n option only when needed & documentation update

README.md

@@ -27,34 +27,48 @@ Require the module in your `Capfile`:
 require 'capistrano/file-permissions'
 ```
 
-Set the (relative) paths to the files you want to be handled during deployment,
-and optionally add a user to give access.
+Set the (relative) paths to the files you want to be handled during deployment:
 
 ```ruby
 set :file_permissions_paths, ["app/logs", "app/cache"]
+```
+
+This will grant access to the deploy user. Optionally, you can add other users or groups:
+```ruby
 set :file_permissions_users, ["www-data"]
+set :file_permissions_groups, ["www-data"]
 ```
 
 ### Acl
 
+*ACL must be enabled on your server for this to work (you should be able to run the **setfacl** command)*
+
 Add the acl task to the deployment flow
 
 ```ruby
 before "deploy:updated", "deploy:set_permissions:acl"
 ```
 
-Assume `app/logs` is a shared directory, and `app/cache` is part of the normal
+Assuming `app/logs` is a shared directory, and `app/cache` is part of the normal
 release, this gem would execute the following:
 
 ```
-[..] setfacl -Rn -m u:www-data:rwX -m u:<deploy-user>:rwX <path-to-app>/shared/app/logs <path-to-app>/<release>/app/cache
+[..] 
+setfacl -R -m u:<deploy-user>:rwX <path-to-app>/shared/app/logs
+setfacl -dR -m u:<deploy-user>:rwX <path-to-app>/shared/app/logs
+setfacl -R -m u:<deploy-user>:rwX <path-to-app>/<release>/app/cache
+setfacl -dR -m u:<deploy-user>:rwX <path-to-app>/<release>/app/cache
+[..]
 ```
 
+(The -d option makes the ACL entry a default for any other file generated in the directory.)
+
+*Note: if ACL is already defined for the directory, the **-n option** will be added, as it avoids problems when setting ACL to new files generated in a directory which already had a default ACL.*
+
 ### Other tasks
 * deploy:set_permissions:chmod
 * deploy:set_permissions:chgrp
 * deploy:set_permissions:chown
-* 
 ### Configuration
 
 The gem makes the following configuration variables available (shown with defaults)

lib/capistrano/tasks/file-permissions.rake

@@ -10,8 +10,8 @@ def absolute_writable_paths
   end
 end
 
-def acl_entries(items, type = 'u', permissions = 'rwX')
-  items.map { |item| "#{type}:#{item}:#{permissions}" }
+def acl_entries(items, type = 'u', permissions = 'rwx')
+  items.map { |item| [item, "#{type}:#{item}:#{permissions}"] }.to_h
 end
 
 namespace :deploy do
@@ -37,13 +37,24 @@ namespace :deploy do
         paths = absolute_writable_paths
 
         if any? :file_permissions_groups
-          entries.push(*acl_entries(fetch(:file_permissions_groups), 'g'))
+          groups = fetch(:file_permissions_groups);
+          entries = entries.merge(acl_entries(groups, 'g'));
         end
 
-        entries = entries.map { |e| "-m #{e}" }.join(' ')
-
-        execute :setfacl, "-R", entries, *paths
-        execute :setfacl, "-dR", entries, *paths.map
+        paths.each do |path|
+          entries.each do |user, entry|
+            #checks if path already has ACL set, to determine if the -n option should be used or not
+            has_facl = (capture "getfacl --absolute-names --tabular #{path} | grep #{user}.*rwx | wc -l").chomp != "0"
+
+            if (has_facl)
+              execute :setfacl, "-Rn", "-m #{entry}", path
+              execute :setfacl, "-dRn", "-m #{entry}", path
+            else
+              execute :setfacl, "-R", "-m #{entry}", path
+              execute :setfacl, "-dR", "-m #{entry}", path
+            end
+          end
+        end
       end
     end