Skip to content
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

Support executable templates #1289

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/pdk/module/convert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 29 additions & 3 deletions lib/pdk/module/update_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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.
Expand All @@ -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

Expand All @@ -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.
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/pdk/template/renderer/v1/legacy_template_dir.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
4 changes: 3 additions & 1 deletion lib/pdk/template/renderer/v1/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 10 additions & 0 deletions lib/pdk/util/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
92 changes: 88 additions & 4 deletions spec/unit/pdk/module/convert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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./)
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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'

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 } }

Expand Down Expand Up @@ -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
Expand All @@ -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'

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading