Issue #150 on hold

posargs configerror

Edward Hope-Morley
created an issue

With tox version 1.7.0 I get the following error:

tox.ConfigError: ConfigError: substitution key 'posargs' not found

Which appears to be caused by the following line in my tox.ini:

commands = python setup.py test --slowest --testr-args='{posargs}'

This worked fine with the previous version of tox i.e. 1.6.1

Comments (36)

  1. Chris

    My read on this is that the CommandParser could probably do a better job of splitting out words. The contract of {posargs} is that it be a single-word replacement; in this instance, it's part of a "word" consisting of --testr-args='{posargs}', which is not going to work as written.

    We might be able to support equality notation there as well, by making some assumptions about the structure of CLI arguments, but I worry that those heuristics may bite us in the end.

    Basically, short answer: Changing the argument parser to make this work may have unexpected consequences; I'm leery of making that change unilaterally.

  2. Clark Boylan

    So the '=' thing is an easy work around. We can use -t '{posargs}' instead. But the handling of single quotes around {posargs} seems to have changed too... Using a different example command, say ls -l '{posargs}' and ls -l {posargs} should have different meanings but they don't. You can work around this by passing the quotes as part of the tox command eg tox -efoo -- 'notice the single quotes here'.

  3. Clark Boylan

    After more fiddling I have discovered by workaround isn't quite sufficient in cases where the option being modified by posargs requires arguments. In that case users of tox would be required to pass posargs on every command which is not user friendly. The only other option I can think of is to pass the entire string through as posargs. ls {posargs} then tox -efoo -- -l 'foo bar baz' which isn't very user friendly either.

    What is the possibility of updating the posargs parser to stop eating quotes? It really should preserve them.

  4. Chris

    Right. posargs is not supposed to be a quoted insertion into the commands in the tox command list, it's supposed to be an in-place insertion into the command. The distinction is perhaps a subtle one, but important. The placeholder is there to indicate the point in the list of command arguments into which we should insert the list of additional command line positional arguments before constructing the complete command.

    I think the gist of this is that what you are asking is something that the component you are using is not designed to do.

  5. Chris

    Clark, your workaround example should result in the shell equivalent of ls -l 'foo bar baz', right?

    so:

    tox config:

    command = ls -l {posargs}
    

    And given the CLI:

    tox -- 'foo bar baz'
    

    That should do what you expect; run ls -l 'foo bar baz'. If it doesn't, that is odd.

  6. Chris

    Right. posargs is not supposed to be a quoted insertion into the commands in the tox command list, it's supposed to be an in-place insertion into the command. The distinction is perhaps a subtle one, but important. The placeholder is there to indicate the point in the list of command arguments into which we should insert the list of additional command line positional arguments before constructing the complete command.

    I think the gist of this is that what you are asking is something that the component you are using is not designed to do.

  7. Clark Boylan

    Given command = ls -l {posargs} and tox -efoo -- 'foo bar baz' you get ls -l 'foo bar baz'.

    Given command = ls -l '{posargs}' and tox -efoo -- foo bar baz you get ls -l foo bar baz which is not expected.

    1.6.1 did the correct thing here and gave you ls -l 'foo bar baz' in both cases. 1.7 does not.

  8. Chris

    I'd consider the second version to be a bug. Certainly, it violates my expectations based on what I wrote, and it also breaks the contract of the code that implements posargs.

    Which is not to say that the behaviour in the case of standalone single-quoted posargs could not be considered, but that's not what the {posargs} placeholder is meant for; the command ls a list, and the {posargs} marker denotes the point in that list where the list of posargs entries from the command line should be inserted. Not substituted in a string.

  9. Chris

    While I concede that there is not a sufficient explanation of the quoting behaviour there, nothing in that disagrees with what I said.

    Anyway, the upshot of it is, a behaviour changed here. I personally think the old way was a bug, and it got inadvertently permitted by parsing logic that was fixed in 1.7.x. We could circle around on this all week. Right now, I'd suggest that we document the current (As of 1.7) quoting behaviour, and consider ways to extend the concept of substitution to permit the desired use case.

  10. jesusaurus

    The posargs substitution should be regular, not context-sensitive. Given command = ls -l '{posargs}' and tox -efoo -- foo bar baz the output should be ls -l 'foo bar baz'. The evaluation of {posargs} should not be dependent on any context, even if that context is within quotation marks.

  11. Holger Krekel repo owner

    I have read Clark Boylan 's PR (also thanks for digging up the commit context) and the issue comments here but am not sure at the moment how to best resolve this issue. We need a clear and predictable behaviour of substitutions and the fact that we are treating {posargs} as a list complicates matters i guess. What about adding a new substitution {posargs-string} which inserts " ".join(posargs) so that the original example in this issue would work like this:

    commands = python setup.py test --slowest --testr-args='{posargs-string}'
    
  12. Chris

    It feels clunky, but it's better than making the string usage impossible. It's a shame we aren't using a template engine in here :) We could use Jinja2-style pipe syntax:

    commands = python setup.py test --slowest --testr-args='{{posargs|join(' ')}}'
    

    That may be a wee bit too general purpose. The point remains, though; posargs-string will do ... what, exactly, with arguments that contain spaces? Will it quote them? Escape them? Ignore them?

  13. Chris

    That's the thing; the handling of whitespace really depends on context. In a shell command, escaping can take several forms -- single-quoting, double-quoting, -escaping -- to say nothing of other shells (I assume that Windows shells will do something different).

    How big of a change is someone willing to write in here? Because to handle the various whitespace scenarios we really need to have support for these:

    Given the command tox -- foo bar baz:

    ls -l {posargs}   --> ls -l foo bar baz
    ls -l '{posargs}'  --> ls -l 'foo bar baz' 
    ls -l "{posargs}" --> ls -l "foo bar baz"
    

    Given the command tox -- "foo bar" baz:

    ls -l {posargs}   --> ls -l "foo bar" baz
    
    # oooohkay... here's where I run out of ideas.
    
    ls -l '{posargs}'  --> ls -l '"foo bar" baz' ?
    ls -l "{posargs}" --> ls -l "foo\ bar baz"  ?
    

    The takeaway, shell quoting is hard. I don't think a one-size-fits-all solution will work.

  14. Clark Boylan

    Holger Krekel Something like {posargs-string} would restore the old behavior if I rewrite all of my tox.ini files. Ideally I wouldn't need to do that, but if tox is committed to not introducing a second backward incompatible change in as many releases I will have to live with that.

    Chris I don't think it is hard if you don't overthink it. The variable substitutions should be regular (IMO all of them should be not just special ones) then allow shlex to parse the resulting string. This makes the behavior consistent and predictable.

    I'm not sure I have time to update my pull request to support {posargs-string} prior to Friday (I think I would need to rewrite much of it to deal with the special case instead of the current behavior of that PR which is to apply the same rules to every variable substitution).

  15. Marc Abramowitz

    Personally, I think the most intuitive behavior (how I expected it to work and was surprised when it didn't work this way) was to do simple, dumb string substitution -- {posargs} gets replaced with your args, space-separated, plan and simple. This is intuitive and easy to understand and reason about. Quotes should never be eaten. As jesusaurus put it, the substitution should not be context-sensitive.

    I gravitate towards simple string substitution with no quote eating, along the lines of what Clark Boylan is suggesting. To me that is easy to implement without bugs and understandable. Trying to emulate bash or do anything complex I think is going to be fraught with peril and I don't see why it's necessary.

  16. Marc Abramowitz

    Holger Krekel mentioned bash on IRC as a possible model to follow since tox is a command executor.

    It just occurred to me that perhaps make is an even closer match, as a tox.ini is extremely similar to a Makefile in purpose and function. Bash has a wider scope than make, because it is used interactively and in scripts. Thus it has a lot of complex substitutions and even a full programming language. By contrast, make is used only for Makefiles and is much simpler, just giving a simple structure and simple variable substitutions.

    For example, take this Makefile:

    # Makefile
    
    POSARGS = --failing --parallel
    
    test:
        python setup.py test --slowest --testr-args='$(POSARGS)'
    
    $ make
    python setup.py test --slowest --testr-args='--failing --parallel'
    

    $(POSARGS) is replaced with the value of the variable. The quotes surrounding it are not touched.

    I think tox should function the same way but it doesn't. Note below that the quotes are stripped out:

    # testr.ini
    
    [testenv]
    commands =
        python setup.py test --slowest --testr-args '{posargs}'
    
    $ tox -c testr.ini -- --failing --parallel
    GLOB sdist-make: /Users/marca/dev/hg-repos/tox/setup.py
    python inst-nodeps: /Users/marca/dev/hg-repos/tox/.tox/dist/tox-1.7.1.zip
    python runtests: PYTHONHASHSEED='614422397'
    python runtests: commands[0] | python setup.py test --slowest --testr-args --failing --parallel
    ...
    

    Two problems in tox:

    1. I had to separate --testr-args and {posargs} with a space, because it doesn't work at all with an equals sign. That shouldn't be necessary.

    2. Even though I put single quotes around {posargs}, they were stripped out, effectively making '{posargs}' the same as {posargs}.

  17. Marc Abramowitz

    Here's another example that illustrates an earlier point that Clark Boylan made using the ls command.

    make

    # Makefile.ls_example
    
    POSARGS = A file with multiple words
    
    test:
        ls -l '$(POSARGS)'
        echo
        ls -l $(POSARGS)
    
    $ make -f Makefile.ls_example
    ls -l 'A file with multiple words'
    -rw-r--r--  1 marca  staff  0 Apr  3 19:44 A file with multiple words
    echo
    
    ls -l A file with multiple words
    ls: A: No such file or directory
    ls: file: No such file or directory
    ls: multiple: No such file or directory
    ls: with: No such file or directory
    ls: words: No such file or directory
    make: *** [test] Error 1
    

    The point here is that ls -l '$(POSARGS)' (quoted) and ls -l $(POSARGS) (not quoted) do different things.

    tox

     # ls_example.ini
    
    [testenv]
    whitelist_externals = ls
    commands =
        ls -l '{posargs}'
        ls -l {posargs}
    
    $ tox -c ls_example.ini -- "A file with multiple words"
    GLOB sdist-make: /Users/marca/dev/hg-repos/tox/setup.py
    python inst-nodeps: /Users/marca/dev/hg-repos/tox/.tox/dist/tox-1.7.1.zip
    python runtests: PYTHONHASHSEED='2518930073'
    python runtests: commands[0] | ls -l A file with multiple words
    -rw-r--r--  1 marca  staff  0 Apr  3 19:44 A file with multiple words
    python runtests: commands[1] | ls -l A file with multiple words
    -rw-r--r--  1 marca  staff  0 Apr  3 19:44 A file with multiple words
    

    Here, the quoted form and non-quoted form do exactly the same thing, so expressiveness has been lost because the substitution eats the quotes.

    I personally would rather have '{posargs}' have the old behavior again. It seems like it was probably that way a lot longer than it was the new way, which only surfaced in tox 1.7.0. Thus, there are likely to be more folks relying on the old semantics than the new semantics. I have seen that using --testr-args='{posargs}' is very common in a lot of OpenStack projects (the world from which Clark Boylan and Edward Hope-Morley are coming). OpenStack has a lot of projects and a lot of tox.inis I think that uses this.

    See:

    So I would say that it would be good to have '{posargs}' have the old behavior again, even if it wasn't the behavior intended by the author, because the old behavior is useful and is being relied upon. If folks want a different semantics, put that in a new variable ({posargs-list}?) or add a setting to switch on those semantics. But I don't really know what the quote-eating behavior is useful for -- I haven't seen a motivating example for that.

  18. Chris

    So, here's a summary of behaviour since 1.4. Details follow, but :

    1.4-1.6

    • tox called with 'quoted args':
    • command has 'quoted args': runs with 'quoted args'
    • command has unquoted args: runs with unquoted args
    • tox called with unquoted args:
    • command has 'quoted args': runs with 'quoted args'
    • command has unquoted args: runs with unquoted args

    1.7

    • tox called with 'quoted args':
    • command has 'quoted args': runs with 'quoted args'
    • command has unquoted args: runs with 'quoted args'
    • tox called with unquoted args:
    • command has 'quoted args': runs with unquoted args
    • command has unquoted args: runs with unquoted args

    A more complex example, with mixed quoting:

    tox-args.ini

    [testenv]
    whitelist_externals = ls
    commands =
        ls -l '{posargs}'
        ls -l {posargs}
    

    This setup exists:

    touch "a file" a-file

    tox 1.6

    tox -c tox-args.ini -- 'a file' a-file

    expectations

    • command 0: expect failure
    • command 1: expect success

    actual

    python runtests: commands[0] | ls -l a file a-file
    ls: a file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a file a-file'
    python runtests: commands[1] | ls -l a file a-file
    ls: a: No such file or directory
    ls: file: No such file or directory
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    ERROR: InvocationError: '/bin/ls -l a file a-file'
    

    tox -c tox-args.ini -- "'a file' a-file"

    expectations

    • command 0: expect failure
    • command 1: expect failure

    actual

    python runtests: commands[0] | ls -l a file a-file
    ls: a: No such file or directory
    ls: file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a file a-file'
    python runtests: commands[1] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    

    tox -c tox-args.ini -- a\ file a-file

    expectations

    • command 0: expect failure
    • command 1: expect success

    actual

    python runtests: commands[0] | ls -l a file a-file
    ls: a file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a file a-file'
    python runtests: commands[1] | ls -l a file a-file
    ls: a: No such file or directory
    ls: file: No such file or directory
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    ERROR: InvocationError: '/bin/ls -l a file a-file'
    

    tox -c tox-args.ini -- 'a\ file a-file'

    expectations

    • command 0: expect failure
    • command 1: expect failure

    actual

    python runtests: commands[0] | ls -l a\ file a-file
    ls: a\ file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a\\ file a-file'
    python runtests: commands[1] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    

    tox 1.7

    tox -c tox-args.ini -- 'a file' a-file

    expectations

    • command 0: expect failure
    • command 1: expect success

    actual

    python runtests: commands[0] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    python runtests: commands[1] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    

    tox -c tox-args.ini -- "'a file' a-file"

    expectations

    • command 0: expect failure
    • command 1: expect failure

    actual

    python runtests: commands[0] | ls -l 'a file' a-file
    ls: 'a file' a-file: No such file or directory
    ERROR: InvocationError: "/bin/ls -l 'a file' a-file"
    python runtests: commands[1] | ls -l 'a file' a-file
    ls: 'a file' a-file: No such file or directory
    ERROR: InvocationError: "/bin/ls -l 'a file' a-file"
    

    tox -c tox-args.ini -- a\ file a-file

    expectations

    • command 0: expect failure
    • command 1: expect success

    actual

    python runtests: commands[0] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    python runtests: commands[1] | ls -l a file a-file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a file
    -rw-r--r--  1 offby1  staff  0 Apr 14 09:28 a-file
    

    tox -c tox-args.ini -- 'a\ file a-file'

    expectations

    • command 0: expect failure
    • command 1: expect failure

    actual

    python runtests: commands[0] | ls -l a\ file a-file
    ls: a\ file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a\\ file a-file'
    python runtests: commands[1] | ls -l a\ file a-file
    ls: a\ file a-file: No such file or directory
    ERROR: InvocationError: '/bin/ls -l a\\ file a-file'
    
  19. Chris

    I have a proposed solution that should address the main concerns:

    pseudocode:

    for w in command.words:
        if w == posargs:
            append posargs as split words
        elif posargs in w:
            nw = replace posargs with escaped string posargs' first value
            append nw
            append remaining posargs
        else
            append w
    

    Proposed result matrix:

                    w       ['w']           w w w       ['w', 'w', 'w']         "w w" w         ['w w', 'w']
    cmd=[]          cmd=w   ['cmd=w']       cmd=w\ w\ w ['cmd=w w w']           cmd=w\ w w      ['cmd=w w', 'w']
    cmd []          cmd w   ['cmd', 'w']    cmd w w w   ['cmd', 'w', 'w', 'w']  cmd 'w w' w     ['cmd', 'w w', 'w']
    cmd '[]'        cmd w   ['cmd', 'w']    cmd 'w w w' ['cmd', 'w w w']        cmd "'w w' w"   ['cmd', "'w w' w"]
    
  20. Holger Krekel repo owner

    Chris do i understand correctly that this would make the open stack use case --testr-args='{posargs}' above work again? I am not sure because your proposed algorithm depends on how words are split, right?

    Marc Abramowitz i tend to agree that if we don't find a solution soon, we should just revert the behaviour and aim for something better in 1.8. I don't think it's going to break things too much because not too many people do wild things with posargs from my experience.

  21. Marc Abramowitz

    Yeah most tox.inis that I see either:

    1. Don't use posargs at all
    2. Use it posargs very simply: e.g.: py.test {posargs}

    So it probably doesn't matter a whole lot to most people.

    But OpenStack has a bazillion tox.inis that use the --testr-args='{posargs}' so they get impacted by this the most.

  22. Holger Krekel repo owner

    fix issue150: parse {posargs} more like we used to do it pre 1.7.0. The 1.7.0 behaviour broke a lot of OpenStack projects. See PR85 and the issue discussions for (far) more details, hopefully resulting in a more refined behaviour in the 1.8 series. And thanks to Clark Boylan for the PR.

    → <<cset acb4f0db1d40>>

  23. Log in to comment