Merge pull request #3569 from andersonmills/improvement/master/PUP-3254-Remove_Modulefile_functionality

adreyer · adreyer · commit bf6978a0a411 · 2015-02-06T11:44:58.000-08:00
(PUP-3254) Removing Modulefile functionality
diff --git a/acceptance/tests/modules/changes/missing_checksums_json.rb b/acceptance/tests/modules/changes/missing_checksums_json.rb
@@ -0,0 +1,37 @@
+test_name 'puppet module changes (on a module which is missing checksums.json)'
+
+step 'Setup'
+
+stub_forge_on(master)
+testdir = master.tmpdir('module_changes_on_invalid_checksums')
+
+apply_manifest_on master, %Q{
+  file { '#{testdir}/nginx': ensure => directory;
+         '#{testdir}/nginx/metadata.json': ensure => present,
+           content => '
+{
+  "name": "puppetlabs-nginx",
+  "version": "0.0.1",
+  "author": "Puppet Labs",
+  "summary": "Nginx Module",
+  "license": "Apache Version 2.0",
+  "source": "git://github.com/puppetlabs/puppetlabs-nginx.git",
+  "project_page": "http://github.com/puppetlabs/puppetlabs-nginx",
+  "issues_url": "http://github.com/puppetlabs/puppetlabs-nginx",
+  "dependencies": [
+    {"name":"puppetlabs-stdlub","version_requirement":">= 1.0.0"}
+  ]
+}'
+  }
+}
+
+step 'Run module changes on a module which is missing checksums.json'
+on( master, puppet("module changes #{testdir}/nginx"),
+    :acceptable_exit_codes => [1] ) do
+
+  pattern = Regexp.new([
+%Q{.*Error: No file containing checksums found.*},
+%Q{.*Error: Try 'puppet help module changes' for usage.*},
+  ].join("\n"), Regexp::MULTILINE)
+  assert_match(pattern, result.stderr)
+end
diff --git a/acceptance/tests/modules/changes/missing_metadata_json.rb b/acceptance/tests/modules/changes/missing_metadata_json.rb
@@ -6,16 +6,15 @@
 testdir = master.tmpdir('module_changes_on_invalid_metadata')
 
 apply_manifest_on master, %Q{
-  file { '#{testdir}/nginx': ensure => directory;
-         '#{testdir}/nginx/Modulefile': ensure => present }
+  file { '#{testdir}/nginx': ensure => directory }
 }
 
-step 'Run module changes on a module witch is missing metadata.json'
+step 'Run module changes on a module which is missing metadata.json'
 on( master, puppet("module changes #{testdir}/nginx"),
     :acceptable_exit_codes => [1] ) do
 
   pattern = Regexp.new([
-%Q{.*Error: No file containing checksums found.*},
+%Q{.*Error: Could not find a valid module at.*},
 %Q{.*Error: Try 'puppet help module changes' for usage.*},
   ].join("\n"), Regexp::MULTILINE)
   assert_match(pattern, result.stderr)
diff --git a/lib/puppet/face/module/build.rb b/lib/puppet/face/module/build.rb
@@ -5,9 +5,9 @@
       Prepares a local module for release on the Puppet Forge by building a
       ready-to-upload archive file.
 
-      This action uses the Modulefile in the module directory to set metadata
-      used by the Forge. See <http://links.puppetlabs.com/modulefile> for more
-      about writing modulefiles.
+      This action uses the metadata.json file in the module directory to set metadata
+      used by the Forge. See <https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html> for more
+      about writing metadata.json files.
 
       After being built, the release archive file can be found in the module's
       `pkg` directory.
@@ -43,11 +43,11 @@
         pwd = Dir.pwd
         module_path = Puppet::ModuleTool.find_module_root(pwd)
         if module_path.nil?
-          raise "Unable to find metadata.json or Modulefile in module root #{pwd} or parent directories. See <http://links.puppetlabs.com/modulefile> for required file format."
+          raise "Unable to find metadata.json in module root #{pwd} or parent directories. See <https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html> for required file format."
         end
       else
         unless Puppet::ModuleTool.is_module_root?(module_path)
-          raise "Unable to find metadata.json or Modulefile in module root #{module_path}. See <http://links.puppetlabs.com/modulefile> for required file format."
+          raise "Unable to find metadata.json in module root #{module_path} or parent directories. See <https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html> for required file format."
         end
       end
 
diff --git a/lib/puppet/face/module/generate.rb b/lib/puppet/face/module/generate.rb
@@ -107,9 +107,9 @@
       begin
         # A default dependency for all newly generated modules is being
         # introduced as a substitute for the comments we used to include in the
-        # Modulefile. While introducing a default dependency is less than
-        # perfectly desirable, the cost is low, and the syntax is obtuse enough
-        # to justify its inclusion.
+        # previous module data specifications. While introducing a default
+        # dependency is less than perfectly desirable, the cost is low, and the
+        # syntax is obtuse enough to justify its inclusion.
         metadata = Puppet::ModuleTool::Metadata.new.update(
           'name' => name,
           'version' => '0.1.0',
diff --git a/lib/puppet/module_tool.rb b/lib/puppet/module_tool.rb
@@ -39,8 +39,7 @@ def self.username_and_modname_from(full_module_name)
     end
 
     # Find the module root when given a path by checking each directory up from
-    # its current location until it finds one that contains a file called
-    # 'Modulefile'.
+    # its current location until it finds one that satisfies is_module_root?
     #
     # @param path [Pathname, String] path to start from
     # @return [Pathname, nil] the root path of the module directory or nil if
@@ -56,14 +55,14 @@ def self.find_module_root(path)
     end
 
     # Analyse path to see if it is a module root directory by detecting a
-    # file named 'metadata.json' or 'Modulefile' in the directory.
+    # file named 'metadata.json'
     #
     # @param path [Pathname, String] path to analyse
     # @return [Boolean] true if the path is a module root, false otherwise
     def self.is_module_root?(path)
       path = Pathname.new(path) if path.class == String
 
-      FileTest.file?(path + 'metadata.json') || FileTest.file?(path + 'Modulefile')
+      FileTest.file?(path + 'metadata.json')
     end
 
     # Builds a formatted tree from a list of node hashes containing +:text+
@@ -189,6 +188,5 @@ def self.parse_module_dependency(where, dep)
 require 'puppet/module_tool/contents_description'
 require 'puppet/module_tool/dependency'
 require 'puppet/module_tool/metadata'
-require 'puppet/module_tool/modulefile'
 require 'puppet/forge/cache'
 require 'puppet/forge'
diff --git a/lib/puppet/module_tool/applications/application.rb b/lib/puppet/module_tool/applications/application.rb
@@ -41,10 +41,9 @@ def metadata(require_metadata = false)
         end
 
         if require_metadata && !Puppet::ModuleTool.is_module_root?(@path)
-          raise ArgumentError, "Unable to find metadata.json or Modulefile in module root at #{@path} See http://links.puppetlabs.com/modulefile for required file format."
+          raise ArgumentError, "Unable to find metadata.json in module root at #{@path} See https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html for required file format."
         end
 
-        modulefile_path = File.join(@path, 'Modulefile')
         metadata_path   = File.join(@path, 'metadata.json')
 
         if File.file?(metadata_path)
@@ -57,14 +56,8 @@ def metadata(require_metadata = false)
           end
         end
 
-        if File.file?(modulefile_path)
-          if File.file?(metadata_path)
-            Puppet.warning "Modulefile is deprecated. Merging your Modulefile and metadata.json."
-          else
-            Puppet.warning "Modulefile is deprecated. Building metadata.json from Modulefile."
-          end
-
-          Puppet::ModuleTool::ModulefileReader.evaluate(@metadata, modulefile_path)
+        if File.file?(File.join(@path, 'Modulefile'))
+          Puppet.warning "A Modulefile was found in the root directory of the module. This file will be ignored and can safely be removed."
         end
 
         return @metadata
diff --git a/lib/puppet/module_tool/metadata.rb b/lib/puppet/module_tool/metadata.rb
@@ -66,7 +66,7 @@ def add_dependency(name, version_requirement=nil, repository=nil)
       validate_version_range(version_requirement) if version_requirement
 
       if dup = @data['dependencies'].find { |d| d.full_module_name == name && d.version_requirement != version_requirement }
-        raise ArgumentError, "Dependency conflict for #{full_module_name}: Dependency #{name} was given conflicting version requirements #{version_requirement} and #{dup.version_requirement}. Verify that there are no duplicates in the metadata.json or the Modulefile."
+        raise ArgumentError, "Dependency conflict for #{full_module_name}: Dependency #{name} was given conflicting version requirements #{version_requirement} and #{dup.version_requirement}. Verify that there are no duplicates in the metadata.json."
       end
 
       dep = Dependency.new(name, version_requirement, repository)
diff --git a/lib/puppet/module_tool/modulefile.rb b/lib/puppet/module_tool/modulefile.rb
diff --git a/spec/unit/face/module/build_spec.rb b/spec/unit/face/module/build_spec.rb
@@ -25,7 +25,7 @@
     it "if current directory or parents contain no module root, should return exception" do
       Dir.expects(:pwd).returns('/a/b/c')
       Puppet::ModuleTool.expects(:find_module_root).returns(nil)
-      expect { subject.build }.to raise_error RuntimeError, "Unable to find metadata.json or Modulefile in module root /a/b/c or parent directories. See <http://links.puppetlabs.com/modulefile> for required file format."
+      expect { subject.build }.to raise_error RuntimeError, "Unable to find metadata.json in module root /a/b/c or parent directories. See <https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html> for required file format."
     end
   end
 
@@ -39,7 +39,7 @@
 
     it "if path is not a module root should raise exception" do
       Puppet::ModuleTool.expects(:is_module_root?).with('/a/b/c').returns(false)
-      expect { subject.build('/a/b/c') }.to raise_error RuntimeError, "Unable to find metadata.json or Modulefile in module root /a/b/c. See <http://links.puppetlabs.com/modulefile> for required file format."
+      expect { subject.build('/a/b/c') }.to raise_error RuntimeError, "Unable to find metadata.json in module root /a/b/c or parent directories. See <https://docs.puppetlabs.com/puppet/latest/reference/modules_publishing.html> for required file format."
     end
   end
 
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
@@ -626,19 +626,7 @@
 
   it "should not have metadata if has a metadata file and its data is empty" do
     Puppet::FileSystem.expects(:exist?).with(@module.metadata_file).returns true
-    File.stubs(:read).with(@module.metadata_file).returns "/*
-+-----------------------------------------------------------------------+
-|                                                                       |
-|                    ==> DO NOT EDIT THIS FILE! <==                     |
-|                                                                       |
-|   You should edit the `Modulefile` and run `puppet-module build`      |
-|   to generate the `metadata.json` file for your releases.             |
-|                                                                       |
-+-----------------------------------------------------------------------+
-*/
-
-{}"
-
+    File.stubs(:read).with(@module.metadata_file).returns "This is some invalid json.\n"
     expect(@module).not_to be_has_metadata
   end
 
diff --git a/spec/unit/module_tool/applications/builder_spec.rb b/spec/unit/module_tool/applications/builder_spec.rb
@@ -394,23 +394,4 @@ def create_symlink_gitignore_file
 
     it_behaves_like "a packagable module"
   end
-
-  context 'with Modulefile' do
-    before :each do
-      File.open(File.join(path, 'Modulefile'), 'w') do |f|
-        f.write <<-MODULEFILE
-name    '#{module_name}'
-version '#{version}'
-source 'http://github.com/testing/#{module_name}'
-author 'testing'
-license 'Apache License Version 2.0'
-summary 'Puppet testing module'
-description 'This module can be used for basic testing'
-project_page 'http://github.com/testing/#{module_name}'
-MODULEFILE
-      end
-    end
-
-    it_behaves_like "a packagable module"
-  end
 end
diff --git a/spec/unit/module_tool_spec.rb b/spec/unit/module_tool_spec.rb
@@ -6,27 +6,16 @@
 
 describe Puppet::ModuleTool do
   describe '.is_module_root?' do
-    it 'should return true if directory has a Modulefile file' do
-      FileTest.expects(:file?).with(responds_with(:to_s, '/a/b/c/metadata.json')).
-        returns(false)
-      FileTest.expects(:file?).with(responds_with(:to_s, '/a/b/c/Modulefile')).
-        returns(true)
-
-      expect(subject.is_module_root?(Pathname.new('/a/b/c'))).to be_truthy
-    end
-
     it 'should return true if directory has a metadata.json file' do
       FileTest.expects(:file?).with(responds_with(:to_s, '/a/b/c/metadata.json')).
         returns(true)
 
       expect(subject.is_module_root?(Pathname.new('/a/b/c'))).to be_truthy
     end
 
-    it 'should return false if directory does not have a metadata.json or a Modulefile file' do
+    it 'should return false if directory does not have a metadata.json file' do
       FileTest.expects(:file?).with(responds_with(:to_s, '/a/b/c/metadata.json')).
         returns(false)
-      FileTest.expects(:file?).with(responds_with(:to_s, '/a/b/c/Modulefile')).
-        returns(false)
 
       expect(subject.is_module_root?(Pathname.new('/a/b/c'))).to be_falsey
     end
