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

variable assingment order inconsistency when using file| in set_env #3335

Open
piotr-kubiak opened this issue Aug 29, 2024 · 1 comment
Open
Labels
bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@piotr-kubiak
Copy link

When not using file| in set_env, the variables are assigned in order (with the last value replacing any previous one, as in FOO being overwriten with QUX in my example1).

However, this behaviour is not consistent when using file|, when assingment of variables from file always take precedence (see example2, where FOO gets the value BAR from .env, and is not overwriten by subsequent assignment of QUX).

Expected:

  • consistent beavhiour, so that the last assingment of variable takes precedence, regardles if the previous one was set directly or using file|
  • or at least document the priority of variables when using file|.

tox.ini:

[tox]
env_list = example1,example2

[testenv]
allowlist_externals = bash
commands =
    bash -c 'env | egrep "FOO"'

[testenv:example1]
set_env = 
    FOO=BAZ
    FOO=QUX

[testenv:example2]
set_env = 
    FOO=BAZ
    file|.env
    FOO=QUX

.env:

FOO=BAR

Output:

$ tox -v
4.18.0

$ tox -e example1
FOO=QUX

$ tox -e example2
FOO=BAR
@gaborbernat gaborbernat added bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. labels Sep 8, 2024
@pachewise
Copy link

The regular assignments are processed in SetEnv.__init__, while the file assignments happen in use_replacer, which is called during by build(). Would we be OK with loading these at the same time, either in the __init__ or somewhere further down the line? To me, that seems like the easiest way to make sure that order of the assignments is taken into account, and I'm leaning toward putting them all in the __init__, but doing so feels like we'd be making the constructor "bulky".

I also just started looking at this code, so I am not sure if there was a reason that the env files are loaded after. Original PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

No branches or pull requests

3 participants