(#10802) add new function get_module_path #25
Conversation
|
I'm not sure I'd describe this as "the absolute path of a specified module"? Is this guaranteed to give the same results as the autoloader? I have concerns if this can return different info to the autoloader for situations like multiple modulepaths in the one environment. |
|
how would you describe it? I believe that this correctly searches the modulepath. I tried to use the same code that is used by the template function, so I think that it should work correctly. Do you have concerns b/c the template function returns different results than the auto-loader? (is there a ticket for that?) Could you be more specific about what use case you think will fail? |
|
It certainly sounds like the correct way to do it. Question is weither this is the canonical way of doing a module search: ... that is also used by the autoloader. At least your not traversing the config files knowledge of module path yourself ... which would be less canonical I'd imagine. |
|
Actually wrote something specific for customer trying to load of file out of module files dir instead of specifying full path to module using file() function: https://github.com/puppetlabs/customer-libadi/blob/master/lib/puppet/parser/functions/libadi_members.rb Puppet::Module.find() appears to be what template uses. I used Puppet[:environment] instead compiler.environment, is there a difference? In the case above, should we just update file function to support template type behavior? |
| require 'tempfile' | ||
| require 'fileutils' | ||
| describe Puppet::Parser::Functions.function(:get_module_path) do | ||
| def tmp_dir(name) |
There was a problem hiding this comment.
tmpdir/tmpfile is already in stdlib. You can just use that. It was nabbed from puppet.
There was a problem hiding this comment.
ah, I definitely did not reactor to re-use things from stdlib. I had written it to stand alone since I wasn't sure if it would be accepted into stdlib.
There was a problem hiding this comment.
I re-pushed to the branch. This should be resolved
|
I'm worried we could end up implementing a subtly different method of looking up module paths than the one the autoloader uses. I'd just like to see us be absolutely sure this isn't the case, and that this works exactly the same way with modulepaths in environments, particularly when you have multiple components in the modulepath and the same module exists in more than one component. If this isn't a concern at all, then full speed ahead. |
|
Here is the function that does it in Module: https://github.com/puppetlabs/puppet/blob/master/lib/puppet/module.rb#L144 And the one that does it in Autoload: https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/autoload.rb#L146 More or less the same procedure. I've tested this on a multi-module path setup with environments. And ran it through multiple ruby versions - except for Ruby 1.8.5 (which I can't get running on my mac). I'm cool with this part of it. But I still have comments to make. |
| get_scope.function_get_module_path(['foo']).should == @foo_path | ||
| end | ||
| it 'should be able to find module paths from the environment' do | ||
| @conf_file = Tempfile.new('conffile') |
There was a problem hiding this comment.
Your still using your own Tempfile implementation. Since you've removed the cleanup this will leave stray files ;-).
Also - why are you using an instance variable for this?
There was a problem hiding this comment.
that was a copy paste error, this is resolved in the latest branch
This commit adds a new function called get_module_path. get_module_path returns the absolute path of a specified module. The code and functionality is very similar to how templates and files are detected inside of modules. the function has been tested against puppet 2.6.10 and 2.7.x
(#10802) add new function get_module_path
This commit adds a new function called get_module_path.
get_module_path returns the absolute path of a specified module. The
code and functionality is very similar to how templates and files
are detected inside of modules.
the function has been tested against puppet 2.6.10 and 2.7.x