From 24b966685253de5bf6d68303877672935bf64a0c Mon Sep 17 00:00:00 2001 From: Rodrigo Chiossi Date: Fri, 18 May 2018 04:50:12 +0000 Subject: [PATCH] config: Let config handle filename Instead of relying on external sources to keep track of the active config file, store it as a private variable inside MixConfig. This change prevents inconsistencies regarding filename and also simplifies the code since the caller doesn't need to initialize the path or know the filename if it was not the one that initialized that config object. Signed-off-by: Rodrigo Chiossi --- builder/builder.go | 16 +--------- builder/bundles.go | 2 +- config/config.go | 76 +++++++++++++++++++++++++++++++-------------- mixer/cmd/config.go | 19 +----------- mixer/cmd/root.go | 3 +- mixin/helpers.go | 2 +- 6 files changed, 59 insertions(+), 59 deletions(-) diff --git a/builder/builder.go b/builder/builder.go index c90581d..30667e3 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -55,7 +55,6 @@ type Builder struct { Config config.MixConfig BuildScript string - BuildConf string MixVer string MixVerFile string @@ -103,7 +102,7 @@ func NewFromConfig(conf string) (*Builder, error) { if err := b.Config.LoadDefaults(false); err != nil { return nil, err } - if err := b.LoadBuilderConf(conf); err != nil { + if err := b.Config.LoadConfig(conf); err != nil { return nil, err } if err := b.ReadVersions(); err != nil { @@ -274,19 +273,6 @@ func (b *Builder) InitMix(upstreamVer string, mixVer string, allLocal bool, allU return nil } -// LoadBuilderConf will read the builder configuration from the command line if -// it was provided, otherwise it will fall back to reading the configuration from -// the local builder.conf file. -func (b *Builder) LoadBuilderConf(builderconf string) error { - var err error - b.BuildConf, err = config.GetConfigPath(builderconf) - if err != nil { - return err - } - - return b.Config.LoadConfig(b.BuildConf) -} - // ReadVersions will initialise the mix versions (mix and clearlinux) from // the configuration files in the version directory. func (b *Builder) ReadVersions() error { diff --git a/builder/bundles.go b/builder/bundles.go index daea511..a86bb6f 100644 --- a/builder/bundles.go +++ b/builder/bundles.go @@ -625,7 +625,7 @@ func (b *Builder) buildBundles(set bundleSet) error { // TODO: Do not touch config code that is in flux at the moment, reparsing it here to grab // information that previously Mixer didn't care about. Move that to the configuration part // of Mixer. - cfg, err := readBuildBundlesConfig(b.BuildConf) + cfg, err := readBuildBundlesConfig(b.Config.GetConfigFileName()) if err != nil { return err } diff --git a/config/config.go b/config/config.go index e23163a..39d0dae 100644 --- a/config/config.go +++ b/config/config.go @@ -39,6 +39,9 @@ type MixConfig struct { Swupd swupdConf Server serverConf Mixer mixerConf + + /* hidden properties */ + filename string } type builderConf struct { @@ -108,6 +111,8 @@ func (config *MixConfig) LoadDefaultsForPath(localrpms bool, path string) { config.Mixer.LocalRPMDir = "" config.Mixer.LocalRepoDir = "" } + + config.filename = filepath.Join(path, "builder.conf") } // CreateDefaultConfig creates a default builder.conf using the active @@ -121,21 +126,21 @@ func (config *MixConfig) CreateDefaultConfig(localrpms bool) error { return err } - filename, err := GetConfigPath("") + err := config.initConfigPath("") if err != nil { return err } - return config.SaveConfig(filename) + return config.SaveConfig() } // SaveConfig saves the properties in MixConfig to a TOML config file -func (config *MixConfig) SaveConfig(filename string) error { +func (config *MixConfig) SaveConfig() error { if !UseNewConfig { return errors.Errorf("SaveConfig can only be used with --new-config flag") } - w, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) + w, err := os.OpenFile(config.filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { return err } @@ -191,7 +196,7 @@ func (config *MixConfig) createLegacyConfig(localrpms bool) error { // SetProperty parse a property in the format "Section.Property", finds and sets it within the // config structure and saves the config file. -func (config *MixConfig) SetProperty(filename string, propertyStr string, value string) error { +func (config *MixConfig) SetProperty(propertyStr string, value string) error { if !UseNewConfig { return errors.Errorf("SetProperty requires --new-config flag") } @@ -214,7 +219,7 @@ func (config *MixConfig) SetProperty(filename string, propertyStr string, value if ok && tag == property { sectionV.Field(i).SetString(value) - return config.SaveConfig(filename) + return config.SaveConfig() } } @@ -224,7 +229,10 @@ func (config *MixConfig) SetProperty(filename string, propertyStr string, value // LoadConfig loads a configuration file from a provided path or from local directory // is none is provided func (config *MixConfig) LoadConfig(filename string) error { - if err := config.Parse(filename); err != nil { + if err := config.initConfigPath(filename); err != nil { + return err + } + if err := config.Parse(); err != nil { return err } if err := config.expandEnv(); err != nil { @@ -235,13 +243,13 @@ func (config *MixConfig) LoadConfig(filename string) error { } // Parse reads the values from a config file without performing validation or env expansion -func (config *MixConfig) Parse(filename string) error { +func (config *MixConfig) Parse() error { if !UseNewConfig { - if err := config.legacyParse(filename); err != nil { + if err := config.legacyParse(); err != nil { return err } } else { - if _, err := toml.DecodeFile(filename, &config); err != nil { + if _, err := toml.DecodeFile(config.filename, &config); err != nil { return err } } @@ -249,12 +257,12 @@ func (config *MixConfig) Parse(filename string) error { return nil } -func (config *MixConfig) legacyParse(filename string) error { +func (config *MixConfig) legacyParse() error { if UseNewConfig { return errors.Errorf("legacyParse is not compatible with --new-config flag") } - lines, err := helpers.ReadFileAndSplit(filename) + lines, err := helpers.ReadFileAndSplit(config.filename) if err != nil { return errors.Wrap(err, "Failed to read buildconf") } @@ -314,6 +322,11 @@ func (config *MixConfig) expandEnv() error { for i := 0; i < rv.NumField(); i++ { sectionV := rv.Field(i) + /* ignore unexported fields */ + if !sectionV.CanSet() { + continue + } + for j := 0; j < sectionV.NumField(); j++ { val := sectionV.Field(j).String() matches := re.FindAllStringSubmatch(val, -1) @@ -336,8 +349,13 @@ func (config *MixConfig) validate() error { rv := reflect.ValueOf(config).Elem() for i := 0; i < rv.NumField(); i++ { - sectionT := reflect.TypeOf(rv.Field(i).Interface()) sectionV := rv.Field(i) + /* ignore unexported fields */ + if !sectionV.CanSet() { + continue + } + + sectionT := reflect.TypeOf(rv.Field(i).Interface()) for j := 0; j < sectionT.NumField(); j++ { tag, ok := sectionT.Field(j).Tag.Lookup("required") @@ -359,20 +377,24 @@ func (config *MixConfig) validate() error { // Convert parses an old config file and converts it to TOML format func (config *MixConfig) Convert(filename string) error { - // Force UseNewConfig to false - UseNewConfig = false - if err := config.Parse(filename); err != nil { + if err := config.initConfigPath(filename); err != nil { return err } - if err := helpers.CopyFile(filename+".bkp", filename); err != nil { + // Force UseNewConfig to false + UseNewConfig = false + if err := config.Parse(); err != nil { + return err + } + + if err := helpers.CopyFile(config.filename+".bkp", config.filename); err != nil { return err } // Force UseNewConfig to true UseNewConfig = true - return config.SaveConfig(filename) + return config.SaveConfig() } // Print print variables and values of a MixConfig struct @@ -389,16 +411,24 @@ func (config *MixConfig) Print() error { return nil } -// GetConfigPath returns the default config path if the provided path is empty -func GetConfigPath(path string) (string, error) { +func (config *MixConfig) initConfigPath(path string) error { if path != "" { - return path, nil + config.filename = path + return nil } pwd, err := os.Getwd() if err != nil { - return "", err + return err } - return filepath.Join(pwd, "builder.conf"), nil + config.filename = filepath.Join(pwd, "builder.conf") + + return nil +} + +// GetConfigFileName returns the file name of current config +func (config *MixConfig) GetConfigFileName() string { + /* This variable cannot be public or else it will be added to the config file */ + return config.filename } diff --git a/mixer/cmd/config.go b/mixer/cmd/config.go index 831303b..7d02453 100644 --- a/mixer/cmd/config.go +++ b/mixer/cmd/config.go @@ -32,12 +32,6 @@ var configValidateCmd = &cobra.Command{ Long: `Parse a builder config file and display its properties. Properties containing environment variables will be expanded`, Run: func(cmd *cobra.Command, args []string) { - var err error - if configFile, err = config.GetConfigPath(configFile); err != nil { - // Print error, but don't print command usage - fail(err) - } - var mc config.MixConfig if err := mc.LoadConfig(configFile); err != nil { fail(err) @@ -57,11 +51,6 @@ var configConvertCmd = &cobra.Command{ a backup file of the old config and will replace it with the converted one. Environment variables will not be expanded and the values will not be validated`, Run: func(cmd *cobra.Command, args []string) { - var err error - if configFile, err = config.GetConfigPath(configFile); err != nil { - fail(err) - } - var mc config.MixConfig if err := mc.Convert(configFile); err != nil { fail(err) @@ -81,18 +70,12 @@ var configSetCmd = &cobra.Command{ if !config.UseNewConfig { fail(errors.New("config set requires `--new-config` flag`")) } - - var err error - if configFile, err = config.GetConfigPath(configFile); err != nil { - fail(err) - } - var mc config.MixConfig if err := mc.LoadConfig(configFile); err != nil { fail(err) } - if err := mc.SetProperty(configFile, args[0], args[1]); err != nil { + if err := mc.SetProperty(args[0], args[1]); err != nil { fail(err) } diff --git a/mixer/cmd/root.go b/mixer/cmd/root.go index 5d6ecd4..6935eb1 100644 --- a/mixer/cmd/root.go +++ b/mixer/cmd/root.go @@ -118,7 +118,8 @@ var initCmd = &cobra.Command{ fail(err) } } - if err := b.LoadBuilderConf(configFile); err != nil { + + if err := b.Config.LoadConfig(configFile); err != nil { fail(err) } err := b.InitMix(initFlags.clearVer, strconv.Itoa(initFlags.mixver), initFlags.allLocal, initFlags.allUpstream, initFlags.noDefaults, initFlags.upstreamURL, initFlags.git) diff --git a/mixin/helpers.go b/mixin/helpers.go index eea9135..84aa3ac 100644 --- a/mixin/helpers.go +++ b/mixin/helpers.go @@ -110,7 +110,7 @@ func setUpMixDir(upstreamVer, mixVer int) error { c.Swupd.Bundle = "os-core" c.Swupd.ContentURL = "file:///usr/share/mix/update/www" c.Swupd.VersionURL = "file:///usr/share/mix/update/www" - if err = c.SaveConfig(filepath.Join(mixWS, "builder.conf")); err != nil { + if err = c.SaveConfig(); err != nil { return err } err = ioutil.WriteFile(filepath.Join(mixWS, "mixversion"),