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>

16 files changed, 500 insertions, 299 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"
diff --git a/spec/functional/resource/git_spec.rb b/spec/functional/resource/git_spec.rb
index 677cfe4bf0..6d4d889adc 100644
--- a/spec/functional/resource/git_spec.rb
+++ b/spec/functional/resource/git_spec.rb
@@ -17,30 +17,18 @@
 #
 
 require "spec_helper"
-require "chef/mixin/shell_out"
 require "tmpdir"
-require "shellwords"
 
 # Deploy relies heavily on symlinks, so it doesn't work on windows.
 describe Chef::Resource::Git, requires_git: true do
-  include Chef::Mixin::ShellOut
-  let(:file_cache_path) { Dir.mktmpdir }
+  include RecipeDSLHelper
+
   # Some versions of git complains when the deploy directory is
   # already created. Here we intentionally don't create the deploy
   # directory beforehand.
   let(:base_dir_path) { Dir.mktmpdir }
   let(:deploy_directory) { File.join(base_dir_path, make_tmpname("git_base")) }
 
-  let(:node) do
-    Chef::Node.new.tap do |n|
-      n.name "rspec-test"
-      n.consume_external_attrs(@ohai.data, {})
-    end
-  end
-
-  let(:event_dispatch) { Chef::EventDispatch::Dispatcher.new }
-  let(:run_context) { Chef::RunContext.new(node, {}, event_dispatch) }
-
   # These tests use git's bundle feature, which is a way to export an entire
   # git repo (or subset of commits) as a single file.
   #
@@ -64,33 +52,40 @@ describe Chef::Resource::Git, requires_git: true do
   let(:rev_testing) { "972d153654503bccec29f630c5dd369854a561e8" }
   let(:rev_head) { "d294fbfd05aa7709ad9a9b8ef6343b17d355bf5f" }
 
-  let(:git_user_config) do
-    <<~E
-      [user]
-        name = frodoTbaggins
-        email = frodo@shire.org
-    E
-  end
-
   before(:each) do
-    Chef::Log.level = :warn # silence git command live streams
-    @old_file_cache_path = Chef::Config[:file_cache_path]
-    shell_out!("git clone \"#{git_bundle_repo}\" example", cwd: origin_repo_dir)
-    File.open("#{origin_repo}/.git/config", "a+") { |f| f.print(git_user_config) }
-    Chef::Config[:file_cache_path] = file_cache_path
+    shell_out!("git", "clone", git_bundle_repo, "example", cwd: origin_repo_dir)
+    File.open("#{origin_repo}/.git/config", "a+") do |f|
+      f.print <<~EOF
+        [user]
+          name = frodoTbaggins
+          email = frodo@shire.org
+      EOF
+    end
   end
 
   after(:each) do
-    Chef::Config[:file_cache_path] = @old_file_cache_path
     FileUtils.remove_entry_secure deploy_directory if File.exist?(deploy_directory)
     FileUtils.remove_entry_secure base_dir_path
-    FileUtils.remove_entry_secure file_cache_path
     FileUtils.remove_entry_secure origin_repo_dir
   end
 
-  before(:all) do
-    @ohai = Ohai::System.new
-    @ohai.all_plugins(%w{platform os})
+  def expect_revision_to_be(revision, version)
+    rev_ver = shell_out!("git", "rev-parse", revision, cwd: deploy_directory).stdout.strip
+    expect(rev_ver).to eq(version)
+  end
+
+  def expect_branch_upstream_to_be(branch, upstream)
+    branch_upstream = shell_out("git", "rev-parse", "--abbrev-ref", "#{branch}@{upstream}", cwd: deploy_directory).stdout.strip
+    if upstream.nil?
+      expect(branch_upstream).to eq("")
+    else
+      expect(branch_upstream).to eq(upstream)
+    end
+  end
+
+  def expect_branch_to_be(branch)
+    head_branch = shell_out!("git name-rev --name-only HEAD", cwd: deploy_directory).stdout.strip
+    expect(head_branch).to eq(branch)
   end
 
   context "working with pathes with special characters" do
@@ -102,156 +97,222 @@ describe Chef::Resource::Git, requires_git: true do
     end
 
     it "clones a repository with a space in the path" do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository "#{path_with_spaces}/example-repo.gitbundle"
-      end.run_action(:sync)
+      repo = "#{path_with_spaces}/example-repo.gitbundle"
+      git(deploy_directory) do
+        repository repo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
     end
   end
 
   context "when deploying from an annotated tag" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "v1.0.0"
-      end
-    end
-
-    # We create a copy of the basic_git_resource so that we can run
-    # the resource again and verify that it doesn't update.
-    let(:copy_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "v1.0.0"
-      end
-    end
-
     it "checks out the revision pointed to by the tag commit, not the tag commit itself" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(v1_commit)
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", v1_commit)
+      expect_branch_to_be("tags/v1.0.0^0") # detatched
       # also verify the tag commit itself is what we expect as an extra sanity check
-      rev = shell_out!("git rev-parse v1.0.0", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(rev).to eq(v1_tag)
+      expect_revision_to_be("v1.0.0", v1_tag)
     end
 
     it "doesn't update if up-to-date" do
-      # this used to fail because we didn't resolve the annotated tag
-      # properly to the pointed to commit.
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(v1_commit)
-
-      copy_git_resource.run_action(:sync)
-      expect(copy_git_resource).not_to be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+        expect_branch_to_be("tags/v1.0.0^0") # detatched
+      end.should_not_be_updated
     end
   end
 
   context "when deploying from a SHA revision" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository git_bundle_repo
-      end
-    end
-
-    # We create a copy of the basic_git_resource so that we can run
-    # the resource again and verify that it doesn't update.
-    let(:copy_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-      end
+    it "checks out the expected revision ed18" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+      expect_branch_to_be("master~1") # detatched
     end
 
-    it "checks out the expected revision ed18" do
-      basic_git_resource.revision rev_foo
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_foo)
+    it "checks out the expected revision ed18 to a local branch" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+      expect_branch_to_be("deploy") # detatched
     end
 
     it "doesn't update if up-to-date" do
-      basic_git_resource.revision rev_foo
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_foo)
-
-      copy_git_resource.revision rev_foo
-      copy_git_resource.run_action(:sync)
-      expect(copy_git_resource).not_to be_updated
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+
+      git deploy_directory do
+        repository origin_repo
+        revision rev_foo
+      end.should_not_be_updated
+      expect_branch_to_be("master~1") # detatched
     end
 
     it "checks out the expected revision 972d" do
-      basic_git_resource.revision rev_testing
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_testing)
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_testing
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_testing)
+      expect_branch_to_be("master~2") # detatched
+    end
+
+    it "checks out the expected revision 972d to a local branch" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_testing
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_testing)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when deploying from a revision named 'HEAD'" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "HEAD"
-      end
+    it "checks out the expected revision" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out the expected revision" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out the expected revision, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out the expected revision to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when deploying from the default revision" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        # use default
-      end
+    it "checks out HEAD as the default revision" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      expect_branch_upstream_to_be("master", "origin/master")
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out HEAD as the default revision" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out HEAD as the default revision, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out HEAD as the default revision to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_branch_upstream_to_be("deploy", nil)
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when dealing with a repo with a degenerate tag named 'HEAD'" do
     before do
-      shell_out!("git tag -m\"degenerate tag\" HEAD ed181b3419b6f489bedab282348162a110d6d3a1",
-        cwd: origin_repo)
+      shell_out!("git", "tag", "-m\"degenerate tag\"", "HEAD", "ed181b3419b6f489bedab282348162a110d6d3a1", cwd: origin_repo)
     end
 
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "HEAD"
-      end
+    it "checks out the (master) HEAD revision and ignores the tag" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    let(:git_resource_default_rev) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        # use default of revision
-      end
+    it "checks out the (master) HEAD revision and ignores the tag, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out the (master) HEAD revision and ignores the tag" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD",
-        cwd: deploy_directory,
-        returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out the (master) HEAD revision and ignores the tag to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
 
     it "checks out the (master) HEAD revision when no revision is specified (ignores tag)" do
-      git_resource_default_rev.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD",
-        cwd: deploy_directory,
-        returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out the (master) HEAD revision when no revision is specified (ignores tag), and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
+    it "checks out the (master) HEAD revision when no revision is specified (ignores tag) to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
+    end
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 127af0470f..e2e967ba9c 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -74,6 +74,8 @@ require "spec/support/local_gems.rb" if File.exists?(File.join(File.dirname(__FI
 require "spec/support/platform_helpers"
 require "spec/support/shared/unit/mock_shellout"
 
+require "spec/support/recipe_dsl_helper"
+
 # Autoloads support files
 # Excludes support/platforms by default
 # Do not change the gsub.
diff --git a/spec/support/recipe_dsl_helper.rb b/spec/support/recipe_dsl_helper.rb
new file mode 100644
index 0000000000..2542345ed4
--- /dev/null
+++ b/spec/support/recipe_dsl_helper.rb
@@ -0,0 +1,83 @@
+#
+# This is a helper for functional tests to embed the recipe DSL directly into the rspec example blocks using
+# unified mode.
+#
+# If you wind up wanting to stub/expect on internal details of the resource/provider you are not testing the
+# public API and are trying to write a unit test, which this is not designed for.
+#
+# If you want to start writing full recipes and testing them, doing notifies/subscribes/etc then you are writing
+# an integration test, and not a functional single-resource test, which this is not designed for.
+#
+# Examples:
+#
+# it "creates a file" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_be_updated
+#   expect(IO.read("/tmp/foo.xyz").to eql("content")
+# end
+#
+# it "is idempotent" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_be_updated
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_not_be_updated
+#   expect(IO.read("/tmp/foo.xyz").to eql("content")
+# end
+#
+# it "has a failure" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   expect { file "/tmp/lksjdflksjdf/foo.xyz" do
+#     content "whatever"
+#   end }.to raise_error(Chef::Exception::EnclosingDirectoryDoesNotExist)
+# end
+#
+module RecipeDSLHelper
+  include Chef::DSL::Recipe
+  def event_dispatch
+    @event_dispatch ||= Chef::EventDispatch::Dispatcher.new
+  end
+
+  def node
+    @node ||= Chef::Node.new.tap do |n|
+      # clone the global ohai data to keep tests fast but reasonably isolated
+      n.consume_external_attrs(OHAI_SYSTEM.data.dup, {})
+    end
+  end
+
+  def run_context
+    @run_context ||= Chef::RunContext.new(node, {}, event_dispatch).tap do |rc|
+      rc.resource_collection.unified_mode = true
+      Chef::Runner.new(rc)
+    end
+  end
+
+  def cookbook_name
+    "rspec"
+  end
+
+  def recipe_name
+    "default"
+  end
+
+  def declare_resource(type, name, created_at: nil, run_context: self.run_context, &resource_attrs_block)
+    created_at = caller[0]
+    rspec_context = self
+    # we slightly abuse the "enclosing_provider" method_missing magic to send methods to the rspec example block so that
+    # rspec `let` methods work as arguments to resource properties
+    resource = super(type, name, created_at: created_at, run_context: run_context, enclosing_provider: rspec_context, &resource_attrs_block)
+    # we also inject these methods to make terse expression of checking the updated status (so it is more readiable and
+    # therefore should get used more -- even though it is "should" vs. "expect")
+    resource.define_singleton_method(:should_be_updated) do
+      rspec_context.expect(self).to be_updated
+    end
+    resource.define_singleton_method(:should_not_be_updated) do
+      rspec_context.expect(self).not_to be_updated
+    end
+    resource
+  end
+end
diff --git a/spec/unit/provider/git_spec.rb b/spec/unit/provider/git_spec.rb
index b98745b3ca..2f253358f1 100644
--- a/spec/unit/provider/git_spec.rb
+++ b/spec/unit/provider/git_spec.rb
@@ -400,7 +400,8 @@ describe Chef::Provider::Git do
     @provider.clone
   end
 
-  it "runs a checkout command with default options" do
+  it "runs a checkout command when the local branch is set" do
+    @resource.checkout_branch "deploy"
     expect(@provider).to receive(:shell_out!).with("git branch -f deploy d35af14d41ae22b19da05d7d03a0bafc321b244c", cwd: "/my/deploy/dir",
                                                                                                                     log_tag: "git[web2.0 app]").ordered
     expect(@provider).to receive(:shell_out!).with("git checkout deploy", cwd: "/my/deploy/dir",
@@ -607,7 +608,7 @@ describe Chef::Provider::Git do
 
     it "does not raise an error if user exists" do
       allow(@provider).to receive(:get_homedir).with(@resource.user).and_return("/home/test")
-      expect { @provider.run_action(:sync) }.not_to raise_error(ArgumentError)
+      expect { @provider.run_action(:sync) }.not_to raise_error
     end
   end
 
@@ -622,8 +623,10 @@ describe Chef::Provider::Git do
     end
 
     it "does not raise an error if user exists" do
+      allow(@provider).to receive(:action_sync) # stub the entire action
+      allow(::File).to receive(:directory?).with("/my/deploy").and_return(true)
       allow(@provider).to receive(:get_homedir).with(@resource.user).and_return("/home/test")
-      expect { @provider.run_action(:sync) }.not_to raise_error(Chef::Exceptions::User)
+      expect { @provider.run_action(:sync) }.not_to raise_error
     end
   end
 
diff --git a/spec/unit/provider/subversion_spec.rb b/spec/unit/provider/subversion_spec.rb
index f0393f6b40..f3d8404841 100644
--- a/spec/unit/provider/subversion_spec.rb
+++ b/spec/unit/provider/subversion_spec.rb
@@ -46,7 +46,8 @@ describe Chef::Provider::Subversion do
   it "converts resource properties to options for shell_out" do
     expect(@provider.run_options).to eq({})
     @resource.user "deployninja"
-    expect(@provider.run_options).to eq({ user: "deployninja" })
+    expect(@provider).to receive(:get_homedir).and_return("/home/deployninja")
+    expect(@provider.run_options).to eq({ user: "deployninja", environment: { "HOME" => "/home/deployninja" } })
   end
 
   context "determining the revision of the currently deployed code" do
@@ -221,7 +222,8 @@ describe Chef::Provider::Subversion do
     @resource.user "whois"
     @resource.group "thisis"
     expected_cmd = "svn checkout -q   -r12345 http://svn.example.org/trunk/ /my/deploy/dir"
-    expect(@provider).to receive(:shell_out!).with(expected_cmd, { user: "whois", group: "thisis" })
+    expect(@provider).to receive(:get_homedir).and_return("/home/whois")
+    expect(@provider).to receive(:shell_out!).with(expected_cmd, { user: "whois", group: "thisis", environment: { "HOME" => "/home/whois" } })
     @provider.run_action(:checkout)
     expect(@resource).to be_updated
   end
diff --git a/spec/unit/resource/git_spec.rb b/spec/unit/resource/scm/git_spec.rb
index 740af48e5f..706b7c370b 100644
--- a/spec/unit/resource/git_spec.rb
+++ b/spec/unit/resource/scm/git_spec.rb
@@ -17,6 +17,7 @@
 #
 
 require "spec_helper"
+require_relative "scm"
 
 describe Chef::Resource::Git do
 
@@ -29,8 +30,55 @@ describe Chef::Resource::Git do
 
   let(:resource) { Chef::Resource::Git.new("fakey_fakerton") }
 
-  it "is a subclass of Chef::Resource::Scm" do
-    expect(resource).to be_a_kind_of(Chef::Resource::Scm)
+  it_behaves_like "an SCM resource"
+
+  it "takes the depth as an integer for shallow clones" do
+    resource.depth 5
+    expect(resource.depth).to eq(5)
+    expect { resource.depth "five" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to nil depth for a full clone" do
+    expect(resource.depth).to be_nil
+  end
+
+  it "takes a boolean for #enable_submodules" do
+    resource.enable_submodules true
+    expect(resource.enable_submodules).to be_truthy
+    expect { resource.enable_submodules "lolz" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to not enabling submodules" do
+    expect(resource.enable_submodules).to be_falsey
+  end
+
+  it "takes a boolean for #enable_checkout" do
+    resource.enable_checkout true
+    expect(resource.enable_checkout).to be_truthy
+    expect { resource.enable_checkout "lolz" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to enabling checkout" do
+    expect(resource.enable_checkout).to be_truthy
+  end
+
+  it "takes a string for the remote" do
+    resource.remote "opscode"
+    expect(resource.remote).to eql("opscode")
+    expect { resource.remote 1337 }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to ``origin'' for the remote" do
+    expect(resource.remote).to eq("origin")
+  end
+
+  it "takes a string for the ssh wrapper" do
+    resource.ssh_wrapper "with_ssh_fu"
+    expect(resource.ssh_wrapper).to eql("with_ssh_fu")
+  end
+
+  it "defaults to nil for the ssh wrapper" do
+    expect(resource.ssh_wrapper).to be_nil
   end
 
   it "uses aliases revision as branch" do
diff --git a/spec/unit/resource/scm_spec.rb b/spec/unit/resource/scm/scm.rb
index 6f282c1350..28c3f73136 100644
--- a/spec/unit/resource/scm_spec.rb
+++ b/spec/unit/resource/scm/scm.rb
@@ -19,9 +19,7 @@
 
 require "spec_helper"
 
-describe Chef::Resource::Scm do
-  let(:resource) { Chef::Resource::Scm.new("fakey_fakerton") }
-
+shared_examples_for "an SCM resource" do
   it "the destination property is the name_property" do
     expect(resource.destination).to eql("fakey_fakerton")
   end
@@ -78,55 +76,6 @@ describe Chef::Resource::Scm do
     expect(resource.group).to eq(23)
   end
 
-  it "takes the depth as an integer for shallow clones" do
-    resource.depth 5
-    expect(resource.depth).to eq(5)
-    expect { resource.depth "five" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to nil depth for a full clone" do
-    expect(resource.depth).to be_nil
-  end
-
-  it "takes a boolean for #enable_submodules" do
-    resource.enable_submodules true
-    expect(resource.enable_submodules).to be_truthy
-    expect { resource.enable_submodules "lolz" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to not enabling submodules" do
-    expect(resource.enable_submodules).to be_falsey
-  end
-
-  it "takes a boolean for #enable_checkout" do
-    resource.enable_checkout true
-    expect(resource.enable_checkout).to be_truthy
-    expect { resource.enable_checkout "lolz" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to enabling checkout" do
-    expect(resource.enable_checkout).to be_truthy
-  end
-
-  it "takes a string for the remote" do
-    resource.remote "opscode"
-    expect(resource.remote).to eql("opscode")
-    expect { resource.remote 1337 }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to ``origin'' for the remote" do
-    expect(resource.remote).to eq("origin")
-  end
-
-  it "takes a string for the ssh wrapper" do
-    resource.ssh_wrapper "with_ssh_fu"
-    expect(resource.ssh_wrapper).to eql("with_ssh_fu")
-  end
-
-  it "defaults to nil for the ssh wrapper" do
-    expect(resource.ssh_wrapper).to be_nil
-  end
-
   it "defaults to nil for the environment" do
     expect(resource.environment).to be_nil
   end
diff --git a/spec/unit/resource/subversion_spec.rb b/spec/unit/resource/scm/subversion_spec.rb
index 48cc7fea42..394ed6be9f 100644
--- a/spec/unit/resource/subversion_spec.rb
+++ b/spec/unit/resource/scm/subversion_spec.rb
@@ -17,6 +17,7 @@
 #
 
 require "spec_helper"
+require_relative "scm"
 
 describe Chef::Resource::Subversion do
   static_provider_resolution(
@@ -28,9 +29,7 @@ describe Chef::Resource::Subversion do
 
   let(:resource) { Chef::Resource::Subversion.new("fakey_fakerton") }
 
-  it "is a subclass of Resource::Scm" do
-    expect(resource).to be_a_kind_of(Chef::Resource::Scm)
-  end
+  it_behaves_like "an SCM resource"
 
   it "the destination property is the name_property" do
     expect(resource.destination).to eql("fakey_fakerton")