Comment 8 for bug 832061

Revision history for this message
John A Meinel (jameinel) wrote :

bzrlib.groupcompress.GroupCompressVersionedFiles._DEFAULT_MAX_BYTES_TO_INDEX
Is the *default*, but the value that is used at runtime is:

    def _get_compressor_settings(self):
        if self._max_bytes_to_index is None:
            # TODO: VersionedFiles don't know about their containing
            # repository, so they don't have much of an idea about their
            # location. So for now, this is only a global option.
            c = config.GlobalConfig()
            val = c.get_user_option('bzr.groupcompress.max_bytes_to_index')
            if val is not None:
                try:
                    val = int(val)
                except ValueError, e:
                    trace.warning('Value for '
                                  '"bzr.groupcompress.max_bytes_to_index"'
                                  ' %r is not an integer'
                                  % (val,))
                    val = None
            if val is None:
                val = self._DEFAULT_MAX_BYTES_TO_INDEX
            self._max_bytes_to_index = val

So *if* the user sets "bzr.groupcompress.max_bytes_to_index" in bazaar.conf, it supersedes the DEFAULT value. (I don't think we need to add a configuration to set the DEFAULT on top of being able to set the actual value, do you?)

"Similarly" _DEFAULT_MAX_FILE_SIZE:
    _DEFAULT_MAX_FILE_SIZE = 20000000
    _optionName = 'add.maximum_file_size'
    _maxSize = None

    def skip_file(self, tree, path, kind, stat_value = None):
        if kind != 'file':
            return False
        if self._maxSize is None:
            config = tree.branch.get_config()
            self._maxSize = config.get_user_option_as_int_from_SI(
                self._optionName,
                self._DEFAULT_MAX_FILE_SIZE)
...

So again the *DEFAULT* is a constant, but the actual value used is configurable with "add.maximum_file_size".

Yes, these could be rewritten to use the new config syntax. However, they are *already* configurable. So I don't understand whether this bug applies or not.

I don't think the locking stuff needs to be configurable. We could, but I think you're far of in diminishing returns at that point. (We've had the same value for ~3-5 years now.)