search

namecoin / nmcontrol

Namecoin Control. This repo is deprecated in favor of https://github.com/namecoin/ncdns
136 stars 38 forks source link

Fix a command line flag issue #29

Closed JeremyRand closed 10 years ago

JeremyRand commented 10 years ago

This should fix https://github.com/namecoin/nmcontrol/issues/26 . Can everyone please test this for regressions relating to command line option processing? I don't expect it to cause problems, but it definitely changes the data that services are receiving.

In particular, the most likely command line option to be adversely affected is --dns.disable_ns_lookups=0 and/or --dns.disable_ns_lookups=1 . That option seems to work fine for me. But additional testing would be appreciated.

JeremyRand commented 10 years ago

Suggested Tip4Commit award: 1%

phelixbtc commented 10 years ago

Will this set the options twice, both for services and plugins?

JeremyRand commented 10 years ago

@phelixbtc The problem was that services and plugins that have the same name conflicted. The previous behavior caused the DNS plugin to receive (along with intended options) unused config options (which were intended for the DNS service) and the DNS service to not receive any options at all. The new behavior causes both the DNS plugin and DNS service to receive all intended options, but also some unused options which don't affect anything. The only way this behavior would fail is if a plugin and a service both have the same name AND have a config option with the same name AND those two options need to have different values. That's not the case with the current code, and I can't see a reason why it would ever happen. This method was chosen because it preserves the command line option syntax, whereas giving the plugin and service different options would require changes to command line syntax.

phelixbtc commented 10 years ago

OK. Can we make it a rule from now on that services and plugins should have different names?

JeremyRand commented 10 years ago

Yes. I don't know why khal gave the DNS service and the DNS plugin the same name; IMO it was probably in error. I agree that going forward, new services and plugins should have unique names.

phelixbtc commented 10 years ago

Tested ok with my normal config. ACK