Merge pull request #1289 from nabertrand/executable_templates_with_tests

Support executable templates

lib/pdk/module/convert.rb

@@ -148,20 +148,25 @@ def stage_changes!(context = PDK.context)
           module_name = new_metadata.nil? ? 'new-module' : new_metadata.data['name']
           metadata_for_render = new_metadata.nil? ? {} : new_metadata.data
 
-          template_dir.render_new_module(module_name, metadata_for_render) do |relative_file_path, file_content, file_status|
+          template_dir.render_new_module(module_name, metadata_for_render) do |relative_file_path, file_content, file_status, file_executable|
             absolute_file_path = File.join(module_dir, relative_file_path)
             case file_status
             when :unmanage
               PDK.logger.debug(format("skipping '%{path}'", path: absolute_file_path))
             when :delete
               update_manager.remove_file(absolute_file_path)
             when :init
-              update_manager.add_file(absolute_file_path, file_content) if convert? && !PDK::Util::Filesystem.exist?(absolute_file_path)
+              if convert? && !PDK::Util::Filesystem.exist?(absolute_file_path)
+                update_manager.add_file(absolute_file_path, file_content)
+                update_manager.make_file_executable(absolute_file_path) if file_executable
+              end
             when :manage
               if PDK::Util::Filesystem.exist?(absolute_file_path)
                 update_manager.modify_file(absolute_file_path, file_content)
+                update_manager.make_file_executable(absolute_file_path) if file_executable && !PDK::Util::Filesystem.executable?(absolute_file_path)
               else
                 update_manager.add_file(absolute_file_path, file_content)
+                update_manager.make_file_executable(absolute_file_path) if file_executable
               end
             end
           end

lib/pdk/module/update_manager.rb

@@ -11,6 +11,7 @@ def initialize
         @modified_files = Set.new
         @added_files = Set.new
         @removed_files = Set.new
+        @executable_files = Set.new
         @diff_cache = {}
       end
 
@@ -37,6 +38,13 @@ def remove_file(path)
         @removed_files << path
       end
 
+      # Store a pending file execute mode change.
+      #
+      # @param path [String] The path to the file to be made executable.
+      def make_file_executable(path)
+        @executable_files << path
+      end
+
       # Generate a summary of the changes that will be applied to the module.
       #
       # @raise (see #calculate_diffs)
@@ -49,7 +57,8 @@ def changes
         {
           added: @added_files,
           removed: @removed_files.select { |f| PDK::Util::Filesystem.exist?(f) },
-          modified: @diff_cache.compact
+          modified: @diff_cache.compact,
+          'made executable': @executable_files
         }
       end
 
@@ -60,7 +69,8 @@ def changes
       def changes?
         !changes[:added].empty? ||
           !changes[:removed].empty? ||
-          changes[:modified].any? { |_, value| !value.nil? }
+          changes[:modified].any? { |_, value| !value.nil? } ||
+          !changes[:'made executable'].empty?
       end
 
       # Check if the update manager will change the specified file upon sync.
@@ -72,13 +82,15 @@ def changes?
       def changed?(path)
         changes[:added].any? { |add| add[:path] == path } ||
           changes[:removed].include?(path) ||
-          changes[:modified].key?(path)
+          changes[:modified].key?(path) ||
+          changes[:'made executable'].include?(path)
       end
 
       def clear!
         @modified_files.clear
         @added_files.clear
         @removed_files.clear
+        @executable_files.clear
         nil
       end
 
@@ -100,6 +112,10 @@ def sync_changes!
         files_to_write.each do |file|
           write_file(file[:path], file[:content])
         end
+
+        @executable_files.each do |file|
+          update_execute_bits(file)
+        end
       end
 
       # Remove a file from disk.
@@ -215,6 +231,16 @@ def unified_diff(path, old_content, new_content, lines_of_context = 3)
 
         output.join($INPUT_RECORD_SEPARATOR)
       end
+
+      # Set the execute bits on a file
+      def update_execute_bits(path)
+        require 'pdk/util/filesystem'
+
+        PDK.logger.debug(format("making '%{path}' executable", path: path))
+        PDK::Util::Filesystem.make_executable(path)
+      rescue Errno::EACCES
+        raise PDK::CLI::ExitWithError, format("You do not have permission to make '%{path}' executable", path: path)
+      end
     end
   end
 end

lib/pdk/template/renderer/v1/legacy_template_dir.rb

@@ -56,7 +56,7 @@ def config_for(dest_path, sync_config_path = nil)
               @config = conf_defaults
               @config.deep_merge!(@sync_config, knockout_prefix: '---') unless @sync_config.nil?
             end
-            file_config = @config.fetch(:global, {})
+            file_config = @config.fetch('common', {}).clone
             file_config['module_metadata'] = @module_metadata
             file_config.merge!(@config.fetch(dest_path, {})) unless dest_path.nil?
             file_config.merge!(@config).tap do |c|

lib/pdk/template/renderer/v1/renderer.rb

@@ -95,7 +95,9 @@ def render_module(options = {})
                 end
               end
 
-              yield dest_path, dest_content, dest_status
+              dest_executable = config['manage_execute_permissions'] && PDK::Util::Filesystem.executable?(File.join(template_loc, template_file))
+
+              yield dest_path, dest_content, dest_status, dest_executable
             end
           end
           # :nocov:

lib/pdk/util/filesystem.rb

@@ -27,6 +27,11 @@ def read_file(file, nil_on_error: false, open_args: 'r')
       end
       module_function :read_file
 
+      def make_executable(file)
+        FileUtils.chmod('a+x', file)
+      end
+      module_function :make_executable
+
       # :nocov:
       # These methods just wrap core Ruby functionality and
       # can be ignored for code coverage
@@ -133,6 +138,11 @@ def mv(*args, **kwargs)
         end
       end
       module_function :mv
+
+      def executable?(*args)
+        File.executable?(*args)
+      end
+      module_function :executable?
       # :nocov:
     end
   end

spec/unit/pdk/module/convert_spec.rb

@@ -58,6 +58,12 @@ def module_path(relative_path)
     end
   end
 
+  shared_context 'files made executable in the summary' do
+    before do
+      allow($stdout).to receive(:puts).with(/-Files to be made executable-/i)
+    end
+  end
+
   shared_context 'outputs a convert report' do
     before do
       allow($stdout).to receive(:puts).with(/You can find detailed differences in convert_report.txt./)
@@ -88,19 +94,20 @@ def module_path(relative_path)
     let(:update_manager) { instance_double(PDK::Module::UpdateManager, sync_changes!: true) }
     let(:template_dir) { instance_double(PDK::Template::TemplateDir, metadata: {}) }
     let(:metadata) { instance_double(PDK::Module::Metadata, data: {}) }
-    let(:template_files) { { path: 'a/path/to/file', content: 'file contents', status: :manage } }
+    let(:template_files) { { path: 'a/path/to/file', content: 'file contents', status: :manage, executable: true } }
     let(:added_files) { Set.new }
     let(:removed_files) { Set.new }
     let(:modified_files) { {} }
+    let(:executable_files) { Set.new }
 
     before do
-      changes = { added: added_files, removed: removed_files, modified: modified_files }
+      changes = { added: added_files, removed: removed_files, modified: modified_files, 'made executable': executable_files }
 
       allow(PDK::Module::UpdateManager).to receive(:new).and_return(update_manager)
       allow(instance).to receive(:update_metadata).with(any_args).and_return(metadata)
       allow(PDK::Template).to receive(:with).with(anything, anything).and_yield(template_dir)
       allow(PDK::Util::Git).to receive(:repo?).with(anything).and_return(true)
-      allow(template_dir).to receive(:render_new_module).and_yield(template_files[:path], template_files[:content], template_files[:status])
+      allow(template_dir).to receive(:render_new_module).and_yield(template_files[:path], template_files[:content], template_files[:status], template_files[:executable])
       allow(update_manager).to receive(:changes).and_return(changes)
       allow(update_manager).to receive(:changed?).with('Gemfile').and_return(false)
       allow(update_manager).to receive(:unlink_file).with('Gemfile.lock')
@@ -172,6 +179,7 @@ def module_path(relative_path)
     context 'when the Gemfile has been modified' do
       include_context 'has changes in the summary'
       include_context 'modified files in the summary'
+      include_context 'files made executable in the summary'
       include_context 'outputs a convert report'
       include_context 'prompt to continue', true
       include_context 'completes a convert'
@@ -184,6 +192,7 @@ def module_path(relative_path)
 
         allow(update_manager).to receive(:add_file).with(module_path('metadata.json'), anything)
         allow(update_manager).to receive(:modify_file).with(module_path(template_files[:path]), template_files[:content])
+        allow(update_manager).to receive(:make_file_executable).with(module_path(template_files[:path]))
         allow($stdout).to receive(:puts).with(/1 files modified/)
         allow(update_manager).to receive(:changed?).with('Gemfile').and_return(true)
         allow(update_manager).to receive(:remove_file).with(anything)
@@ -213,6 +222,7 @@ def module_path(relative_path)
     context 'when there are changes to apply' do
       include_context 'has changes in the summary'
       include_context 'modified files in the summary'
+      include_context 'files made executable in the summary'
       include_context 'outputs a convert report'
       include_context 'completes a convert'
 
@@ -221,9 +231,11 @@ def module_path(relative_path)
         allow(update_manager).to receive(:modify_file).with(any_args)
         allow(update_manager).to receive(:changes?).and_return(true)
         allow($stdout).to receive(:puts).with(['some/file'])
+        allow($stdout).to receive(:puts).with(['some/executable/file'])
 
         allow(update_manager).to receive(:add_file).with(module_path('metadata.json'), anything)
         allow(update_manager).to receive(:modify_file).with(module_path(template_files[:path]), template_files[:content])
+        allow(update_manager).to receive(:make_file_executable).with(module_path(template_files[:path]))
         allow($stdout).to receive(:puts).with(/1 files modified/)
         allow($stdout).to receive(:puts).with(/You can find a report of differences in convert_report.txt./)
       end
@@ -234,11 +246,20 @@ def module_path(relative_path)
         }
       end
 
+      let(:executable_files) do
+        Set.new(
+          [
+            'some/executable/file'
+          ]
+        )
+      end
+
       context 'and run normally' do
         include_context 'prompt to continue', false
 
         it 'prints a diff of the changed files' do
           expect($stdout).to receive(:puts).with(['some/file'])
+          expect($stdout).to receive(:puts).with(['some/executable/file'])
         end
 
         it 'prompts the user to continue' do
@@ -252,6 +273,36 @@ def module_path(relative_path)
             expect(update_manager).to receive(:sync_changes!)
           end
 
+          it 'lists the count of modified files' do
+            expect($stdout).to receive(:puts).with(/1 files modified/)
+          end
+
+          it 'lists the count of files made executable' do
+            expect($stdout).to receive(:puts).with(/1 files made executable/)
+          end
+
+          context 'when managing file execute bits' do
+            context 'when file exists and has correct execute bits' do
+              before do
+                allow(PDK::Util::Filesystem).to receive(:executable?).with(module_path('/a/path/to/file')).and_return(true)
+              end
+
+              it 'does not stage the file for making executable' do
+                expect(update_manager).not_to receive(:make_file_executable).with(module_path('/a/path/to/file'))
+              end
+            end
+
+            context 'when file exists but has incorrect execute bits' do
+              before do
+                allow(PDK::Util::Filesystem).to receive(:executable?).with(module_path('/a/path/to/file')).and_return(false)
+              end
+
+              it 'stages the file for making executable' do
+                expect(update_manager).to receive(:make_file_executable).with(module_path('/a/path/to/file'))
+              end
+            end
+          end
+
           context 'and it is to add tests' do
             let(:options) { { 'add-tests': true } }
 
@@ -352,7 +403,7 @@ def module_path(relative_path)
     context 'when there are init files to add' do
       let(:options) { { noop: true } }
       let(:template_files) do
-        { path: 'a/path/to/file', content: 'file contents', status: :init }
+        { path: 'a/path/to/file', content: 'file contents', status: :init, executable: true }
       end
 
       context 'and the files already exist' do
@@ -361,29 +412,40 @@ def module_path(relative_path)
         before do
           allow(PDK::Util::Filesystem).to receive(:exist?).with(module_path(template_files[:path])).and_return(true)
           allow(update_manager).to receive(:changes?).and_return(false)
+          allow(update_manager).to receive(:add_file).with(module_path('metadata.json'), anything)
         end
 
         it 'does not stage the file for addition' do
           expect(update_manager).not_to receive(:add_file).with(module_path(template_files[:path]), anything)
         end
+
+        it 'does not stage the file for making executable' do
+          expect(update_manager).not_to receive(:make_file_executable).with(module_path(template_files[:path]))
+        end
       end
 
       context 'and the files do not exist' do
         before do
           allow(PDK::Util::Filesystem).to receive(:exist?).with(module_path(template_files[:path])).and_return(false)
           allow(update_manager).to receive(:changes?).and_return(true)
           allow(update_manager).to receive(:add_file)
+          allow(update_manager).to receive(:make_file_executable)
         end
 
         it 'stages the file for addition' do
           expect(update_manager).to receive(:add_file).with(module_path(template_files[:path]), template_files[:content])
         end
+
+        it 'stages the file for making executable' do
+          expect(update_manager).to receive(:make_file_executable).with(module_path(template_files[:path]))
+        end
       end
     end
 
     context 'when there are files to add' do
       include_context 'has changes in the summary'
       include_context 'added files in the summary'
+      include_context 'files made executable in the summary'
       include_context 'outputs a convert report'
       include_context 'completes a convert'
 
@@ -396,12 +458,22 @@ def module_path(relative_path)
         )
       end
 
+      let(:executable_files) do
+        Set.new(
+          [
+            'path/to/executable/file'
+          ]
+        )
+      end
+
       before do
         allow(update_manager).to receive(:changes?).and_return(true)
         allow($stdout).to receive(:puts).with(['path/to/file'])
+        allow($stdout).to receive(:puts).with(['path/to/executable/file'])
 
         allow(update_manager).to receive(:add_file).with(module_path('metadata.json'), anything)
         allow(update_manager).to receive(:add_file).with(module_path(template_files[:path]), template_files[:content])
+        allow(update_manager).to receive(:make_file_executable).with(module_path(template_files[:path]))
         allow($stdout).to receive(:puts).with(/1 files added/)
         allow($stdout).to receive(:puts).with(/You can find a report of differences in convert_report.txt./)
       end
@@ -413,6 +485,10 @@ def module_path(relative_path)
           expect($stdout).to receive(:puts).with(['path/to/file'])
         end
 
+        it 'prints a path of the files made executable' do
+          expect($stdout).to receive(:puts).with(['path/to/executable/file'])
+        end
+
         it 'prompts the user to continue' do
           expect(PDK::CLI::Util).to receive(:prompt_for_yes).with(anything).and_return(false)
         end
@@ -423,6 +499,14 @@ def module_path(relative_path)
           it 'syncs the pending changes' do
             expect(update_manager).to receive(:sync_changes!)
           end
+
+          it 'lists the count of added files' do
+            expect($stdout).to receive(:puts).with(/1 files added/)
+          end
+
+          it 'lists the count of files made executable' do
+            expect($stdout).to receive(:puts).with(/1 files made executable/)
+          end
         end
 
         context 'if the user chooses not to continue' do