Please review pull request #503: Ticket/2.7.x/12256 module tool core changes opened by (mmrobins)
Description:
These are the changes needed for the module tool that affect areas of code in other parts of Puppet.
- Opened: Wed Feb 15 17:58:06 UTC 2012
- Based on: puppetlabs:2.7.x (b9655fea47c5eefaa6f4768b750e634aeb063aca)
- Requested merge: mmrobins:ticket/2.7.x/12256-module_tool_core_changes (05c6857fa294aedc80a321792fb10ec38d442ad5)
Diff follows:
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index 4282fc6..b68cd87 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -1,5 +1,6 @@
require 'puppet/util/logging'
require 'semver'
+require 'puppet/module_tool/applications'
# Support for modules
class Puppet::Module
@@ -31,7 +32,7 @@ def self.find(modname, environment = nil)
attr_reader :name, :environment
attr_writer :environment
- attr_accessor :dependencies
+ attr_accessor :dependencies, :forge_name
attr_accessor :source, :author, :version, :license, :puppetversion, :summary, :description, :project_page
def has_metadata?
@@ -40,6 +41,8 @@ def has_metadata?
return false unless FileTest.exist?(metadata_file)
metadata = PSON.parse File.read(metadata_file)
+
+
return metadata.is_a?(Hash) && !metadata.keys.empty?
end
@@ -108,6 +111,8 @@ def license_file
def load_metadata
data = "" File.read(metadata_file)
+ @forge_name = data['name'].gsub('-', '/') if data['name']
+
[:source, :author, :version, :license, :puppetversion, :dependencies].each do |attr|
unless value = data[attr.to_s]
unless attr == :puppetversion
@@ -155,6 +160,26 @@ def to_s
result
end
+ def dependencies_as_modules
+ dependent_modules = []
+ dependencies and dependencies.each do |dep|
+ author, dep_name = dep["name"].split('/')
+ found_module = environment.module(dep_name)
+ dependent_modules << found_module if found_module
+ end
+
+ dependent_modules
+ end
+
+ def required_by
+ environment.module_requirements[self.forge_name] || {}
+ end
+
+ def has_local_changes?
+ changes = Puppet::Module::Tool::Applications::Checksummer.run(path)
+ !changes.empty?
+ end
+
def unmet_dependencies
return [] unless dependencies
diff --git a/lib/puppet/module_tool/applications.rb b/lib/puppet/module_tool/applications.rb
index 24bcbe2..d5eb7f5 100644
--- a/lib/puppet/module_tool/applications.rb
+++ b/lib/puppet/module_tool/applications.rb
@@ -1,13 +1,17 @@
-module Puppet::Module::Tool
- module Applications
- require 'puppet/module_tool/applications/application'
- require 'puppet/module_tool/applications/builder'
- require 'puppet/module_tool/applications/checksummer'
- require 'puppet/module_tool/applications/cleaner'
- require 'puppet/module_tool/applications/generator'
- require 'puppet/module_tool/applications/installer'
- require 'puppet/module_tool/applications/searcher'
- require 'puppet/module_tool/applications/unpacker'
- require 'puppet/module_tool/applications/uninstaller'
+require 'puppet/module'
+
+class Puppet::Module
+ module Tool
+ module Applications
+ require 'puppet/module_tool/applications/application'
+ require 'puppet/module_tool/applications/builder'
+ require 'puppet/module_tool/applications/checksummer'
+ require 'puppet/module_tool/applications/cleaner'
+ require 'puppet/module_tool/applications/generator'
+ require 'puppet/module_tool/applications/installer'
+ require 'puppet/module_tool/applications/searcher'
+ require 'puppet/module_tool/applications/unpacker'
+ require 'puppet/module_tool/applications/uninstaller'
+ end
end
end
diff --git a/lib/puppet/module_tool/applications/application.rb b/lib/puppet/module_tool/applications/application.rb
index 43d5c04..fd398da 100644
--- a/lib/puppet/module_tool/applications/application.rb
+++ b/lib/puppet/module_tool/applications/application.rb
@@ -1,10 +1,11 @@
require 'net/http'
require 'semver'
+require 'puppet/module_tool/utils/interrogation'
module Puppet::Module::Tool
module Applications
class Application
- include Utils::Interrogation
+ include Puppet::Module::Tool::Utils::Interrogation
def self.run(*args)
new(*args).run
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 06be16d..2a64f01 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -93,6 +93,14 @@ def module(name)
mod
end
+ def module_by_forge_name(forge_name)
+ author, modname = forge_name.split('/')
+ found_mod = self.module(modname)
+ found_mod and found_mod.forge_name == forge_name ?
+ found_mod :
+ nil
+ end
+
# Cache the modulepath, so that we aren't searching through
# all known directories all the time.
cached_attr(:modulepath, Puppet[:filetimeout]) do
@@ -128,6 +136,24 @@ def modules_by_path
modules_by_path
end
+ def module_requirements
+ deps = {}
+ modules.each do |mod|
+ next unless mod.forge_name
+ deps[mod.forge_name] ||= []
+ mod.dependencies and mod.dependencies.sort_by {|mod_dep| mod_dep['name']}.each do |mod_dep|
+ deps[mod_dep['name']] ||= []
+ dep_details = {
+ 'name' => mod.forge_name,
+ 'version' => mod.version,
+ 'version_requirement' => mod_dep['version_requirement']
+ }
+ deps[mod_dep['name']] << dep_details
+ end
+ end
+ deps
+ end
+
def to_s
name.to_s
end
diff --git a/spec/lib/puppet_spec/modules.rb b/spec/lib/puppet_spec/modules.rb
new file mode 100644
index 0000000..156e466
--- /dev/null
+++ b/spec/lib/puppet_spec/modules.rb
@@ -0,0 +1,26 @@
+module PuppetSpec::Modules
+ class << self
+ def create(name, dir, options = {})
+ module_dir = File.join(dir, name)
+ FileUtils.mkdir_p(module_dir)
+
+ environment = Puppet::Node::Environment.new(options[:environment])
+
+ if metadata = options[:metadata]
+ metadata[:source] ||= 'github'
+ metadata[:author] ||= 'puppetlabs'
+ metadata[:version] ||= '9.9.9'
+ metadata[:license] ||= 'to kill'
+ metadata[:dependencies] ||= []
+
+ metadata[:name] = "#{metadata[:author]}/#{name}"
+
+ File.open(File.join(module_dir, 'metadata.json'), 'w') do |f|
+ f.write(metadata.to_pson)
+ end
+ end
+
+ Puppet::Module.new(name, :environment => environment, :path => module_dir)
+ end
+ end
+end
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
index 695f648..657ca40 100755
--- a/spec/unit/module_spec.rb
+++ b/spec/unit/module_spec.rb
@@ -1,6 +1,8 @@
#!/usr/bin/env rspec
require 'spec_helper'
require 'puppet_spec/files'
+require 'puppet_spec/modules'
+require 'puppet/module_tool/checksums'
describe Puppet::Module do
include PuppetSpec::Files
@@ -532,13 +534,14 @@
end
describe Puppet::Module do
+ include PuppetSpec::Files
before do
- Puppet::Module.any_instance.stubs(:path).returns "/my/mod/path"
- @module = Puppet::Module.new("foo")
+ @modpath = tmpdir('modpath')
+ @module = PuppetSpec::Modules.create('mymod', @modpath)
end
it "should use 'License' in its current path as its metadata file" do
- @module.license_file.should == "/my/mod/path/License"
+ @module.license_file.should == "#{@modpath}/mymod/License"
end
it "should return nil as its license file when the module has no path" do
@@ -547,13 +550,13 @@
end
it "should cache the license file" do
- Puppet::Module.any_instance.expects(:path).once.returns nil
- mod = Puppet::Module.new("foo")
- mod.license_file.should == mod.license_file
+ @module.expects(:path).once.returns nil
+ @module.license_file
+ @module.license_file
end
it "should use 'metadata.json' in its current path as its metadata file" do
- @module.metadata_file.should == "/my/mod/path/metadata.json"
+ @module.metadata_file.should == "#{@modpath}/mymod/metadata.json"
end
it "should return nil as its metadata file when the module has no path" do
@@ -657,4 +660,72 @@
it "should fail if the discovered name is different than the metadata name"
end
+
+ it "should be able to tell if there are local changes" do
+ modpath = tmpdir('modpath')
+ foo_checksum = 'acbd18db4cc2f85cedef654fccc4a4d8'
+ checksummed_module = PuppetSpec::Modules.create(
+ 'changed',
+ modpath,
+ :metadata => {
+ :checksums => {
+ "foo" => foo_checksum,
+ }
+ }
+ )
+
+ foo_path = Pathname.new(File.join(checksummed_module.path, 'foo'))
+
+ IO.binwrite(foo_path, 'notfoo')
+ Puppet::Module::Tool::Checksums.new(foo_path).checksum(foo_path).should_not == foo_checksum
+ checksummed_module.has_local_changes?.should be_true
+
+ IO.binwrite(foo_path, 'foo')
+
+ Puppet::Module::Tool::Checksums.new(foo_path).checksum(foo_path).should == foo_checksum
+ checksummed_module.has_local_changes?.should be_false
+ end
+
+ it "should know what other modules require it" do
+ Puppet.settings[:modulepath] = @modpath
+ dependable = PuppetSpec::Modules.create(
+ 'dependable',
+ @modpath,
+ :metadata => {:author => 'puppetlabs'}
+ )
+ PuppetSpec::Modules.create(
+ 'needy',
+ @modpath,
+ :metadata => {
+ :author => 'beggar',
+ :dependencies => [{
+ "version_requirement" => ">= 2.2.0",
+ "name" => "puppetlabs/dependable"
+ }]
+ }
+ )
+ PuppetSpec::Modules.create(
+ 'wantit',
+ @modpath,
+ :metadata => {
+ :author => 'spoiled',
+ :dependencies => [{
+ "version_requirement" => "< 5.0.0",
+ "name" => "puppetlabs/dependable"
+ }]
+ }
+ )
+ dependable.required_by.should =~ [
+ {
+ "name" => "beggar/needy",
+ "version" => "9.9.9",
+ "version_requirement" => ">= 2.2.0"
+ },
+ {
+ "name" => "spoiled/wantit",
+ "version" => "9.9.9",
+ "version_requirement" => "< 5.0.0"
+ }
+ ]
+ end
end
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index d5d3068..4adc5a6 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -5,6 +5,7 @@
require 'puppet/node/environment'
require 'puppet/util/execution'
+require 'puppet_spec/modules'
describe Puppet::Node::Environment do
let(:env) { Puppet::Node::Environment.new("testing") }
@@ -124,8 +125,10 @@
describe "when validating modulepath or manifestdir directories" do
before :each do
- @path_one = make_absolute('/one')
- @path_two = make_absolute('/two')
+ @path_one = tmpdir("path_one")
+ @path_two = tmpdir("path_one")
+ sep = File::PATH_SEPARATOR
+ Puppet[:modulepath] = "#{@path_one}#{sep}#{@path_two}"
end
it "should not return non-directories" do
@@ -167,15 +170,13 @@
end
it "should return nil if asked for a module that does not exist in its path" do
-
- mod = mock 'module'
- Puppet::Module.expects(:new).with("one", :environment => env).returns mod
- mod.expects(:exist?).returns false
+ modpath = tmpdir('modpath')
+ env.modulepath = [modpath]
env.module("one").should be_nil
end
- describe ".modules_by_path" do
+ describe "module data" do
before do
dir = tmpdir("deep_path")
@@ -187,75 +188,153 @@
FileUtils.mkdir_p(@second)
end
- it "should return an empty list if there are no modules" do
- env.modules_by_path.should == {
- @first => [],
- @second => []
- }
- end
-
- it "should include modules even if they exist in multiple dirs in the modulepath" do
- modpath1 = File.join(@first, "foo")
- FileUtils.mkdir_p(modpath1)
- modpath2 = File.join(@second, "foo")
- FileUtils.mkdir_p(modpath2)
-
- env.modules_by_path.should == {
- @first => [Puppet::Module.new('foo', :environment => env, :path => modpath1)],
- @second => [Puppet::Module.new('foo', :environment => env, :path => modpath2)]
- }
- end
- end
-
- describe ".modules" do
- it "should return an empty list if there are no modules" do
- env.modulepath = %w{/a /b}
- Dir.expects(:entries).with("/a").returns []
- Dir.expects(:entries).with("/b").returns []
-
- env.modules.should == []
- end
-
- it "should return a module named for every directory in each module path" do
- env.modulepath = %w{/a /b}
- Dir.expects(:entries).with("/a").returns %w{foo bar}
- Dir.expects(:entries).with("/b").returns %w{bee baz}
-
- env.modules.collect{|mod| mod.name}.sort.should == %w{foo bar bee baz}.sort
+ describe "#modules_by_path" do
+ it "should return an empty list if there are no modules" do
+ env.modules_by_path.should == {
+ @first => [],
+ @second => []
+ }
+ end
+
+ it "should include modules even if they exist in multiple dirs in the modulepath" do
+ modpath1 = File.join(@first, "foo")
+ FileUtils.mkdir_p(modpath1)
+ modpath2 = File.join(@second, "foo")
+ FileUtils.mkdir_p(modpath2)
+
+ env.modules_by_path.should == {
+ @first => [Puppet::Module.new('foo', :environment => env, :path => modpath1)],
+ @second => [Puppet::Module.new('foo', :environment => env, :path => modpath2)]
+ }
+ end
end
- it "should remove duplicates" do
- env.modulepath = %w{/a /b}
- Dir.expects(:entries).with("/a").returns %w{foo}
- Dir.expects(:entries).with("/b").returns %w{foo}
-
- env.modules.collect{|mod| mod.name}.sort.should == %w{foo}
+ describe "#module_requirements" do
+ it "should return a list of what modules depend on other modules" do
+ PuppetSpec::Modules.create(
+ 'foo',
+ @first,
+ :metadata => {
+ :author => 'puppetlabs',
+ :dependencies => [{ 'name' => 'puppetlabs/bar', "version_requirement" => ">= 1.0.0" }]
+ }
+ )
+ PuppetSpec::Modules.create(
+ 'bar',
+ @second,
+ :metadata => {
+ :author => 'puppetlabs',
+ :dependencies => [{ 'name' => 'puppetlabs/foo', "version_requirement" => "<= 2.0.0" }]
+ }
+ )
+ PuppetSpec::Modules.create(
+ 'baz',
+ @first,
+ :metadata => {
+ :author => 'puppetlabs',
+ :dependencies => [{ 'name' => 'puppetlabs/bar', "version_requirement" => "3.0.0" }]
+ }
+ )
+
+ env.module_requirements.should == {
+ 'puppetlabs/foo' => [
+ {
+ "name" => "puppetlabs/bar",
+ "version" => "9.9.9",
+ "version_requirement" => "<= 2.0.0"
+ }
+ ],
+ 'puppetlabs/bar' => [
+ {
+ "name" => "puppetlabs/baz",
+ "version" => "9.9.9",
+ "version_requirement" => "3.0.0"
+ },
+ {
+ "name" => "puppetlabs/foo",
+ "version" => "9.9.9",
+ "version_requirement" => ">= 1.0.0"
+ }
+ ],
+ 'puppetlabs/baz' => []
+ }
+ end
end
- it "should ignore invalid modules" do
- env.modulepath = %w{/a}
- Dir.expects(:entries).with("/a").returns %w{foo bar}
-
- Puppet::Module.expects(:new).with { |name, env| name == "foo" }.returns mock("foomod", :name => "foo")
- Puppet::Module.expects(:new).with { |name, env| name == "bar" }.raises( Puppet::Module::InvalidName, "name is invalid" )
-
- env.modules.collect{|mod| mod.name}.sort.should == %w{foo}
+ describe ".module_by_forge_name" do
+ it "should find modules by forge_name" do
+ mod = PuppetSpec::Modules.create(
+ 'baz',
+ @first,
+ :metadata => {:author => 'puppetlabs'},
+ :environment => env
+ )
+ env.module_by_forge_name('puppetlabs/baz').should == mod
+ end
+
+ it "should not find modules with same name by the wrong author" do
+ mod = PuppetSpec::Modules.create(
+ 'baz',
+ @first,
+ :metadata => {:author => 'sneakylabs'},
+ :environment => env
+ )
+ env.module_by_forge_name('puppetlabs/baz').should == nil
+ end
+
+ it "should return nil when the module can't be found" do
+ env.module_by_forge_name('ima/nothere').should be_nil
+ end
end
- it "should create modules with the correct environment" do
- env.modulepath = %w{/a}
- Dir.expects(:entries).with("/a").returns %w{foo}
+ describe ".modules" do
+ it "should return an empty list if there are no modules" do
+ env.modules.should == []
+ end
+
+ it "should return a module named for every directory in each module path" do
+ %w{foo bar}.each do |mod_name|
+ FileUtils.mkdir_p(File.join(@first, mod_name))
+ end
+ %w{bee baz}.each do |mod_name|
+ FileUtils.mkdir_p(File.join(@second, mod_name))
+ end
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo bar bee baz}.sort
+ end
+
+ it "should remove duplicates" do
+ FileUtils.mkdir_p(File.join(@first, 'foo'))
+ FileUtils.mkdir_p(File.join(@second, 'foo'))
+
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo}
+ end
+
+ it "should ignore modules with invalid names" do
+ FileUtils.mkdir_p(File.join(@first, 'foo'))
+ FileUtils.mkdir_p(File.join(@first, 'foo2'))
+ FileUtils.mkdir_p(File.join(@first, 'foo-bar'))
+ FileUtils.mkdir_p(File.join(@first, 'foo_bar'))
+ FileUtils.mkdir_p(File.join(@first, 'foo*bar'))
+ FileUtils.mkdir_p(File.join(@first, 'foo bar'))
+
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo foo-bar foo2 foo_bar}
+ end
+
+ it "should create modules with the correct environment" do
+ FileUtils.mkdir_p(File.join(@first, 'foo'))
+
+ env.modules.each {|mod| mod.environment.should == env }
+ end
- env.modules.each {|mod| mod.environment.should == env }
end
+ end
- it "should cache the module list" do
- env.modulepath = %w{/a}
- Dir.expects(:entries).once.with("/a").returns %w{foo}
+ it "should cache the module list" do
+ env.modulepath = %w{/a}
+ Dir.expects(:entries).once.with("/a").returns %w{foo}
- env.modules
- env.modules
- end
+ env.modules
+ env.modules
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
