Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{env:} macro is not expanded in setenv if the default value contains {envdir} #301

Closed
pytoxbot opened this issue Sep 17, 2016 · 3 comments · Fixed by #557
Closed

{env:} macro is not expanded in setenv if the default value contains {envdir} #301

pytoxbot opened this issue Sep 17, 2016 · 3 comments · Fixed by #557

Comments

@pytoxbot
Copy link

pytoxbot commented Sep 17, 2016

The following tox target will not expand {env:} macro with the default value (the substitution for {envdir}):

[testenv:dsvm-functional]
setenv = OS_ROOTWRAP_CMD={env:OS_ROOTWRAP_CMD:{envdir}}
commands =
  env
$ tox -e dsvm-functional
...
OS_ROOTWRAP_CMD={env:OS_ROOTWRAP_CMD:/home/vagrant/git/neutron-vpnaas/.tox/dsvm-functional}
...

Once I replace {envdir} with a hardcoded value, it expands {env:} correctly using the default value.

[testenv:dsvm-functional]
setenv = OS_ROOTWRAP_CMD={env:OS_ROOTWRAP_CMD:XXX}
commands =
  env
$ tox -e dsvm-functional
...
OS_ROOTWRAP_CMD=XXX
...
@obestwalter obestwalter added the bug label Nov 3, 2016
@obestwalter
Copy link
Member

I just tried the example in tox 2.4.1 and the problem still persists.

@eli-collins
Copy link

I got bit by this as well.

From my limited understanding of the code, it looks like Replacer().do_replace()'s regex is only matching against the inner {envdir} reference, and never does the outer subtitution. I tried to take a stab at fixing it, but can see why it's tricky to solve with just a regex, and not a proper parser.

I may take another stab at this at a later date, but for now, wanted to post the test function I worked up which exercises a few related cases:

def test_substitution_env_defaults_issue301(tmpdir, newconfig, monkeypatch):
    monkeypatch.setenv("IGNORE_STATIC_DEFAULT", "env")
    monkeypatch.setenv("IGNORE_DYNAMIC_DEFAULT", "env")
    config = newconfig("""
        [testenv:py27]
        passenv =
            IGNORE_STATIC_DEFAULT
            USE_STATIC_DEFAULT
            IGNORE_DYNAMIC_DEFAULT
            USE_DYNAMIC_DEFAULT
        setenv =
            OTHER_VAR=other
            IGNORE_STATIC_DEFAULT={env:IGNORE_STATIC_DEFAULT:default}
            USE_STATIC_DEFAULT={env:USE_STATIC_DEFAULT:default}
            IGNORE_DYNAMIC_DEFAULT={env:IGNORE_DYNAMIC_DEFAULT:{env:OTHER_VAR}+default}
            USE_DYNAMIC_DEFAULT={env:USE_DYNAMIC_DEFAULT:{env:OTHER_VAR}+default}
            IGNORE_OTHER_DEFAULT={env:OTHER_VAR:{env:OTHER_VAR}+default}
            USE_OTHER_DEFAULT={env:NON_EXISTENT_VAR:{env:OTHER_VAR}+default}
    """)
    conf = config.envconfigs['py27']
    env = conf.setenv
    assert env['IGNORE_STATIC_DEFAULT'] == "env"
    assert env['USE_STATIC_DEFAULT'] == "default"
    assert env['IGNORE_OTHER_DEFAULT'] == "other"
    assert env['USE_OTHER_DEFAULT'] == "other+default"
    assert env['IGNORE_DYNAMIC_DEFAULT'] == "env"
    assert env['USE_DYNAMIC_DEFAULT'] == "other+default"

@obestwalter
Copy link
Member

Hey @eli-collins thanks for sharing that. Mind to turn that into a PR that can serve as a reproducer that we'll XFAIL until we have it fixed?

vlaci pushed a commit to vlaci/tox that referenced this issue Jul 16, 2017
In case of a nested substitution values are expanded starting from the
innermost expression and continued until no successful substitutions
could be found.
vlaci pushed a commit to vlaci/tox that referenced this issue Jul 16, 2017
In case of a nested substitution values are expanded starting from the
innermost expression and continued until no successful substitutions
could be found.
obestwalter added a commit that referenced this issue Jul 16, 2017
config: expand nested substitutions (fixes #301)
@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants