git-like config: Allow .git/lab/lab config to override ~/.config/lab/lab

[prarit]

As stated in https://github.com/zaquestion/lab/pull/414 I'd like to allow overrides between the global (~/.config/lab/lab) and local git-tree (.git/lab/lab) configs. There is a problem with the way viper works that prevents us from doing this and viper would need to be modified with a new function.

Suppose I have two config files with the same name but in different directories:

.first/lab with contents

system]
        name = "first"

[override]
        addr = "10.0.0.1"

and .second/lab with contents

[system]
        name = "second"

and the following program that attempts to merge the configs:

package main

import (
        "fmt"

        "github.com/spf13/viper"
)

const defaultGitLabHost = "https://gitlab.com"

func main() {
        viper.SetConfigType("toml")

        // .first/lab
        viper.SetConfigName("lab")
        viper.AddConfigPath(".first")
        viper.ReadInConfig()

        // .second/lab
        viper.SetConfigName("lab")
        viper.AddConfigPath(".second")
        viper.MergeInConfig()

        fmt.Println(viper.GetString("system.name"))
        fmt.Println(viper.GetString("override.addr"))
        viper.Reset()
}

The expected output is:

second
10.0.0.1

However the actual output is

first
10.0.0.1

This occurs because of the way viper searches for the config file: The two AddConfigPath() calls results in a search path of [.first .second] for a file 'lab'. MergeInConfig() will merge in the first match and it will not be .second/lab, but .first/lab. IOW, because the file name is the same the .second/lab is never found by MergeInConfig.

The only way to fix this is to add functionality for a RemoveConfigPath() to viper:

// RemoveConfigPath removes a path for Viper to search for the config file in.
// Can be called multiple times to remove multiple search paths.
func RemoveConfigPath(out string) { v.RemoveConfigPath(out) }
func (v *Viper) RemoveConfigPath(out string) {
        if out == "" {
                return
        }
        absout := absPathify(out)
        jww.INFO.Println("removing", absout, "from paths to search")
        if stringInSlice(absout, v.configPaths) {
                for i, path := range v.configPaths {
                        if path == absout  {
                                v.configPaths = append(v.configPaths[:i], v.configPaths[i+1:]...)
                                break
                        }
                }
        }
}

I thought about changing the code for MergeInConfig(), however, deemed it too risky to existing users to change the behavior to a last-found-first approach.

In any case I'm going to have to solve the viper issue before I can solve number 3 in #407.

[bmeneg]

@prarit, two things:

1. I think your message's body is malformatted because you placed a '[' before the quotation mark "```" right in the first code :)
2. what if we proactively search for the existence of the files before passing it to viper? I mean, we know what are the config levels we have: user level with the config file in $HOME/.config/lab/ and the project level with .git/lab/, thus:

• the code search for the user level, iff it exists, pass it to viper (SetConfigName() + ReadInConfig() maybe?) and place all config options there available in a structure
• the code then searches for the project level, iff it exists, pass it to viper again and update all config options there available in the same structure.
• rely on the "config" structure we've created and updated, instead of calling viper.Get__() throughout lab's code.

Of course it would be better to have the option of using viper to do that, but what do you think about this approach while viper doesn't have it?

[prarit]

@prarit, two things:

1. I think your message's body is malformatted because you placed a '[' before the quotation mark "```" right in the first code :)

Hah :) Thanks. I didn't have a chance to go back and think about it.

2. what if we proactively search for the existence of the files before passing it to viper? I mean, we know what are the config levels we have: user level with the config file in $HOME/.config/lab/ and the project level with .git/lab/, thus:


* the code search for the user level, iff it exists, pass it to viper (`SetConfigName()` + `ReadInConfig()` maybe?) and place all config options there available in a structure

* the code then searches for the project level, iff it exists, pass it to viper again and update all config options there available in the same structure.

* rely on the "config" structure we've created and updated, instead of calling `viper.Get__()` throughout lab's code.

I think that would work. I was debating about using viper.UnMarshall() in the #414 but wanted to concentrate on using toml first.

Of course it would be better to have the option of using viper to do that, but what do you think about this approach while viper doesn't have it?

[prarit]

After looking at the viper code, this works for my config examples above.

package main

import (
        "bytes"
        "fmt"
        "log"

        "github.com/spf13/viper"
        "github.com/spf13/afero"
)

const defaultGitLabHost = "https://gitlab.com"

func main() {
        viper.SetConfigType("toml")

        // .first/lab
        viper.SetConfigName("lab")
        viper.AddConfigPath(".first")
        viper.ReadInConfig()

        file, err := afero.ReadFile(afero.NewOsFs(), ".second/lab")
        if err != nil {
                log.Fatal(err)
        }

        viper.MergeConfig(bytes.NewReader(file))
        fmt.Println(viper.GetString("system.name"))
        fmt.Println(viper.GetString("override.addr"))
        viper.Reset()
}

[bmeneg]

Ah, nice! Simpler and cleaner :)

[prarit]

Today there are three ways to get core config data:

1. Override via LAB_ env values
2. "dot" (ie the local directory)
3. ~/.config/lab

I propose to change this to a more git-like experience of

1. Override via LAB_ env values
2. Optional --config <config_name> parameter that is NOT overriden by any other config
3. ~/.config/lab/
4. work tree local .git/lab/

This would allow, for example, a ~/.config/lab/lab.toml of

[core] host = "gitlab.com" user = "prarit" token = "abcde12345"

comments = "true"

and a .git/lab/show_metadata.toml of

comments = "false"

then the .git/lab/show_metadata.toml would override the ~/.config/lab/lab.toml setting of comments, ie) comments would be "false"

I'm working on some code atm and should have a MR available in the next few days as a WIP.

[zaquestion]

Just for the record, in the config overall you added loading of config files from the .git directory https://github.com/zaquestion/lab/blob/master/internal/config/config.go#L248-L250

I think this issue and the above comment is entirely related to the desired new merge functionality?

[prarit]

Yup, this issue and the above comment is related to the desired new merge functionality.

[zaquestion]

@prarit where did we land on this?

[bmeneg]

I think it was solved with 7dcf390188c61d8db988254a35c6bd439b779844, wasn't it @prarit ?