'bzr config' fails to display config templates referring to "unknown" options

Bug #996401 reported by Wouter van Heyst on 2012-05-08
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Bazaar
High
Vincent Ladeuil

Bug Description

To set a custom mergetool `bzr help configuration` lists {base}, {this} etc to pass options to the tool.
Setting this works fine in 2.5, but retrieving it trips on option expansion. Bzr 2.4 works fine in this regard.

    >> bzr config bzr.mergetool.MyCoolMergeTool='mycoolmergetool --wait {this} {other} {result} {base}'
    >> ~/src/bzr/2.4/bzr config bzr.mergetool.MyCoolMergeTool
    mycoolmergetool --wait {this} {other} {result} {base}

    >> ~/src/bzr/2.5/bzr config bzr.mergetool.MyCoolMergeTool -Derror
    bzr: ERROR: bzrlib.errors.ExpandingUnknownOption: Option this is not defined while expanding "mycoolmergetool --wait {this} {other} {result} {base}".

    Traceback (most recent call last):
      File "/home/larstiq/src/bzr/2.5/bzrlib/commands.py", line 920, in exception_to_return_code
        return the_callable(*args, **kwargs)
      File "/home/larstiq/src/bzr/2.5/bzrlib/commands.py", line 1131, in run_bzr
        ret = run(*run_argv)
      File "/home/larstiq/src/bzr/2.5/bzrlib/commands.py", line 673, in run_argv_aliases
        return self.run(**all_cmd_args)
      File "/home/larstiq/src/bzr/2.5/bzrlib/commands.py", line 695, in run
        return self._operation.run_simple(*args, **kwargs)
      File "/home/larstiq/src/bzr/2.5/bzrlib/cleanup.py", line 136, in run_simple
        self.cleanups, self.func, *args, **kwargs)
      File "/home/larstiq/src/bzr/2.5/bzrlib/cleanup.py", line 166, in _do_with_cleanups
        result = func(*args, **kwargs)
      File "/home/larstiq/src/bzr/2.5/bzrlib/commands.py", line 1148, in ignore_pipe
        result = func(*args, **kwargs)
      File "/home/larstiq/src/bzr/2.5/bzrlib/config.py", line 4024, in run
        self._show_value(name, directory, scope)
      File "/home/larstiq/src/bzr/2.5/bzrlib/config.py", line 4064, in _show_value
        value = conf.get(name, expand=True)
      File "/home/larstiq/src/bzr/2.5/bzrlib/config.py", line 3680, in get
        value = expand_and_convert(value)
      File "/home/larstiq/src/bzr/2.5/bzrlib/config.py", line 3651, in expand_and_convert
        val = self._expand_options_in_string(val)
      File "/home/larstiq/src/bzr/2.5/bzrlib/config.py", line 3738, in _expand_options_in_string
        raise errors.ExpandingUnknownOption(name, string)
    ExpandingUnknownOption: Option this is not defined while expanding "mycoolmergetool --wait {this} {other} {result} {base}".

Related branches

Vincent Ladeuil (vila) on 2012-05-08
summary: - mergetool config clashes with config option expansion
+ 'bzr config' fails to display config templates referring to "unknown"
+ options
Vincent Ladeuil (vila) wrote :

'bzr config' does not expand the values displayed.

'bzr config <option>' tries to expand the value to present what bzr will use.

Since templates (rightly) use options that come from outside the config files, this fails.

I'm not quite sure what to do about that... suggestions ?

On one hand, the behaviour is sound, 'this' can't be expanded so the error is "correct".

On the other hand, one could imagine an additional command-line switch to explicitly control the expansion.

Martin Packman (gz) wrote :

Just on the general point though Vincent, how do you actually specify a working merge tool line? Wouter found that {{x}} also didn't work as escaping.

Vincent Ladeuil (vila) wrote :

> how do you actually specify a working merge tool line?

No expansion occurs when a value is set, so there is no problem there. This bug report is about displaying the value (which the first command set correctly).

> Wouter found that {{x}} also didn't work as escaping.

1) Rightly so, {{x}} with x = y expands as {y}, this is a supported feature.

2) Since setting a value doesn't try to expand it, there is nothing to escape from.

Overall, the doc should mention the difference between:

- 'bzr config <option>' which displays the expanded value for the option

- 'bzr config --all <regexp>' which displays the raw values

I.e.:

vila:~/src/bzr/trunk :) $ bzr config bzr.mergetool.MyCoolMergeTool='mycoolmergetool --wait {this} {other} {result} {base}'
vila:~/src/bzr/trunk :) $ bzr config bzr.mergetool.MyCoolMergeTool
bzr: ERROR: Option this is not defined while expanding "mycoolmergetool --wait {this} {other} {result} {base}".
vila:~/src/bzr/trunk :( $ bzr config --all bzr.mergetool.MyCoolMergeTool
branch:
  bzr.mergetool.MyCoolMergeTool = mycoolmergetool --wait {this} {other} {result} {base}

Gordon Tyler (doxxx) wrote :

That seems weirdly inconsistent. `bzr config` should not do expansion unless you give an option to do so.

Vincent Ladeuil (vila) wrote :

The use cases are:

- give me the value for an option: 'bzr config option'

- give me all the definitions for an option: 'bzr config --all option'

Does this sound more coherent ?

Gordon Tyler (doxxx) wrote :

Not really. What purpose does `bzr config <option>` server other than to show the user what the option is set to? For least surprise, it should show exactly what the user originally set.

$ bzr config foo=stuff
$ bzr config bar='my {foo}'
$ bzr config bar
  bar = my {foo}
$ bzr config --expand bar
  bar = my stuff

To me, that is a far more logical sequence.

Vincent Ladeuil (vila) wrote :

> What purpose does `bzr config <option>` server other than to show the user what the option is set to?

No other but that's an important use case not covered otherwise.

'bzr config --all option' serves a different purpose: it tells the user where the option is defined (in any config file) and how default/override definitions interact.

Also, 'bzr config option' is the shortest syntax *because* it's meant to be used in scripts:

   bzr push `bzr config mypush`

Forcing the user to use --expand won't be friendly.

Finally, 'bzr config' itself defaults to 'bzr config --all .*' and that's what people are expected to use to start with.

Gordon Tyler (doxxx) wrote :

Perhaps I lack imagination :) but I wouldn't have thought of using `bzr config option` to get the value to pass to another bzr command.

However, given that `bzr config` by itself does the right thing, that's probably okay.

Vincent Ladeuil (vila) on 2012-09-19
Changed in bzr:
assignee: nobody → Vincent Ladeuil (vila)
milestone: none → 2.6b3
status: Confirmed → Fix Released
Vincent Ladeuil (vila) on 2013-07-27
Changed in bzr:
milestone: 2.6b3 → 2.6.0
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers