The environment validates its modulepath and manifestdir settings, but
it uses Dir.getwd to convert a relative path into an absolute path. The
problem is that on Windows, Dir.getwd returns a path with backslashes.
(Interestingly this only happens when puppet is loaded, not in irb for
example.) And since we do not yet support backslashes in Windows paths
or UNC paths, the directory is not included in the environment.

For the time being, I am using File.expand_path to normalize the path.
It has the side-effect of converting backslashes to forward slashes.
This is sufficient to work around backslashes in Dir.getwd. In the near
future, I will be refactoring how paths are split, validated, tested,
etc, and I have a REMIND in place to fix the environment.

But as a result of this change it exposed a bug in our rdoc parser
dealing with the finding the root of a path. The parser assumed that the
root was '/', but caused an infinite loop when passed a Windows path.

I added a test for this case, which is only run on Windows, because on
Unix File.dirname("C:/") == '.'.

After all of that, I had to disable one of the rdoc spec tests, because
it attempted to reproduce a specific bug, which caused rdoc to try to
create a directory of the form: C:/.../files/C:/.... Of course, this
fails because ':' is not a valid filename character on Windows.

Paired-with: Nick Lewis <n...@puppetlabs.com>
Reviewed-by: Jacob Helwig <ja...@puppetlabs.com>
Signed-off-by: Josh Cooper <j...@puppetlabs.com>
---
Local-branch: feature/master/8268-puppet-agent-windows
 lib/puppet/node/environment.rb           |    5 ++++-
 lib/puppet/util/rdoc/parser.rb           |    4 +++-
 spec/integration/application/doc_spec.rb |    2 +-
 spec/unit/node/environment_spec.rb       |    2 +-
 spec/unit/util/rdoc/parser_spec.rb       |    4 ++++
 5 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index f25bb65..4fc314a 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -131,9 +131,12 @@ class Puppet::Node::Environment
 
   def validate_dirs(dirs)
     dir_regex = Puppet.features.microsoft_windows? ? 
/^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/
+    # REMIND: Dir.getwd on windows returns a path containing backslashes, 
which when joined with
+    # dir containing forward slashes, breaks our regex matching. In general, 
path validation needs
+    # to be refactored which will be handled in a future commit.
     dirs.collect do |dir|
       if dir !~ dir_regex
-        File.join(Dir.getwd, dir)
+        File.expand_path(File.join(Dir.getwd, dir))
       else
         dir
       end
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index 762ce25..a8996ee 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -113,7 +113,9 @@ class Parser
       Puppet::Module.modulepath.each do |mp|
         # check that fullpath is a descendant of mp
         dirname = fullpath
-        while (dirname = File.dirname(dirname)) != '/'
+        previous = dirname
+        while (dirname = File.dirname(previous)) != previous
+          previous = dirname
           return nil if File.identical?(dirname,mp)
         end
       end
diff --git a/spec/integration/application/doc_spec.rb 
b/spec/integration/application/doc_spec.rb
index 47fd93a..2cf5fd1 100755
--- a/spec/integration/application/doc_spec.rb
+++ b/spec/integration/application/doc_spec.rb
@@ -5,7 +5,7 @@ require 'puppet_spec/files'
 describe Puppet::Application::Doc do
   include PuppetSpec::Files
 
-  it "should not generate an error when module dir overlaps parent of site.pp 
(#4798)", :'fails_on_ruby_1.9.2' => true, :fails_on_windows => true do
+  it "should not generate an error when module dir overlaps parent of site.pp 
(#4798)", :'fails_on_ruby_1.9.2' => true, :unless => 
Puppet.features.microsoft_windows? do
     begin
       # Note: the directory structure below is more complex than it
       # needs to be, but it's representative of the directory structure
diff --git a/spec/unit/node/environment_spec.rb 
b/spec/unit/node/environment_spec.rb
index eb20aa7..78d3834 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -144,7 +144,7 @@ describe Puppet::Node::Environment do
       FileTest.stubs(:directory?).returns true
       env = Puppet::Node::Environment.new("testing")
 
-      two = File.join(Dir.getwd, "two")
+      two = File.expand_path(File.join(Dir.getwd, "two"))
       env.validate_dirs([@path_one, 'two']).should == [@path_one, two]
     end
   end
diff --git a/spec/unit/util/rdoc/parser_spec.rb 
b/spec/unit/util/rdoc/parser_spec.rb
index 4c2c79e..29e3298 100755
--- a/spec/unit/util/rdoc/parser_spec.rb
+++ b/spec/unit/util/rdoc/parser_spec.rb
@@ -149,6 +149,10 @@ describe RDoc::Parser, :'fails_on_ruby_1.9.2' => true do
       File.stubs(:identical?).returns(false)
       @parser.split_module("/path/to/manifests/init.pp").should == 
RDoc::Parser::SITE
     end
+
+    it "should handle windows paths with drive letters", :if => 
Puppet.features.microsoft_windows? do
+      @parser.split_module("C:/temp/init.pp").should == RDoc::Parser::SITE
+    end
   end
 
   describe "when parsing AST elements" do
-- 
1.7.5.4

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to 
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to