Chef-16 git provider fixes

The git provider now no longer checks out to a "deploy" branch by
default and now checks out to the branch (with a remote upstream) or
else checks out to a detatched head.

The prior behavior can be restored by using "checkout branch 'deploy'".

This also removes the SCM resource base class and replaces it with a
resource partial and does some internal reorganization.

It also introduces the RecipeDSLHelper for better functional tests and
cleans up the functional tests of the git provider.

Properties that were only ever implemented on the git provider were
removed from the subversion provider where they had been inherited from
the base class incorrectly.

Some additional env var handling was added to the subversion handler in
the process of sorting out the common properties, including HOME
handling for alternative users.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

8 files changed, 157 insertions, 103 deletions

diff --git a/lib/chef/dsl/declare_resource.rb b/lib/chef/dsl/declare_resource.rb
index f032a76bc7..02ad64c77a 100644
--- a/lib/chef/dsl/declare_resource.rb
+++ b/lib/chef/dsl/declare_resource.rb
@@ -267,10 +267,10 @@ class Chef
       #     action :delete
       #   end
       #
-      def declare_resource(type, name, created_at: nil, run_context: self.run_context, &resource_attrs_block)
+      def declare_resource(type, name, created_at: nil, run_context: self.run_context, enclosing_provider: nil, &resource_attrs_block)
         created_at ||= caller[0]
 
-        resource = build_resource(type, name, created_at: created_at, &resource_attrs_block)
+        resource = build_resource(type, name, created_at: created_at, enclosing_provider: enclosing_provider, &resource_attrs_block)
 
         run_context.resource_collection.insert(resource, resource_type: resource.declared_type, instance_name: resource.name)
         resource
@@ -297,13 +297,15 @@ class Chef
       #     action :delete
       #   end
       #
-      def build_resource(type, name, created_at: nil, run_context: self.run_context, &resource_attrs_block)
+      def build_resource(type, name, created_at: nil, run_context: self.run_context, enclosing_provider: nil, &resource_attrs_block)
         created_at ||= caller[0]
 
         # this needs to be lazy in order to avoid circular dependencies since ResourceBuilder
         # will requires the entire provider+resolver universe
         require_relative "../resource_builder" unless defined?(Chef::ResourceBuilder)
 
+        enclosing_provider ||= self if is_a?(Chef::Provider)
+
         Chef::ResourceBuilder.new(
           type:                type,
           name:                name,
@@ -312,7 +314,7 @@ class Chef
           run_context:         run_context,
           cookbook_name:       cookbook_name,
           recipe_name:         recipe_name,
-          enclosing_provider:  is_a?(Chef::Provider) ? self : nil
+          enclosing_provider:  enclosing_provider
         ).build(&resource_attrs_block)
       end
 
diff --git a/lib/chef/provider/git.rb b/lib/chef/provider/git.rb
index 9a86f26f60..c8b48f5602 100644
--- a/lib/chef/provider/git.rb
+++ b/lib/chef/provider/git.rb
@@ -61,7 +61,7 @@ class Chef
           a.assertion { ::File.directory?(dirname) }
           a.whyrun("Directory #{dirname} does not exist, this run will fail unless it has been previously created. Assuming it would have been created.")
           a.failure_message(Chef::Exceptions::MissingParentDirectory,
-            "Cannot clone #{new_resource} to #{cwd}, the enclosing directory #{dirname} does not exist")
+                            "Cannot clone #{new_resource} to #{cwd}, the enclosing directory #{dirname} does not exist")
         end
 
         requirements.assert(:all_actions) do |a|
@@ -182,13 +182,24 @@ class Chef
       end
 
       def checkout
-        sha_ref = target_revision
-
-        converge_by("checkout ref #{sha_ref} branch #{new_resource.revision}") do
+        converge_by("checkout ref #{target_revision} branch #{new_resource.revision}") do
           # checkout into a local branch rather than a detached HEAD
-          git("branch", "-f", new_resource.checkout_branch, sha_ref, cwd: cwd)
-          git("checkout", new_resource.checkout_branch, cwd: cwd)
-          logger.info "#{new_resource} checked out branch: #{new_resource.revision} onto: #{new_resource.checkout_branch} reference: #{sha_ref}"
+          if new_resource.checkout_branch
+            # check out to a local branch
+            git("branch", "-f", new_resource.checkout_branch, target_revision, cwd: cwd)
+            git("checkout", new_resource.checkout_branch, cwd: cwd)
+            logger.info "#{new_resource} checked out branch: #{new_resource.revision} onto: #{new_resource.checkout_branch} reference: #{target_revision}"
+          elsif sha_hash?(new_resource.revision) || !is_branch?
+            # detached head
+            git("checkout", target_revision, cwd: cwd)
+            logger.info "#{new_resource} checked out reference: #{target_revision}"
+          else
+            # need a branch with a tracking branch
+            git("branch", "-f", new_resource.revision, target_revision, cwd: cwd)
+            git("checkout", new_resource.revision, cwd: cwd)
+            git("branch", "-u", "#{new_resource.remote}/#{new_resource.revision}", cwd: cwd)
+            logger.info "#{new_resource} checked out branch: #{new_resource.revision} reference: #{target_revision}"
+          end
         end
       end
 
@@ -211,7 +222,19 @@ class Chef
           logger.trace "Fetching updates from #{new_resource.remote} and resetting to revision #{target_revision}"
           git("fetch", "--prune", new_resource.remote, cwd: cwd)
           git("fetch", new_resource.remote, "--tags", cwd: cwd)
-          git("reset", "--hard", target_revision, cwd: cwd)
+          if new_resource.checkout_branch
+            # check out to a local branch
+            git("branch", "-f", new_resource.checkout_branch, target_revision, cwd: cwd)
+            git("checkout", new_resource.checkout_branch, cwd: cwd)
+          elsif sha_hash?(new_resource.revision) || is_tag?
+            # detached head
+            git("reset", "--hard", target_revision, cwd: cwd)
+          else
+            # need a branch with a tracking branch
+            git("branch", "-f", new_resource.revision, target_revision, cwd: cwd)
+            git("checkout", new_resource.revision, cwd: cwd)
+            git("branch", "-u", "#{new_resource.remote}/#{new_resource.revision}", cwd: cwd)
+          end
         end
       end
 
@@ -287,9 +310,18 @@ class Chef
 
       def find_revision(refs, revision, suffix = "")
         found = refs_search(refs, rev_match_pattern("refs/tags/", revision) + suffix)
-        found = refs_search(refs, rev_match_pattern("refs/heads/", revision) + suffix) if found.empty?
-        found = refs_search(refs, revision + suffix) if found.empty?
-        found
+        if !found.empty?
+          @is_tag = true
+          found
+        else
+          found = refs_search(refs, rev_match_pattern("refs/heads/", revision) + suffix)
+          if !found.empty?
+            @is_branch = true
+            found
+          else
+            refs_search(refs, revision + suffix)
+          end
+        end
       end
 
       def rev_match_pattern(prefix, revision)
@@ -320,6 +352,14 @@ class Chef
 
       private
 
+      def is_branch?
+        !!@is_branch
+      end
+
+      def is_tag?
+        !!@is_tag
+      end
+
       def run_options(run_opts = {})
         env = {}
         if new_resource.user
@@ -341,7 +381,7 @@ class Chef
       def git(*args, **run_opts)
         git_command = ["git", args].compact.join(" ")
         logger.trace "running #{git_command}"
-        shell_out!(git_command, run_options(run_opts))
+        shell_out!(git_command, **run_options(run_opts))
       end
 
       def sha_hash?(string)
diff --git a/lib/chef/provider/subversion.rb b/lib/chef/provider/subversion.rb
index 486bb38e5d..270f7457fa 100644
--- a/lib/chef/provider/subversion.rb
+++ b/lib/chef/provider/subversion.rb
@@ -149,9 +149,15 @@ class Chef
       end
 
       def run_options(run_opts = {})
-        run_opts[:user] = new_resource.user if new_resource.user
+        env = {}
+        if new_resource.user
+          run_opts[:user] = new_resource.user
+          env["HOME"] = get_homedir(new_resource.user)
+        end
         run_opts[:group] = new_resource.group if new_resource.group
         run_opts[:timeout] = new_resource.timeout if new_resource.timeout
+        env.merge!(new_resource.environment) if new_resource.environment
+        run_opts[:environment] = env unless env.empty?
         run_opts
       end
 
@@ -225,6 +231,20 @@ class Chef
           raise Chef::Exceptions::MissingParentDirectory, msg
         end
       end
+
+      # Returns the home directory of the user
+      # @param [String] user must be a string.
+      # @return [String] the home directory of the user.
+      #
+      def get_homedir(user)
+        require "etc" unless defined?(Etc)
+        case user
+        when Integer
+          Etc.getpwuid(user).dir
+        else
+          Etc.getpwnam(user.to_s).dir
+        end
+      end
     end
   end
 end
diff --git a/lib/chef/resource/scm.rb b/lib/chef/resource/scm.rb
deleted file mode 100644
index a09168dc11..0000000000
--- a/lib/chef/resource/scm.rb
+++ /dev/null
@@ -1,78 +0,0 @@
-#
-# Author:: Daniel DeLeo (<dan@kallistec.com>)
-# Copyright:: Copyright (c) Chef Software Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require_relative "../resource"
-
-class Chef
-  class Resource
-    class Scm < Chef::Resource
-      unified_mode true
-
-      default_action :sync
-      allowed_actions :checkout, :export, :sync, :diff, :log
-
-      property :destination, String,
-        description: "The location path to which the source is to be cloned, checked out, or exported. Default value: the name of the resource block.",
-        name_property: true
-
-      property :repository, String
-
-      property :revision, String,
-        description: "The revision to checkout.",
-        default: "HEAD"
-
-      property :user, [String, Integer],
-        description: "The system user that is responsible for the checked-out code."
-
-      property :group, [String, Integer],
-        description: "The system group that is responsible for the checked-out code."
-
-      # Capistrano and git-deploy use ``shallow clone''
-      property :depth, Integer,
-        description: "The number of past revisions to be included in the git shallow clone. Unless specified the default behavior will do a full clone."
-
-      property :enable_submodules, [TrueClass, FalseClass],
-        description: "Perform a sub-module initialization and update.",
-        default: false
-
-      property :enable_checkout, [TrueClass, FalseClass],
-        description: "Check out a repo from master. Set to false when using the checkout_branch attribute to prevent the git resource from attempting to check out master from master.",
-        default: true
-
-      property :remote, String,
-        default: "origin"
-
-      property :ssh_wrapper, String,
-        desired_state: false
-
-      property :timeout, Integer,
-        description: "The amount of time (in seconds) to wait before timing out.",
-        desired_state: false
-
-      property :checkout_branch, String,
-        description: "Do a one-time checkout **or** use when a branch in the upstream repository is named 'deploy'. To prevent the resource from attempting to check out master from master, set 'enable_checkout' to 'false' when using the 'checkout_branch' property.",
-        default: "deploy"
-
-      property :environment, [Hash, nil],
-        description: "A Hash of environment variables in the form of ({'ENV_VARIABLE' => 'VALUE'}).",
-        default: nil
-
-      alias :env :environment
-    end
-  end
-end
diff --git a/lib/chef/resource/scm/_scm.rb b/lib/chef/resource/scm/_scm.rb
new file mode 100644
index 0000000000..da1e100fcf
--- /dev/null
+++ b/lib/chef/resource/scm/_scm.rb
@@ -0,0 +1,48 @@
+#
+# Author:: Daniel DeLeo (<dan@kallistec.com>)
+# Copyright:: Copyright (c) Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+unified_mode true
+
+default_action :sync
+allowed_actions :checkout, :export, :sync, :diff, :log
+
+property :destination, String,
+  description: "The location path to which the source is to be cloned, checked out, or exported. Default value: the name of the resource block.",
+  name_property: true
+
+property :repository, String
+
+property :revision, String,
+  description: "The revision to checkout.",
+  default: "HEAD"
+
+property :user, [String, Integer],
+  description: "The system user that is responsible for the checked-out code."
+
+property :group, [String, Integer],
+  description: "The system group that is responsible for the checked-out code."
+
+property :timeout, Integer,
+  description: "The amount of time (in seconds) to wait before timing out.",
+  desired_state: false
+
+property :environment, [Hash, nil],
+  description: "A Hash of environment variables in the form of ({'ENV_VARIABLE' => 'VALUE'}).",
+  default: nil
+
+alias :env :environment
diff --git a/lib/chef/resource/git.rb b/lib/chef/resource/scm/git.rb
index 2ae68b94af..f2d5b6980a 100644
--- a/lib/chef/resource/git.rb
+++ b/lib/chef/resource/scm/git.rb
@@ -16,11 +16,13 @@
 # limitations under the License.
 #
 
-require_relative "scm"
+require_relative "../../resource"
 
 class Chef
   class Resource
-    class Git < Chef::Resource::Scm
+    class Git < Chef::Resource
+      use "scm"
+
       unified_mode true
 
       provides :git
@@ -31,6 +33,26 @@ class Chef
         description: "A Hash of additional remotes that are added to the git repository configuration.",
         default: lazy { {} }
 
+      property :depth, Integer,
+        description: "The number of past revisions to be included in the git shallow clone. Unless specified the default behavior will do a full clone."
+
+      property :enable_submodules, [TrueClass, FalseClass],
+        description: "Perform a sub-module initialization and update.",
+        default: false
+
+      property :enable_checkout, [TrueClass, FalseClass],
+        description: "Check out a repo from master. Set to false when using the checkout_branch attribute to prevent the git resource from attempting to check out master from master.",
+        default: true
+
+      property :remote, String,
+        default: "origin"
+
+      property :ssh_wrapper, String,
+        desired_state: false
+
+      property :checkout_branch, String,
+        description: "Set this to use a local branch to avoid checking SHAs or tags to a detatched head state."
+
       alias :branch :revision
       alias :reference :revision
       alias :repo :repository
diff --git a/lib/chef/resource/subversion.rb b/lib/chef/resource/scm/subversion.rb
index c1edabfd8a..0f615ba8db 100644
--- a/lib/chef/resource/subversion.rb
+++ b/lib/chef/resource/scm/subversion.rb
@@ -17,12 +17,13 @@
 # limitations under the License.
 #
 
-require_relative "scm"
-require_relative "../dist"
+require_relative "../../dist"
 
 class Chef
   class Resource
-    class Subversion < Chef::Resource::Scm
+    class Subversion < Chef::Resource
+      use "scm"
+
       unified_mode true
 
       provides :subversion
diff --git a/lib/chef/resources.rb b/lib/chef/resources.rb
index f997d43a39..6a87960972 100644
--- a/lib/chef/resources.rb
+++ b/lib/chef/resources.rb
@@ -53,7 +53,7 @@ require_relative "resource/file"
 require_relative "resource/freebsd_package"
 require_relative "resource/ips_package"
 require_relative "resource/gem_package"
-require_relative "resource/git"
+require_relative "resource/scm/git"
 require_relative "resource/group"
 require_relative "resource/http_request"
 require_relative "resource/hostname"
@@ -108,7 +108,6 @@ require_relative "resource/solaris_package"
 require_relative "resource/route"
 require_relative "resource/ruby"
 require_relative "resource/ruby_block"
-require_relative "resource/scm"
 require_relative "resource/script"
 require_relative "resource/service"
 require_relative "resource/sudo"
@@ -117,7 +116,7 @@ require_relative "resource/swap_file"
 require_relative "resource/systemd_unit"
 require_relative "resource/ssh_known_hosts_entry"
 require_relative "resource/windows_service"
-require_relative "resource/subversion"
+require_relative "resource/scm/subversion"
 require_relative "resource/smartos_package"
 require_relative "resource/template"
 require_relative "resource/user"