svType != tvType during config merge when keys can have different types

[Envek]

In Lefhook git hooks manager we had ability to skip some git hooks in user's local config. That was done via skip boolean setting in the config where usually it wasn't specified in main config defaulting to false, but in local config user could set it to true. However, reverse situations also exists in the wild.

Recently we decided to allow users not only skip hooks always (true) or never (false) but also by more complex conditions (e.g. during rebase and merge). This means, that this setting now can have different types of values in different configs: either boolean, string, or list of strings.

However, when we tried to merge configs with these “multitype” setting, values were ignored. I'm not sure whether it is a bug in Viper (I couldn't find any reasoning for the current implementation), but at least surprising behavior. If there is a way to properly handle this situation without changing Viper, please let us know!

Expected behavior (what you expected to happen):
When I merge two configs with keys of different types, values are being overriden by values from merging config.

Actual behavior (what actually happened):
New values are ignored with following messages printed to stderr:

ERROR 2021/05/22 12:16:28 svType != tvType; key=skip, st=string, tt=bool, sv=rebase, tv=true
ERROR 2021/05/22 12:16:28 svType != tvType; key=skip, st=[]interface {}, tt=bool, sv=[rebase merge], tv=true

Repl.it link:
https://replit.com/@Envek/Viper-example#main.go

Code reproducing the issue:

viper := viper.New()
viper.AddConfigPath(".")

viper.SetConfigName("lefthook")
_ = viper.ReadInConfig()

viper.SetConfigName("lefthook-local")
_ = viper.MergeInConfig()
	
fmt.Printf("\nAll configuration: %+v\n\n", viper.AllSettings())

Configuration files:

lefthook.yml

post-checkout:
  piped: true
  scripts:
    01-bundle-checkinstall:
      skip: true
    02-db-migrate:
      skip: true
    03-crystalball-update:
      skip: true

lefthook-local.yml

post-checkout:
  scripts:
    01-bundle-checkinstall:
      skip: false
    02-db-migrate:
      skip: rebase
    03-crystalball-update:
      skip:
        - rebase
        - merge

Environment:

• Viper version: 1.7.1
• Config source: file
• File format: YAML

Anything else we should know?:
Pull request in Lefthook where this behavior was discovered (more context can be found here): evilmartians/lefthook#173
Similar issue in Viper: #1134
Commit that introduces code of interest: 991d18a

And thank you for Viper!

[github-actions]

👋 Thanks for reporting!

A maintainer will take a look at your issue shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news,
either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

[sagikazarmark]

@Envek I've tested the patch that I've just merged into master and it seems to be working with your repl.