On Sat, Mar 21, 2009 at 1:40 PM, Paul Nasrat <[email protected]> wrote:
> But you really should target master for new features like this. > Sure. I've just rebased against master. > Also rather than a monolithic patch could you post individual patches, see > > http://reductivelabs.com/trac/puppet/wiki/DevelopmentLifecycle > Sorry, I really should have read this document before submitting my patch. > > > The code is up at http://github.com/finalprefix/puppet/tree/0.24.7-win. > > > > I'd love any feedback you might have! > > How are you finding running it on windows, there are a bunch of path > issues I know that need to be cleaned up to run puppet. > Well Facter tries to run domainname and dnsdomainname, which don't exist on windows. I'll probably patch Facter in a bit. Further, I've been focused on the providers, and I've been running them using the standalone puppet command. At the moment, puppetd doesn't work because fork is not implemented in ruby on Windows. The win32-process gem has an implementation of fork for windows. Perhaps it will solve the problem. Daemonization on puppetd will also require puppet to be a windows service. Again, there's a gem for this called win32-service. I also need to figure out how to get non inheritable pipes for lines 78 and 79 of lib\puppet\external\event-loop\event-loop.rb: # prevent file descriptor leaks @notify_src.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) @notify_snk.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) I attempt to run puppet using "puppet test.pp", and the fcntl calls throws: c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\external\event-loop\event-loop.rb:78:in `initialize': uninitialized constant Fcntl::F_SETFD (NameError) c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\external\event-loop\event-loop.rb:31:in `new' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\external\event-loop\event-loop.rb:31:in `default' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\external\event-loop\event-loop.rb:35:in `current' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\external\event-loop\event-loop.rb:285:in `initialize' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util\settings.rb:532:in `new' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util\settings.rb:532:in `set_filetimeout_timer' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util\settings.rb:341:in `parse' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\application.rb:215:in `run' c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\bin\puppet:68 c:\ruby-1.8.7\bin\puppet:19:in `load' c:\ruby-1.8.7\bin\puppet:19 I tried commenting out those two lines, and things worked fine. >From what I read so far, using O_NOINHERIT on windows should be the equivalent of FD_CLOEXEC. However, IO.pipe makes the call to _pipe without this flag. I think it should be possible to make the call with O_NOINHERIT using the Win32API gem. > > If I get a chance this week I'll try and produce a combined tree > cherry-picking your stuff against master. > > Further comments inline > > > diff --git a/lib/puppet.rb b/lib/puppet.rb > > index 4b0091c..77dbdfe 100644 > > --- a/lib/puppet.rb > > +++ b/lib/puppet.rb > > @@ -190,53 +190,57 @@ module Puppet > > # Trap a couple of the main signals. This should probably be > > handled > > + begin > > + [:INT, :TERM].each do |signal| > > + trap(signal) do > > + Puppet.notice "Caught #{signal}; shutting down" > > + Puppet.debug "Signal caught here:" > > + caller.each { |l| Puppet.debug l } > > + Puppet.shutdown > > + end > > + end > ... > > + rescue ArgumentError => e > > + raise if e.to_s.index('SIGHUP') == nil > > end > > end > > What's the issue you're fixing here, I'm assuming its due to SIGHUP > not being defined on windows. There is probably a more explicit way of > handling this, as it's such a long method it makes it hard to see the > intent. > This part isn't necessary anymore, since the settrap call has gone from puppet.rb. > diff --git a/test/ral/providers/group_win.rb b/test/ral/providers/ > > OK we're moving to rspec for testing all new functionality should be > tested using rspec rather than Test::Unit, although tests appreciated. > Will do. > > If you need a hand with this I'm often on irc during UTC working hours > (nick nasrat) > > > diff --git a/test/ral/providers/syslog.rb b/test/ral/providers/ > > syslog.rb > > new file mode 100644 > > index 0000000..396b45c > > --- /dev/null > > +++ b/test/ral/providers/syslog.rb > > @@ -0,0 +1,4 @@ > > +# The purpose of this file is merely to fake the syslog module on > > windows. > > +# I placed it here since I was running the tests from here. > > +# I'll probably leave the file here until I wrap the windows event > > log in a syslogish module for windows. > > +# I haven't thought about it much. I'm open to better ideas too. > > I have an actual fix for not forcing loading syslog on windows, so we > can just force the log type in config for now. > > Again thanks > > Paul > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
