[PATCH mock] Enhancements of the config_opts['macros'] handling

[Michael E Brown]

On Mon, May 07, 2007 at 08:13:35PM +0200, Enrico Scholz wrote:
> Michael E Brown <Michael_E_Brown at dell.com> writes:
> 
> >> +    def _expand_macro_string(self, macros):
> >> +        res = []
> >> +        if isinstance(macros, dict):
> >> +            for (key,v) in macros.items():
> >> +                res.append('%s\t%s' % (key,v))
> >> +        elif isinstance(macros,  basestring):
> >> +            res = [macros]
> >> +        else:
> >> +            for v in macros:
> >> +                res.append(self._expand_macro_string(v))
> >> +
> >> +        return '\n'.join(res) + '\n'
> >
> > How does this affect the ability to overwrite config from a config file
> > higher in the heirarchy?
> 
> You can write e.g.
> 
> | -- defaults.cfg --
> | config_opts['macros'] = "%foo 1"

If something like this goes in, I expect that to become the defualt for
everything we ship. In this case:
    - setup_default_config_opts() would set up defaults using new format.
    - defaults.cfg probably wouldnt have this option
    - get rid of the recursive processing. If the option is a string,
      return it. If it is not a string, use dict processing model.

This means:
    - everything we ship uses new format
    - old users of this option still work but are encouraged to switch
      to the new format

Additionally:
    - We need a way to override options in subconfig files.

Suggestion:
    - We should use a dict for the option. If the hash value is None, or
      some variation thereof, dont

defaults.conf:
  config_opts['macros'] = { '%foo': "xyz", "%bar": "123", "%baz": "quux" }

my-root.conf:  # completely override all defaults
  config_opts['macros'] = { "%someopt": "someval" }

my-root.conf:  # only override some
  config_opts['macros']['%foo'] = "myvalue"
  config_opts['macros']['%bar'] = None      #dont output this in final
                                            #  .rpmmacros

--
Michael



> | 
> | -- my-root.cfg ==
> | config_opts['macros'] = [
> |   config_opts['macros'],
> |   {
> |     '%bar' : 2
> |   }
> | ]
> 
> Overwriting is not possible/easy when we want to retain backward
> compatibility.  When implemented from scratch, I would have implemented
> this option completely as a dictionary (I do not think that order of
> macros is for much interest).
> 
> 
> >
> >> +
> >>      def _build_dir_setup(self):
> >>          # purge the builddir, if it exists
> >>          bd_out = '%s%s' % (self.rootdir, self.builddir)
> >> @@ -788,8 +801,12 @@ class Root:
> >>          macrofile_out = '%s%s/.rpmmacros' % (self.rootdir, self.homedir)
> >>          if not os.path.exists(macrofile_out):
> >>              rpmmacros = open(macrofile_out, 'w')
> >> -            self.config['macros'] = self.config['macros'] + "\n%%_rpmlock_path	%s/var/lib/rpm/__db.000" % self.basedir
> >> -            rpmmacros.write(self.config['macros'])
> >> +
> >> +            rpmmacros.write(self._expand_macro_string(self.config['macros']))
> >> +            rpmmacros.write(self._expand_macro_string(
> >> +                {"%_rpmlock_path" : "%s/var/lib/rpm/__db.000" % self.basedir}
> >> +                ))
> >
> > This should go into the default config section, rather than hidden
> > here.
> 
> I just rewrote this hunk of code. In a later version of my mock
> modifications I removed this completely and replaced it by bind-mounting
> the chrooted /var/lib/rpm directory into toplevel /var/lib/rpm.
> 
> 
> > You need to convert the setup functions to use the new format by
> > default, as well as any existing config file references in the
> > default supplied configs.
> 
> See above for statement regarding backward compatibility; as long as
> you do not change the default string-type, old/existing configurations
> should still work.
> 
> When defaults would be dictionaries or vectors, overwriting can fail
> when people append an string.
> 
> 
> 
> 
> Enrico



> --
> Fedora-buildsys-list mailing list
> Fedora-buildsys-list at redhat.com
> https://www.redhat.com/mailman/listinfo/fedora-buildsys-list