fix git provider checkout of a remote ref on an existing repo
authorJoshua Hoblitt <jhoblitt@cpan.org>
Tue, 16 Jul 2013 23:25:09 +0000 (16:25 -0700)
committerJoshua Hoblitt <jhoblitt@vm2.sdm.noao.edu>
Wed, 17 Jul 2013 20:39:29 +0000 (13:39 -0700)
Per discussion of https://github.com/puppetlabs/puppetlabs-vcsrepo/issues/51 in
the git channel on freenode, EugeneKay <eugene@kashpureff.org> stated that `git
rev-parse` is not capable of inspecting remote refs but that `git ls-remote`
is.  This patch makes a second attempt to resolve the ref with `ls-remote` if
`rev-parse` fails.

The git provider also appears to support several type features that are not
tagged under `has_features`.  It's not clear if this is the best way to resolve
this issue or if the provider should be refactored to work with different type
features.

Demonstration of the problem with changing refs (branches and tags)

    $ git --version
    git version 1.7.1
    $ cat master.pp branch.pp
      vcsrepo { '/tmp/vcsrepo':
        ensure    => present,
        provider  => git,
        source    => 'https://github.com/puppetlabs/puppetlabs-vcsrepo.git',
        revision  => 'master',
      }
      vcsrepo { '/tmp/vcsrepo':
        ensure    => present,
        provider  => git,
        source    => 'https://github.com/puppetlabs/puppetlabs-vcsrepo.git',
        revision  => 'feature/cvs',
      }
    $ puppet apply --modulepath=`pwd`/.. master.pp
    Notice: /Stage[main]//Vcsrepo[/tmp/vcsrepo]/ensure: Creating repository from present
    Notice: /Stage[main]//Vcsrepo[/tmp/vcsrepo]/ensure: created
    Notice: Finished catalog run in 2.19 seconds
    $ puppet apply --modulepath=`pwd`/.. branch.pp
    Error: /Stage[main]//Vcsrepo[/tmp/vcsrepo]: Could not evaluate: Execution of '/usr/bin/git rev-parse feature/cvs' returned 128: fatal: ambiguous argument 'feature/cvs': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions
    feature/cvs

    Notice: Finished catalog run in 1.69 seconds

lib/puppet/provider/vcsrepo/git.rb
spec/unit/puppet/provider/vcsrepo/git_spec.rb

index 76fa315..ac56ac5 100644 (file)
@@ -56,7 +56,20 @@ Puppet::Type.type(:vcsrepo).provide(:git, :parent => Puppet::Provider::Vcsrepo)
     if tag_revision?(@resource.value(:revision))
       canonical = at_path { git_with_identity('show', @resource.value(:revision)).scan(/^commit (.*)/).to_s }
     else
     if tag_revision?(@resource.value(:revision))
       canonical = at_path { git_with_identity('show', @resource.value(:revision)).scan(/^commit (.*)/).to_s }
     else
-      canonical = at_path { git_with_identity('rev-parse', @resource.value(:revision)).chomp }
+      # if it's not a tag, look for it as a local ref
+      canonical = at_path { git_with_identity('rev-parse', '--revs-only', @resource.value(:revision)).chomp }
+      if canonical.empty?
+        # git rev-parse executed properly but didn't find the ref;
+        # look for it in the remote
+        remote_ref = at_path { git_with_identity('ls-remote', '--heads', '--tags', @resource.value(:remote), @resource.value(:revision)).chomp }
+        if remote_ref.empty?
+          fail("#{@resource.value(:revision)} is not a local or remote ref")
+        end
+
+        # $ git ls-remote --heads --tags origin feature/cvs 
+        # 7d4244b35e72904e30130cad6d2258f901c16f1a     refs/heads/feature/cvs
+        canonical = remote_ref.split.first
+      end
     end
 
     if current == canonical
     end
 
     if current == canonical
index cb6c0c6..e10fd1e 100644 (file)
@@ -132,7 +132,7 @@ describe_provider :vcsrepo, :git, :resource => {:path => '/tmp/vcsrepo'} do
           provider.expects(:git).with('config', 'remote.origin.url').returns('')
           provider.expects(:git).with('fetch', 'origin') # FIXME
           provider.expects(:git).with('fetch', '--tags', 'origin')
           provider.expects(:git).with('config', 'remote.origin.url').returns('')
           provider.expects(:git).with('fetch', 'origin') # FIXME
           provider.expects(:git).with('fetch', '--tags', 'origin')
-          provider.expects(:git).with('rev-parse', resource.value(:revision)).returns('currentsha')
+          provider.expects(:git).with('rev-parse', '--revs-only', resource.value(:revision)).returns('currentsha')
           provider.expects(:git).with('tag', '-l').returns("Hello")
           provider.revision.should == resource.value(:revision)
         end
           provider.expects(:git).with('tag', '-l').returns("Hello")
           provider.revision.should == resource.value(:revision)
         end
@@ -143,12 +143,36 @@ describe_provider :vcsrepo, :git, :resource => {:path => '/tmp/vcsrepo'} do
           provider.expects(:git).with('config', 'remote.origin.url').returns('')
           provider.expects(:git).with('fetch', 'origin') # FIXME
           provider.expects(:git).with('fetch', '--tags', 'origin')
           provider.expects(:git).with('config', 'remote.origin.url').returns('')
           provider.expects(:git).with('fetch', 'origin') # FIXME
           provider.expects(:git).with('fetch', '--tags', 'origin')
-          provider.expects(:git).with('rev-parse', resource.value(:revision)).returns('othersha')
+          provider.expects(:git).with('rev-parse', '--revs-only', resource.value(:revision)).returns('othersha')
           provider.expects(:git).with('tag', '-l').returns("Hello")
           provider.revision.should == 'currentsha'
         end
       end
 
           provider.expects(:git).with('tag', '-l').returns("Hello")
           provider.revision.should == 'currentsha'
         end
       end
 
+      context "when its a ref to a remote head" do
+        it "should return the revision" do
+          provider.expects(:git).with('config', 'remote.origin.url').returns('')
+          provider.expects(:git).with('fetch', 'origin') # FIXME
+          provider.expects(:git).with('fetch', '--tags', 'origin')
+          provider.expects(:git).with('tag', '-l').returns("Hello")
+          provider.expects(:git).with('rev-parse', '--revs-only', resource.value(:revision)).returns('')
+          provider.expects(:git).with('ls-remote', '--heads', '--tags', 'origin', resource.value(:revision)).returns("newsha refs/heads/#{resource.value(:revision)}")
+          provider.revision.should == 'currentsha'
+        end
+      end
+
+      context "when its a ref to non existant remote head" do
+        it "should fail" do
+          provider.expects(:git).with('config', 'remote.origin.url').returns('')
+          provider.expects(:git).with('fetch', 'origin') # FIXME
+          provider.expects(:git).with('fetch', '--tags', 'origin')
+          provider.expects(:git).with('tag', '-l').returns("Hello")
+          provider.expects(:git).with('rev-parse', '--revs-only', resource.value(:revision)).returns('')
+          provider.expects(:git).with('ls-remote', '--heads', '--tags', 'origin', resource.value(:revision)).returns('')
+          expect { provider.revision }.should raise_error(Puppet::Error, /not a local or remote ref$/)
+        end
+      end
+
       context "when the source is modified" do
         resource_with :source => 'git://git@foo.com/bar.git' do
           it "should update the origin url" do
       context "when the source is modified" do
         resource_with :source => 'git://git@foo.com/bar.git' do
           it "should update the origin url" do
@@ -156,7 +180,7 @@ describe_provider :vcsrepo, :git, :resource => {:path => '/tmp/vcsrepo'} do
             provider.expects(:git).with('config', 'remote.origin.url', 'git://git@foo.com/bar.git')
             provider.expects(:git).with('fetch', 'origin') # FIXME
             provider.expects(:git).with('fetch', '--tags', 'origin')
             provider.expects(:git).with('config', 'remote.origin.url', 'git://git@foo.com/bar.git')
             provider.expects(:git).with('fetch', 'origin') # FIXME
             provider.expects(:git).with('fetch', '--tags', 'origin')
-            provider.expects(:git).with('rev-parse', resource.value(:revision)).returns('currentsha')
+            provider.expects(:git).with('rev-parse', '--revs-only', resource.value(:revision)).returns('currentsha')
             provider.expects(:git).with('tag', '-l').returns("Hello")
             provider.revision.should == resource.value(:revision)
           end
             provider.expects(:git).with('tag', '-l').returns("Hello")
             provider.revision.should == resource.value(:revision)
           end