[MERGE REVIEW] Remove ConfigObj validation

Martin Pool mbp at canonical.com
Sun Jun 11 03:46:22 BST 2006


On  8 Jun 2006, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> John Meinel has observed that importing the ConfigObj validator was
> taking a significant portion of startup time.  Since the validator is an
> optional component, and we're not using it, this patch deletes it.
> 
> It also adds a benchmark for startup time.  According to the benchmark,
> this cuts our startup time from 107 ms to 47 ms.

Jolly good.  I think this merged already but I have one comment:

> +    def run_bzr_external(self, *args, **kwargs):
> +        bzr_path = os.path.dirname(os.path.dirname(bzrlib.__file__))+'/bzr'
> +        if len(args) == 1:
> +            args = shlex.split(args[0])
> +        args = list(args)
> +        process = Popen([bzr_path]+args, stdout=PIPE, stderr=PIPE)
> +        out = process.stdout.read()
> +        err = process.stderr.read()
> +        retcode = process.wait()
> +        supplied_retcode = kwargs.get('retcode')
> +        if supplied_retcode is not None:
> +            assert supplied_retcode == retcode
> +        else:
> +            assert retcode == 0
> +        return [out, err]
> +

+1 to having something like this added back in; it is useful in rare
cases.  Perhaps 'run_bzr_subprocess' would be better?  I do think it
should have a docstring:

  Run bzr in a subprocess for testing.

  This starts a new Python interpreter and runs bzr in there. 
  This should only be used for tests that have a justifiable need for
  this isolation: e.g. they are testing startup time, or signal
  handling, or early startup code, etc.  Subprocess code can't be 
  profiled or debugged so easily.

Robert (iirc) expressed the other day a dislike of run_bzr things that
split up strings; they're prone to split the wrong way.  (Though using
shlex might help.)  Also we have a general principle against magic-dwim
on arguments.  Since this should be called rarely I would rather just
delete the splitting of args and make the caller be explicit.

It may be better to run sys.executable, which will get the same Python
interpreter and might help on Windows.

Since retcode is the only kwarg why not just take it explicitly?

-- 
Martin




More information about the bazaar mailing list