Added support for p4config.

- Removed p4port, p4client, p4user to keep name space clean.
 - Changed notify to Puppet.debug
 - Updated markdown and examples
 - Updated unit tests

8 files changed, 57 insertions, 94 deletions

diff --git a/README.markdown b/README.markdown
index 6dc36d2..5f8aa9d 100644
--- a/README.markdown
+++ b/README.markdown
@@ -329,18 +329,19 @@ connection settings.
       provider   => p4
     }
 
-If no `p4client` name is provided a workspace generated name is calculated based on the 
-Digest of path.  For example:
+If no `P4CLIENT` environment name is provided a workspace generated name is calculated
+based on the Digest of path.  For example:
 
     puppet-91bc00640c4e5a17787286acbe2c021c
 
-Providing a `p4client` name will create/update the client workspace in Perforce.  The
-value replaces the P4CLIENT environment variable.
+A Perforce configuration file can be used by setting the `P4CONFIG` environment or
+defining `p4config`.  If a configuration is defined, then the environment variable for 
+`P4CLIENT` is replaced.
  
     vcsrepo { "/path/to/repo":
       ensure     => present,
-      provider   => p4
-      p4client   => "my_client_ws"
+      provider   => p4,
+      p4config   => '.p4config'
     }
 
 #####To create/update and sync a Perforce workspace
@@ -371,36 +372,10 @@ You can also set `revision` to a label:
         revision => 'my_label'
     }
 
-Check out as a user by setting `p4user`:
-
-    vcsrepo { "/path/to/repo":
-        ensure   => present,
-        provider => p4,
-        source   => '//depot/branch/...',
-        p4user   => 'user'
-    }
-
-You can set `p4port` to specify a Perforce server:
-
-    vcsrepo { "/path/to/repo":
-        ensure   => present,
-        provider => p4,
-        source   => '//depot/branch/...',
-        p4port   => 'ssl:perforce.com:1666'
-    }
-
-You can set `p4passwd` for authentication :
-
-    vcsrepo { "/path/to/repo":
-        ensure   => present,
-        provider => p4,
-        source   => '//depot/branch/...',
-        p4port   => 'ssl:perforce.com:1666'
-    }
+#####To authenticate against the Perforce server
 
-If `p4port`, `p4user`, `p4charset`, `p4passwd` or `p4client` are specified they will 
-override the environment variabels P4PORT, P4USER, etc... If a P4CONFIG file is defined,
-the config file settings will take precedence.
+Either set the environment variables `P4USER` and `P4PASSWD` or use a configuration file.
+For secure servers set the `P4PASSWD` with a valid ticket generated using `p4 login -p`.
 
 #####Further Examples
 
diff --git a/examples/p4/create_client.pp b/examples/p4/create_client.pp
index 94017bc..f491701 100644
--- a/examples/p4/create_client.pp
+++ b/examples/p4/create_client.pp
@@ -1,5 +1,4 @@
-vcsrepo { "/tmp/vcstest-p4-create_client":
+vcsrepo { "/tmp/vcstest/p4_client_root":
   ensure    => present,
-  provider  => p4, 
-  p4client  => "puppet-test001"
+  provider  => p4
 }
\ No newline at end of file
diff --git a/examples/p4/delete_client.pp b/examples/p4/delete_client.pp
index 20bdd47..239349b 100644
--- a/examples/p4/delete_client.pp
+++ b/examples/p4/delete_client.pp
@@ -1,5 +1,4 @@
-vcsrepo { "/tmp/vcstest-p4-create_client":
+vcsrepo { "/tmp/vcstest/p4_client_root":
   ensure    => absent,
-  provider  => p4, 
-  p4client  => "puppet-test001"
+  provider  => p4
 }
\ No newline at end of file
diff --git a/examples/p4/latest_client.pp b/examples/p4/latest_client.pp
index 54f9062..9dcd68e 100644
--- a/examples/p4/latest_client.pp
+++ b/examples/p4/latest_client.pp
@@ -1,6 +1,5 @@
-vcsrepo { "/tmp/vcstest-p4-create_client":
+vcsrepo { "/tmp/vcstest/p4_client_root":
   ensure    => latest,
   provider  => p4, 
-  p4client  => "puppet-test001",
   source    => "//depot/..."
 }
\ No newline at end of file
diff --git a/examples/p4/sync_client.pp b/examples/p4/sync_client.pp
index e22f6b0..55dc9dc 100644
--- a/examples/p4/sync_client.pp
+++ b/examples/p4/sync_client.pp
@@ -1,7 +1,6 @@
-vcsrepo { "/tmp/vcstest-p4-create_client":
+vcsrepo { "/tmp/vcstest/p4_client_root":
   ensure    => present,
   provider  => p4, 
-  p4client  => "puppet-test001",
-  source    => "//depot/..."
+  source    => "//depot/...",
   revision  => "30"
 }
\ No newline at end of file
diff --git a/lib/puppet/provider/vcsrepo/p4.rb b/lib/puppet/provider/vcsrepo/p4.rb
index de3bc7a..ec40cf3 100644
--- a/lib/puppet/provider/vcsrepo/p4.rb
+++ b/lib/puppet/provider/vcsrepo/p4.rb
@@ -3,7 +3,7 @@ require File.join(File.dirname(__FILE__), '..', 'vcsrepo')
 Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) do
   desc "Supports Perforce depots"
 
-  has_features :filesystem_types, :reference_tracking, :p4_config
+  has_features :filesystem_types, :reference_tracking, :p4config
   
   def create
     # create or update client 
@@ -90,7 +90,7 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
   # +source+:: Depot path to sync
   # +revision+:: Perforce change list to sync to (optional)
   def sync_client(source, revision)
-    notice "Syncing: #{source}"
+    Puppet.debug "Syncing: #{source}"
     args = ['sync']
     if revision
       args.push(source + "@" + revision)
@@ -102,12 +102,23 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
   
   # Returns the name of the Perforce client workspace 
   def client_name
+    p4config = @resource.value(:p4config)
+    
+    # default (generated) client name
     path = @resource.value(:path)
-    client = @resource.value(:p4client)
-    if not client
-      client = "puppet-" + Digest::MD5.hexdigest(path)
+    default = "puppet-" + Digest::MD5.hexdigest(path)
+    
+    # check config for client name
+    set_client = nil
+    if p4config && File.file?(p4config)
+      open(p4config) do |f|
+        m = f.grep(/^P4CLIENT=/).pop
+        p = /^P4CLIENT=(.*)$/
+        set_client = p.match(m)[1] if m
+      end
     end
-    return client
+  
+    return set_client || ENV['P4CLIENT'] || default
   end
   
   # Create (or update) a client workspace spec.
@@ -116,7 +127,7 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
   # +client+:: Name of client workspace
   # +path+:: The Root location of the Perforce client workspace
   def create_client(client, path)
-    notice "Creating client: #{client}"
+    Puppet.debug "Creating client: #{client}"
     hash = parse_client(client)
     hash['Root'] = path
     hash['Description'] = "Generated by Puppet VCSrepo"
@@ -158,20 +169,14 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
     p4(args, {:input => spec, :marshal => false})
   end 
   
+  # Sets Perforce Configuration environment.
+  # P4CLIENT generated, but overwitten if defined in config.
   def config
-    p4port = @resource.value(:p4port)
-    p4user = @resource.value(:p4user)
-    p4charset = @resource.value(:p4charset)
-    p4passwd = @resource.value(:p4passwd)
-    p4client = @resource.value(:p4client) || client_name
-  
-    cfg = Hash.new  
-    cfg.store 'P4USER', p4user if p4user
-    cfg.store 'P4PORT', p4port if p4port
-    cfg.store 'P4CHARSET', p4charset if p4charset
-    cfg.store 'P4PASSWD', p4passwd if p4passwd
-    cfg.store 'P4CLIENT', p4client if p4client
+    p4config = @resource.value(:p4config)
     
+    cfg = Hash.new
+    cfg.store 'P4CONFIG', p4config if p4config
+    cfg.store 'P4CLIENT', client_name
     return cfg  
   end
   
@@ -187,14 +192,14 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
     cmd.push args
     cmd_str = cmd.respond_to?(:join) ? cmd.join(' ') : cmd
     
-    notice "environment: #{config}"
-    notice "command: #{cmd_str}"
+    Puppet.debug "environment: #{config}"
+    Puppet.debug "command: #{cmd_str}"
     
     hash = Hash.new
     Open3.popen3(config, cmd_str) do |i, o, e, t|
       # Send input stream if provided
       if(opts[:input])
-        notice "input:\n" + opts[:input]
+        Puppet.debug "input:\n" + opts[:input]
         i.write opts[:input]
         i.close
       end
@@ -212,7 +217,7 @@ Puppet::Type.type(:vcsrepo).provide(:p4, :parent => Puppet::Provider::Vcsrepo) d
       end
     end
     
-    notice "hash: #{hash}\n"
+    Puppet.debug "hash: #{hash}\n"
     return hash
   end
   
diff --git a/lib/puppet/type/vcsrepo.rb b/lib/puppet/type/vcsrepo.rb
index 649789a..7ada022 100644
--- a/lib/puppet/type/vcsrepo.rb
+++ b/lib/puppet/type/vcsrepo.rb
@@ -40,7 +40,7 @@ Puppet::Type.newtype(:vcsrepo) do
   feature :depth,
           "The provider can do shallow clones"
 
-  feature :p4_config,
+  feature :p4config,
           "The provider understands Perforce Configuration"
 
   ensurable do
@@ -212,26 +212,10 @@ Puppet::Type.newtype(:vcsrepo) do
     desc "The value to be used to do a shallow clone."
   end
 
-  newparam :p4port, :required_features => [:p4_config] do
-    desc "The Perforce P4PORT environment."
-  end
-  
-  newparam :p4user, :required_features => [:p4_config] do
-    desc "The Perforce P4USER environment."
-  end
-  
-  newparam :p4client, :required_features => [:p4_config] do
-    desc "The Perforce P4CLIENT environment."
+  newparam :p4config, :required_features => [:p4config] do
+    desc "The Perforce P4CONFIG environment."
   end
 
-  newparam :p4charset, :required_features => [:p4_config] do
-    desc "The Perforce P4CHARSET environment."
-  end
-  
-  newparam :p4passwd, :required_features => [:p4_config] do
-    desc "The Perforce P4PASSWD environment."
-  end
-    
   autorequire(:package) do
     ['git', 'git-core']
   end
diff --git a/spec/unit/puppet/provider/vcsrepo/p4_spec.rb b/spec/unit/puppet/provider/vcsrepo/p4_spec.rb
index 2c202f2..2bb81c8 100644
--- a/spec/unit/puppet/provider/vcsrepo/p4_spec.rb
+++ b/spec/unit/puppet/provider/vcsrepo/p4_spec.rb
@@ -24,9 +24,10 @@ describe Puppet::Type.type(:vcsrepo).provider(:p4) do
     context 'with source and revision' do
       it "should execute 'p4 sync' with the revision" do
         resource[:source] = 'something'
-        resource[:p4client] = 'client_ws'
         resource[:revision] = '1'
-        provider.expects(:p4).with(['client', '-o', resource.value(:p4client)]).returns({})
+        ENV['P4CLIENT'] = 'client_ws1'
+          
+        provider.expects(:p4).with(['client', '-o', 'client_ws1']).returns({})
         provider.expects(:p4).with(['client', '-i'], spec)
         provider.expects(:p4).with(['sync', resource.value(:source) + "@" + resource.value(:revision)])
         provider.create
@@ -36,8 +37,9 @@ describe Puppet::Type.type(:vcsrepo).provider(:p4) do
     context 'without revision' do
       it "should just execute 'p4 sync' without a revision" do
         resource[:source] = 'something'
-        resource[:p4client] = 'client_ws'
-        provider.expects(:p4).with(['client', '-o', resource.value(:p4client)]).returns({})
+        ENV['P4CLIENT'] = 'client_ws2'
+        
+        provider.expects(:p4).with(['client', '-o', 'client_ws2']).returns({})
         provider.expects(:p4).with(['client', '-i'], spec)
         provider.expects(:p4).with(['sync', resource.value(:source)])
         provider.create
@@ -55,8 +57,9 @@ describe Puppet::Type.type(:vcsrepo).provider(:p4) do
 
   describe 'destroying' do
     it "it should remove the directory" do
-      resource[:p4client] = 'test_client'
-      provider.expects(:p4).with(['client', '-d', '-f', resource.value(:p4client)])
+      ENV['P4CLIENT'] = 'test_client'
+      
+      provider.expects(:p4).with(['client', '-d', '-f', 'test_client'])
       expects_rm_rf
       provider.destroy
     end