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

(SDK-261) Manage basic bundler operations for module dev #62

Merged
merged 1 commit into from
Jun 7, 2017
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
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ language: ruby
before_install: gem install bundler -v 1.14.6
bundler_args: "--without development"
script:
- "bundle binstubs pdk"
- "cat Gemfile"
- "cat Gemfile.lock"
- "bundle exec rake $CHECK"
Expand Down
1 change: 1 addition & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ test_script:
- SET PATH=C:\%PLATFORM%\bin;%PATH%
- SET LOG_SPEC_ORDER=true
- bundle install --jobs 4 --retry 2 --without development
- bundle binstubs pdk
- type Gemfile.lock
- ruby -v
- bundle exec rake spec acceptance:local
Expand Down
25 changes: 22 additions & 3 deletions lib/pdk/cli/exec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ def self.execute(*cmd)
process.io.stderr = Tempfile.new('stderr')

begin
# Make this process leader of a new group
process.leader = true

# start the process
process.start

Expand All @@ -23,6 +26,7 @@ def self.execute(*cmd)
stdout = process.io.stdout.open.read
stderr = process.io.stderr.open.read
ensure
process.stop unless process.exited?
process.io.stdout.close
process.io.stderr.close
end
Expand All @@ -46,9 +50,24 @@ def self.git_bindir

def self.git(*args)
vendored_bin_path = File.join(git_bindir, 'git')
git_path = File.exists?(vendored_bin_path) ? vendored_bin_path : 'git'
PDK.logger.debug(_("Using git from the system PATH, instead of '%{vendored_bin_path}'") % { vendored_bin_path: vendored_bin_path})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this was always logging this message even when the vendored bin was being used :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d'oh

execute(git_path, *args)

execute(try_vendored_bin(vendored_bin_path, 'git'), *args)
end

def self.bundle(*args)
bundle_bin = Gem.win_platform? ? 'bundle.bat' : 'bundle'
vendored_bin_path = File.join(pdk_basedir, 'private', 'ruby', '2.1.9', 'bin', bundle_bin)

execute(try_vendored_bin(vendored_bin_path, bundle_bin), *args)
end

def self.try_vendored_bin(vendored_bin_path, fallback)
if File.exists?(vendored_bin_path)
return vendored_bin_path
else
PDK.logger.debug(_("Trying '%{fallback}' from the system PATH, instead of '%{vendored_bin_path}'") % {fallback: fallback, vendored_bin_path: vendored_bin_path})
return fallback
end
end
end
end
Expand Down
1 change: 0 additions & 1 deletion lib/pdk/cli/tests/unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def self.command
report = Report.new(opts.fetch(:'report-file'), format)
end

puts _("Running unit tests: %{tests}") % {tests: tests}
PDK::Test::Unit.invoke(tests, report)
end
end
Expand Down
5 changes: 5 additions & 0 deletions lib/pdk/tests/unit.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'pdk'
require 'pdk/cli/exec'
require 'pdk/util/bundler'

module PDK
module Test
Expand All @@ -13,6 +14,10 @@ def self.cmd(tests)
end

def self.invoke(tests, report = nil)
PDK::Util::Bundler.ensure_bundle!

puts _("Running unit tests: %{tests}") % {tests: tests}

output = PDK::CLI::Exec.execute(cmd(tests))
report.write(output) if report
end
Expand Down
14 changes: 14 additions & 0 deletions lib/pdk/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,19 @@ def make_tmpdir_name(base)
Dir::Tmpname.make_tmpname(File.join(Dir.tmpdir, base), nil)
end
module_function :make_tmpdir_name

# Returns the fully qualified path to a per-user PDK cachedir.
#
# @return [String] Fully qualified path to per-user PDK cachedir.
def cachedir
if Gem.win_platform?
basedir = ENV['APPDATA']
else
basedir = Dir.home
end

return File.join(basedir, '.pdk', 'cache')
end
module_function :cachedir
end
end
139 changes: 139 additions & 0 deletions lib/pdk/util/bundler.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
require 'bundler'
require 'tty-spinner'
require 'pdk/util'
require 'pdk/cli/exec'

module PDK
module Util
module Bundler
class BundleHelper; end

def self.ensure_bundle!
bundle = BundleHelper.new

if bundle.has_gemfile?
if !bundle.locked?
unless bundle.lock!
raise PDK::CLI::FatalError, _("Unable to resolve Gemfile dependencies.")
end
end

if !bundle.installed?
unless bundle.install!
raise PDK::CLI::FatalError, _("Unable to install missing Gemfile dependencies.")
end
end
end
end

class BundleHelper
def has_gemfile?
# return a pure boolean
!!gemfile
end

def locked?
# return a pure boolean
!!gemfile_lock
end

def installed?
output_start(_("Checking for missing Gemfile dependencies"))

result = invoke('check', "--gemfile=#{gemfile}", "--path=#{bundle_cachedir}")

output_end(:success)

return result[:exit_code] == 0
end

def lock!
output_start(_("Resolving Gemfile dependencies"))

result = invoke('lock')

if result[:exit_code] == 0
output_end(:success)
else
output_end(:failure)
$stderr.puts result[:stdout]
$stderr.puts result[:stderr]
end

return result[:exit_code] == 0
end

def install!
output_start(_("Installing missing Gemfile dependencies"))

result = invoke('install', "--gemfile=#{gemfile}", "--path=#{bundle_cachedir}")

if result[:exit_code] == 0
output_end(:success)
else
output_end(:failure)
$stderr.puts result[:stdout]
$stderr.puts result[:stderr]
end

return result[:exit_code] == 0
end

private

def invoke(*args)
::Bundler.with_clean_env do
PDK::CLI::Exec.bundle(*args)
end
end

def find_upwards(target)
previous = nil
current = File.expand_path(Dir.pwd)

until !File.directory?(current) || current == previous
filename = File.join(current, target)
return filename if File.file?(filename)
current, previous = File.expand_path("..", current), current
end
end

def gemfile
@gemfile ||= find_upwards('Gemfile')
end

def gemfile_lock
@gemfile_lock ||= find_upwards('Gemfile.lock')
end

def bundle_cachedir
@bundle_cachedir ||= File.join(PDK::Util.cachedir, 'bundler')
end

# These two output_* methods are just a way to not try to do the spinner stuff on Windows for now.
def output_start(message)
if Gem.win_platform?
$stderr.print "#{message}... "
else
@spinner = TTY::Spinner.new("[:spinner] #{message}")
@spinner.auto_spin
end
end

def output_end(state)
if Gem.win_platform?
$stderr.print (state == :success) ? _("done.\n") : _("FAILURE!\n")
else
if state == :success
@spinner.success
else
@spinner.error
end

remove_instance_variable(:@spinner)
end
end
end
end
end
end
1 change: 1 addition & 0 deletions pdk.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency 'cri', '~> 2.7.1'
spec.add_runtime_dependency 'childprocess', '~> 0.6.2'
spec.add_runtime_dependency 'gettext-setup', '~> 0.24'
spec.add_runtime_dependency 'tty-spinner', '~> 0.4'
end
13 changes: 3 additions & 10 deletions spec/acceptance/cli_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
require 'spec_helper_acceptance'

describe 'Basic usage of the CLI' do
context 'when the --help options is used' do
let(:help_output) { shell_ex("#{path_to_pdk} --help") }

it 'displays help text on stdout' do
expect(help_output.stdout).to match(/NAME.*USAGE.*DESCRIPTION.*COMMANDS.*OPTIONS/m)
end

it 'has an empty stderr' do
expect(help_output.stderr).to eq('')
end
describe command("#{path_to_pdk} --help") do
its(:stdout) { is_expected.to match(/NAME.*USAGE.*DESCRIPTION.*COMMANDS.*OPTIONS/m) }
its(:stderr) { is_expected.to match(/\A\Z/) }
end
end
60 changes: 60 additions & 0 deletions spec/util/bundler_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
require 'spec_helper'

RSpec.describe PDK::Util::Bundler do
describe '.ensure_bundle!' do
context 'when there is no Gemfile' do
before(:each) do
allow(File).to receive(:file?).with(/Gemfile$/).and_return(false)
end

it 'does nothing' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which situations do you expect this to be the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, technically a puppet module doesn't have to have a Gemfile right? Theoretically somebody could still use pdk to run lint or something on such a module. Did we decide to ignore any use cases that did not involve a module that was generated with pdk new?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, technically no Gemfile is required. Practically, all well-maintained do.

The goal is to provide a template so that pdk new generated modules look very similar to what is currently "on the market", to the point where the pdk can handle unmodified supported and voxpupuli modules.

For less well maintained modules, having a pdk upgrade command applying a template on top of an existing module provides more value, as it'll reduce long-term friction of supporting/working with multiple models.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we just need to figure out how the most graceful way to handle the lack of a Gemfile is. We can change the method name to something like require_bundle! to make it clear that the tool should exit 1 if there is no Gemfile present in the module.

expect(PDK::CLI::Exec).to_not receive(:bundle)

PDK::Util::Bundler.ensure_bundle!
end
end

context 'when there is no Gemfile.lock' do
before(:each) do
allow(File).to receive(:file?).with(/Gemfile$/).and_return(true)
allow(File).to receive(:file?).with(/Gemfile\.lock$/).and_return(false)
allow(PDK::CLI::Exec).to receive(:bundle).with('check', any_args).and_return({exit_code: 1})
allow(PDK::CLI::Exec).to receive(:bundle).with('install', any_args).and_return({exit_code: 0})
end

it 'generates Gemfile.lock' do
expect(PDK::CLI::Exec).to receive(:bundle).with('lock', any_args).and_return({exit_code: 0})

expect { PDK::Util::Bundler.ensure_bundle! }.to output(/resolving gemfile/i).to_stderr
end
end

context 'when there are missing gems' do
before(:each) do
allow(File).to receive(:file?).with(/Gemfile$/).and_return(true)
allow(File).to receive(:file?).with(/Gemfile\.lock$/).and_return(true)
allow(PDK::CLI::Exec).to receive(:bundle).with('check', any_args).and_return({exit_code: 1})
end

it 'installs missing gems' do
expect(PDK::CLI::Exec).to receive(:bundle).with('install', any_args).and_return({exit_code: 0})

expect { PDK::Util::Bundler.ensure_bundle! }.to output(/installing missing gemfile/i).to_stderr
end
end

context 'when there are no missing gems' do
before(:each) do
allow(File).to receive(:file?).with(/Gemfile$/).and_return(true)
allow(File).to receive(:file?).with(/Gemfile\.lock$/).and_return(true)
end

it 'checks for missing but does not install anything' do
expect(PDK::CLI::Exec).to receive(:bundle).with('check', any_args).and_return({exit_code: 0})
expect(PDK::CLI::Exec).to_not receive(:bundle).with('install', any_args)

expect { PDK::Util::Bundler.ensure_bundle! }.to output(/checking for missing/i).to_stderr
end
end
end
end