Automatic handling of deprecated values

[lanoxx]

Currently libconfuse has the option to deprecate config options and also to automatically drop deprecated config options. For deprecated config options that are not flagged to be dropped a warning is printed. For use cases where the user manually edits a config file this is fine, but for use cases where we want to perform automatic migration of deprecated values this is not enough.

I think it would be nice if we had the ability to add a deprecation callback handler in libconfuse which can notify the application of deprecated values in the configuration file. This would give the application a change to migrate these values automatically. The way I imagine this would be as following:

1. We add the CFGF_DEPRECATED to some existing but now obsolete config option, like this:

   CFG_BOOL("super_cow_power", FALSE, CFGF_DEPRECATED),
   CFG_BOOL("unicorn_power", TRUE, CFGF_NONE) /* use this instead */
   

2. We define a deprecation handler callback function type that can be implemented by the application. It gets passed the config pointer, the option_name of the deprecated option and an optional user_data pointer as arguments:

   typedef (int) (*DeprecationHandlerFunc) (cfg_t *config, 
                                            const char *option_name,
                                            void *user_data);
   

3. The deprecationFunc gets called for each deprecated value that remains in the config file. The app can handle the deprecated value and migrate it to some other option. By returning TRUE or FALSE from the deprecation function the app can indicate if the value was correctly migrated. If TRUE is returned no warning should be logged for the deprecated option, otherwise a warning will be logged (current behavior).

   int config_deprecation_handler (const char *option_name) {
       // load the config value from the config object and handle it
       if (migration_successful) {
          return TRUE; // migration successful
       } else {
           return FALSE;
       }
   }
   

4. If additionally the flag CFGF_DROP is specified and the deprecation handler returned TRUE, then the option is automatically removed.

5. When we call cfg_parse, we pass additional parameters with the deprecation_handler function and the user_data as callback:

   int result = cfg_parse (tc, config_file, deprecation_handler,
                           deprecation_user_data);
   

Please let me know what I think. Maybe I missed something, or if if this is already possible in libconfuse then it would be nice to know how.

[troglobit]

Sounds like a great idea! For reference, I use this in the In-a-dyn project to handle a deprecated setting https://github.com/troglobit/inadyn/blob/master/src/conf.c#L62 having a callback instead would be preferable.

Only comments I have are with style requests for your upcoming PR:

1. Please avoid UseOfCamelCase
2. Use /* C style comments */
3. You don't need to use { .. } for one-liner if-statements.
4. If the previous if-statement returns there is no need for an else
5. Initial function { go on its own line after the definition

Basically, it's Linux Coding Style.

[troglobit]

Any progress on this?

[lanoxx]

I don't have any time to work on this at the moment.

[troglobit]

@lanoxx Fully understandable, just curious, take care! :)

[peda-r]

I have been using the new CFGF_MODIFIED flag to accomplish option migration. Something like this:

	cfg_opt_t *dusty = cfg_getopt(cfg, "super_cow_power");
	cfg_opt_t *shiny = cfg_getopt(cfg, "unicorn_power");
	if (!dusty || !shiny)
		bugme();
	if (!(shiny->flags & CFGF_MODIFIED) && (dusty->flags & CFGF_MODIFIED))
		cfg_opt_setnbool(shiny, !cfg_opt_getnbool(dusty, 0), 0);

It would be nice to be able to follow that up with
cfg_rmopt(cfg, "super_cow_power");
but that api does not exist.

[troglobit]

@peda-r Hey, that's a great idea!

[peda-r]

If you are referring to the cfg_rmopt() part, then remove is pretty easy to do for "normal" options (I have a local patch). It gets difficult if you want to remove something that is below a multi-section that is possibly "instantiated" multiple times. You would have to walk the whole cfg-tree for that, which fits poorly with the style of rest of the api as most things (all?) are able to treat a subtree as an independent thing. It also gets a bit difficult with naming. Should cfg_rmopt(cfg, "multi=3|subopt"); be illegal or should it do the same as cfg_rmopt(cfg, "multi|subopt"); (which I think is the only sane notation).

I don't want to think about what can go wrong if the instantiated options in cfg_opt_t->values do not match the template options in cfg_opt_t->subopts, which is inevitably what is going to happen if cfg_rmopt() is used incorrectly with a subtree as base (and therefore not being able to walk the whole tree).