Merge branch 'file-test-on-windows'

* file-test-on-windows:
  Check for the appropriate permissions in File type tests on Windows
  Remove :fails_on_windows from file type tests that no longer fail on Windows
  Disable file bucket diffing tests on Windows
  Always put a slash between the checksum and path in filebucket URLs
  Treat Windows absolute paths as absolute paths
  Consolidate test logic determining if a registered file is in the temp directory
  Clarify logic and error messages when initializing Puppet::FileBucket::File
  Disable symlink related file tests on Windows

7 files changed, 34 insertions, 11 deletions

diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb
index d6f6a3747..870c50eec 100644
--- a/lib/puppet/file_bucket/dipper.rb
+++ b/lib/puppet/file_bucket/dipper.rb
@@ -35,11 +35,12 @@ class Puppet::FileBucket::Dipper
     begin
       file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path)
       files_original_path = absolutize_path(file)
-      dest_path = "#{@rest_path}#{file_bucket_file.name}#{files_original_path}"
+      dest_path = "#{@rest_path}#{file_bucket_file.name}/#{files_original_path}"
+      file_bucket_path = "#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}/#{files_original_path}"
 
       # Make a HEAD request for the file so that we don't waste time
       # uploading it if it already exists in the bucket.
-      unless Puppet::FileBucket::File.indirection.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}#{files_original_path}")
+      unless Puppet::FileBucket::File.indirection.head(file_bucket_path)
         Puppet::FileBucket::File.indirection.save(file_bucket_file, dest_path)
       end
 
diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb
index 08c0329f1..2a0558fde 100644
--- a/lib/puppet/file_bucket/file.rb
+++ b/lib/puppet/file_bucket/file.rb
@@ -15,11 +15,11 @@ class Puppet::FileBucket::File
   attr :bucket_path
 
   def initialize( contents, options = {} )
-    raise ArgumentError if !contents.is_a?(String)
-    @contents  = contents
+    raise ArgumentError.new("contents must be a String, got a #{contents.class}") unless contents.is_a?(String)
+    @contents = contents
 
     @bucket_path = options.delete(:bucket_path)
-    raise ArgumentError if options != {}
+    raise ArgumentError.new("Unknown option(s): #{options.keys.join(', ')}") unless options.empty?
   end
 
   def checksum_type
diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb
index 09cab97d9..706f67af9 100644
--- a/lib/puppet/file_serving/base.rb
+++ b/lib/puppet/file_serving/base.rb
@@ -53,7 +53,10 @@ class Puppet::FileServing::Base
   # Set our base path.
   attr_reader :path
   def path=(path)
-    raise ArgumentError.new("Paths must be fully qualified") unless path =~ /^#{::File::SEPARATOR}/
+    unless path =~ /^#{::File::SEPARATOR}/ or path =~ /^[a-z]:[\/\\]/i
+      raise ArgumentError.new("Paths must be fully qualified")
+    end
+
     @path = path
   end
 
diff --git a/lib/puppet/file_serving/indirection_hooks.rb b/lib/puppet/file_serving/indirection_hooks.rb
index a85e90ef1..2a0dc1792 100644
--- a/lib/puppet/file_serving/indirection_hooks.rb
+++ b/lib/puppet/file_serving/indirection_hooks.rb
@@ -17,6 +17,7 @@ module Puppet::FileServing::IndirectionHooks
 
     # Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol.
     return PROTOCOL_MAP["file"] if request.key =~ /^#{::File::SEPARATOR}/
+    return PROTOCOL_MAP["file"] if request.key =~ /^[a-z]:[\/\\]/i
     return PROTOCOL_MAP["file"] if request.protocol == "file"
 
     # We're heading over the wire the protocol is 'puppet' and we've got a server name or we're not named 'apply' or 'puppet'
diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
index 697d9df47..4dfbac9ab 100644
--- a/lib/puppet/indirector/request.rb
+++ b/lib/puppet/indirector/request.rb
@@ -127,7 +127,9 @@ class Puppet::Indirector::Request
       # because it rewrites the key.  We could otherwise strip server/port/etc
       # info out in the REST class, but it seemed bad design for the REST
       # class to rewrite the key.
-      if key.to_s =~ /^\w+:\/\// # it's a URI
+      if key.to_s =~ /^[a-z]:[\/\\]/i # It's an absolute path for Windows.
+        @key = key
+      elsif key.to_s =~ /^\w+:\/\// # it's a URI
         set_uri_key(key)
       else
         @key = key
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 988416ab5..07409a108 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -23,7 +23,7 @@ Puppet::Type.newtype(:file) do
     location, rather than using native resources, please contact
     Puppet Labs and we can hopefully work with you to develop a
     native resource to support what you are doing.
-    
+
     **Autorequires:** If Puppet is managing the user or group that owns a file, the file resource will autorequire them. If Puppet is managing any parent directories of a file, the file resource will autorequire them."
 
   def self.title_patterns
@@ -36,7 +36,7 @@ Puppet::Type.newtype(:file) do
 
     validate do |value|
       # accept various path syntaxes: lone slash, posix, win32, unc
-      unless (Puppet.features.posix? and value =~ /^\//) or (Puppet.features.microsoft_windows? and (value =~ /^[A-Za-z]:\// or value =~ /^\/\/[^\/]+\/[^\/]+/))
+      unless (Puppet.features.posix? and value =~ /^\//) or (value =~ /^[A-Za-z]:\// or value =~ /^\/\/[^\/]+\/[^\/]+/)
         fail Puppet::Error, "File paths must be fully qualified, not '#{value}'"
       end
     end
@@ -44,7 +44,21 @@ Puppet::Type.newtype(:file) do
     # convert the current path in an index into the collection and the last
     # path name. The aim is to use less storage for all common paths in a hierarchy
     munge do |value|
-      path, name = ::File.split(value.gsub(/\/+/,'/'))
+      # We need to save off, and remove the volume designator in the
+      # path if it is there, since File.split does not handle paths
+      # with volume designators properly, except when run on Windows.
+      # Since we are potentially compiling a catalog for a Windows
+      # machine on a non-Windows master, we need to handle this
+      # ourselves.
+      optional_volume_designator = value.match(/^([a-z]:)[\/\\].*/i)
+      value_without_designator   = value.sub(/^(?:[a-z]:)?(.*)/i, '\1')
+
+      path, name = ::File.split(value_without_designator.gsub(/\/+/,'/'))
+
+      if optional_volume_designator
+        path = optional_volume_designator[1] + path
+      end
+
       { :index => Puppet::FileCollection.collection.index(path), :name => name }
     end
 
diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb
index 39f85e2ad..8653a8f7a 100755
--- a/lib/puppet/type/file/source.rb
+++ b/lib/puppet/type/file/source.rb
@@ -72,7 +72,7 @@ module Puppet
           self.fail "Could not understand source #{source}: #{detail}"
         end
 
-        self.fail "Cannot use URLs of type '#{uri.scheme}' as source for fileserving" unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme)
+        self.fail "Cannot use URLs of type '#{uri.scheme}' as source for fileserving" unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme) or (Puppet.features.microsoft_windows? and uri.scheme =~ /^[a-z]$/i)
       end
     end
 
@@ -180,6 +180,8 @@ module Puppet
     private
 
     def uri
+      return nil if metadata.source =~ /^[a-z]:[\/\\]/i # Abspath for Windows
+
       @uri ||= URI.parse(URI.escape(metadata.source))
     end
   end