Fixes #182 and #131

[amontalban]

Hi,

This is a fix for issues #182 and #131 , this fix was confirmed by @dkarlovi.

Thanks!

[amontalban]

Ping @seth-reeser

[amontalban]

Ping?

[amontalban]

Hey @seth-reeser and @stevebritton, are you still working on this project? Do you need help with it?

Thanks!

[electrofelix]

Being familiar with how a number of other plugins accomplish this, I'm pretty certain it's only supposed to be needed for complex types such as hashes, arrays, or attributes that are based on multiple settings. So I would have thought that only @aliases would have needed to be merged, and perhaps this is masking some other issue?

[amontalban]

Hi @electrofelix,

Thanks for your feedback, based on your comment I see that I forgot to add a merge for @aliases, will try to update the PR ASAP.

Regarding the PR itself I'm not a Ruby developer (Nor developer at all) but this patch is to fix the issue when you configure vagrant-hostmanager in the base box and you just do:

vagrant init my/basebox
vagrant up

With this patch you can configure vagrant-hostmanager in the Vagrantfile included in the box and when you do vagrant up it will merge configurations (In case you have configured it in the local Vagrantfile).

Hope it clarifies why I tried to do this.

Thanks!

[electrofelix]

Fully understand there is an issue around it not merging. The files merged are supposed to be from the box, from the current path and from the users global space ~/.vagrant.d/Vagrantfile.

What I'm saying is that this works out of the box for simple objects for other vagrant plugins, so likely missing something else about how vagrant-hostmanager has been coded in that it's not working by default for the simple true/false settings.

Should only require custom merging for the additional complex data elements that cannot necessarily merge correctly. I'd certainly think it's worth looking closer at the config file that is being used in the snipet below for lib/vagrant-hostmanager/util.rb since taking the config section directly from env.vagrantfile looks suspicious to me.

[electrofelix]

I'd have to dig and test a few things to be sure, but I suspect the reason the custom merge is required for all attributes is that the wrong config object is being accessed by default, taking the one only from workspace Vagrantfile instead of taking the merged config object for the current machine context.

[electrofelix]

Fully understand there is an issue around it not merging. The files merged are supposed to be from the box, from the current path and from the users global space ~/.vagrant.d/Vagrantfile.

What I'm saying is that this works out of the box for simple objects for other vagrant plugins, so likely missing something else about how vagrant-hostmanager has been coded in that it's not working by default for the simple true/false settings.

Should only require custom merging for the additional complex data elements that cannot necessarily merge correctly. I'd certainly think it's worth looking closer at the config file that is being used in the snipet below for lib/vagrant-hostmanager/util.rb since taking the config section directly from env.vagrantfile looks suspicious to me.

[electrofelix]

I suspect this function is part of the problem, in that it's not actually retrieving the correct config block. Rewriting this as the following might solve the problem fully, but I'd need to test to confirm:

def self.get_config(env)
    if Gem::Version.new(::Vagrant::VERSION) >= Gem::Version.new('1.5')
        env[:machine].config
    else
        env[:machine].env.config_global
    end
end

Then change all calls to pass in:

@config = Util.get_config(env)

Using the env that is passed to call(app, env) as opposed to passing in env[:machine].env. This should return the merged config object correctly scoped to have merged not only from individual Vagrantfiles in the correct order, but additionally any provider override blocks defined.