Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Nigel Kersten
On Mon, Jun 6, 2011 at 4:35 PM, R.I.Pienaar wrote: > > The ruby world wants gem extendible tools, I am not a huge fan but it is > convenient and it will make us look a bit less like a wart to a certain type > of hacker who might want to contribute so I thought going the gem route > while more wor

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread R.I.Pienaar
On 7 Jun 2011, at 00:27, Daniel Pittman wrote: > On Mon, Jun 6, 2011 at 16:24, R.I.Pienaar wrote: >> On 7 Jun 2011, at 00:12, Daniel Pittman wrote: >>> On Mon, Jun 6, 2011 at 15:43, Daniel Pittman wrote: On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: > >>> We have a cluster of other

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread James Turnbull
Daniel Pittman wrote: > One, we are concerned that there might be cases where this adds the > gem path, but not everything on the Ruby $LOAD_PATH, to the locations > our autoloader (or one of the other open-coded implementations of the > same) hunt for things on disk. That should be easy enough to

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Daniel Pittman
On Mon, Jun 6, 2011 at 16:24, R.I.Pienaar wrote: > On 7 Jun 2011, at 00:12, Daniel Pittman wrote: >> On Mon, Jun 6, 2011 at 15:43, Daniel Pittman wrote: >>> On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: >> We have a cluster of other issues in the area, though: we need to be >> able to plugi

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread R.I.Pienaar
On 7 Jun 2011, at 00:12, Daniel Pittman wrote: > On Mon, Jun 6, 2011 at 15:43, Daniel Pittman wrote: >> On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: >> So, add that comment and I am happy to see this go in. Throw my reviewed-by on the change and you can go ahead and merge it in

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread R.I.Pienaar
On 7 Jun 2011, at 00:12, Daniel Pittman wrote: > On Mon, Jun 6, 2011 at 15:43, Daniel Pittman wrote: >> On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: >> So, add that comment and I am happy to see this go in. Throw my reviewed-by on the change and you can go ahead and merge it i

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Daniel Pittman
On Mon, Jun 6, 2011 at 15:43, Daniel Pittman wrote: > On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: > >>> So, add that comment and I am happy to see this go in.  Throw my >>> reviewed-by on the change and you can go ahead and merge it into >>> master.  (I think you have the commit rights alrea

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Daniel Pittman
On Mon, Jun 6, 2011 at 13:57, R.I.Pienaar wrote: >> So, add that comment and I am happy to see this go in.  Throw my >> reviewed-by on the change and you can go ahead and merge it into >> master.  (I think you have the commit rights already, but if not I >> will chase Zach to get them for you.) >

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread R.I.Pienaar
- Original Message - > > So, add that comment and I am happy to see this go in. Throw my > reviewed-by on the change and you can go ahead and merge it into > master. (I think you have the commit rights already, but if not I > will chase Zach to get them for you.) > > Thanks for writin

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Daniel Pittman
On Mon, Jun 6, 2011 at 12:25, R.I.Pienaar wrote: > - Original Message - >> > >> > diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb >> > index cfeb73a..00c3b18 100644 >> > --- a/lib/puppet/configurer.rb >> > +++ b/lib/puppet/configurer.rb >> > @@ -156,9 +156,10 @@ class Pupp

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread markus
> > - # The list of directories to search through for loadable plugins. > > + # features uses the autoloader which causes a loop so avoid trying to > > + # ask Gem for its path during feature loading. > > + # > > + # We're using a per-thread cache based on the code in > > #module_directories

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread R.I.Pienaar
- Original Message - > > > > diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb > > index cfeb73a..00c3b18 100644 > > --- a/lib/puppet/configurer.rb > > +++ b/lib/puppet/configurer.rb > > @@ -156,9 +156,10 @@ class Puppet::Configurer > >       return > >     end > >   ensure

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-06 Thread Daniel Pittman
On Sun, Jun 5, 2011 at 01:12, R.I.Pienaar wrote: Hey, cool code. I have some questions that really deserve answers in the code, though, since I don't want to count on remembering the "why" in six months time: > This allows rubygems to be used to extend or override puppet behavior. > > Unfortuna

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-05 Thread markus
+1 On Sun, 2011-06-05 at 09:12 +0100, R.I.Pienaar wrote: > This allows rubygems to be used to extend or override puppet behavior. > > Unfortunately the features system uses the autoloader so checking for > the rubygems feature causes a call loop. It's possible that a more > elegant way to avoid

[Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-05 Thread R.I.Pienaar
This allows rubygems to be used to extend or override puppet behavior. Unfortunately the features system uses the autoloader so checking for the rubygems feature causes a call loop. It's possible that a more elegant way to avoid this exist rather than just checking the @path but this did what I n

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-05 Thread R.I.Pienaar
- Original Message - > > > Also, if you are going to follow the per-compile pattern of module_dir > > > (which seems reasonable) you should also clear the thread var (see > > > lib/puppet/configurer.rb around line 160) so that it doesn't get > > > stale. > > > > this is good I was wonde

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread markus
> > > > > > + if Puppet.features.rubygems? > > > > +Thread.current[:gemsearch_directories] ||= [] > > > > +Thread.current[:gemsearch_directories] = > > > > Gem.all_load_paths > > > > if Thread.current[:gemsearch_directories].empty? > > > > +return Thread.current[:gemse

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread R.I.Pienaar
- Original Message - > R.I. -- > > > > + if Puppet.features.rubygems? > > > +Thread.current[:gemsearch_directories] ||= [] > > > +Thread.current[:gemsearch_directories] = > > > Gem.all_load_paths > > > if Thread.current[:gemsearch_directories].empty? > > > +r

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread markus
R.I. -- > > + if Puppet.features.rubygems? > > +Thread.current[:gemsearch_directories] ||= [] > > +Thread.current[:gemsearch_directories] = Gem.all_load_paths > > if Thread.current[:gemsearch_directories].empty? > > +return Thread.current[:gemsearch_directories] > > +

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread R.I.Pienaar
- Original Message - > + if Puppet.features.rubygems? > +Thread.current[:gemsearch_directories] ||= [] > +Thread.current[:gemsearch_directories] = Gem.all_load_paths > if Thread.current[:gemsearch_directories].empty? > +return Thread.current[:gemsearch_direct

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread James Turnbull
R.I.Pienaar wrote: > This allows rubygems to be used to extend or override puppet behavior. > > Unfortunately the features system uses the autoloader so checking for > the rubygems feature causes a call loop. It's possible that a more > elegant way to avoid this exist rather than just checking th

[Puppet-dev] [PATCH/puppet 1/1] (#7788) Allow gem directories to be searched while autoloading

2011-06-04 Thread R.I.Pienaar
This allows rubygems to be used to extend or override puppet behavior. Unfortunately the features system uses the autoloader so checking for the rubygems feature causes a call loop. It's possible that a more elegant way to avoid this exist rather than just checking the @path but this did what I n