Provision: ignore other test environments #2865

masenf · 2023-01-16T05:05:08Z

When running provision, ignore other environments, since their configuration might not be valid until running in the provisioned environment.

When creating provision environment, seed the loader early via memory_seed_loader to ensure it's available when the environment is created. The provision environment must be configured explicitly, it will not inherit from [testenv].

Fix #2862

Thanks for contribution

Please, make sure you address all the checklists (for details on how see
development documentation)!

ran the linter to address style issues (tox -e fix)
wrote descriptive pull request text
ensured there are test(s) validating the fix
added news fragment in docs/changelog folder
updated/extended the documentation

src/tox/session/env_select.py

gaborbernat · 2023-01-16T05:09:33Z

src/tox/session/env_select.py

@@ -241,13 +242,29 @@ def _finalize_config(self) -> None:
        self._state.conf.core.mark_finalized()

    def _build_run_env(self, name: str) -> RunToxEnv | None:
-        if self._provision is not None and self._provision[0] is False and name == self._provision[1]:
+        provision_on = provision_tox_env = provision_loader = None


I think we just need to change the base for the provision env.

not sure how to proceed on that one... but i'll think about it.

i'm off for the night here. it's been really fun hacking on tox today, thanks for the opportunity.

See

tox/src/tox/provision.py

Line 92 in a2222e9

base=[], # disable inheritance for provision environments

; we should already not use inheritance from testenv here, we need to find out why it still happens. Likely a bug somewhere.

Right in the middle of that now. I think it's related to caching the "base" value from the Ini before the provision loader is even added to env_conf.

> /Users/masen/code/tox-dev/tox/.tox/py311/lib/python3.11/site-packages/tox/session/env_select.py(259)_build_run_env() -> env_conf.loaders.insert(0, provision_loader) (Pdb) env_conf._defined["base"] ConfigDynamicDefinition(keys=('base',), desc=inherit missing keys from these sections, of_type=typing.List[str], default=['testenv'], _cache=['testenv']) (Pdb) env_conf.loaders [IniLoader(section=testenv, overrides={})]

If I reset the _cache values for the following keys after adding the provision_loader at index 0, then they can get the new values from the provision_loader.

(Pdb) tuple(env_conf._defined) ('set_env', 'setenv', 'base')

Something like

for config_def in env_conf._defined: if isinstance(config_def, ConfigDynamicDefinition): config_def._cache = _PLACE_HOLDER

But this actually doesn't work, because the loader for the testenv section is already in the chain at this point; removing it expicitly would be messy.

So the approach I'm taking now is to update Config.get_env to accept a base parameter, and if that's passed through it overrides the defaults. I couldn't find a way to reliably NOT include the base via config only, since shimming in the provision_loader occurs after Config.get_env, so the testenv default base just gets baked into the env's loaders.

I think instead insert we need to use https://github.com/tox-dev/tox/blob/main/src/tox/session/cmd/devenv.py#L38, as it's too late for inserting the loader 🤔

gaborbernat · 2023-01-16T19:08:31Z

src/tox/session/env_select.py

@@ -241,13 +241,29 @@ def _finalize_config(self) -> None:
        self._state.conf.core.mark_finalized()

    def _build_run_env(self, name: str) -> RunToxEnv | None:
-        if self._provision is not None and self._provision[0] is False and name == self._provision[1]:
+        provision_on = provision_tox_env = provision_loader = None


I think this file needs to be untouched, only the provision.py should be changed.

A testenv depends on a runner supplied via a plugin that tox needs to provision. Added the plugin to demo_pkg_inline since it seemed like the easiest place.

Ensure that the provision environment config is available _before_ attempting to load it, otherwise `base` will take the default resulting in values from the testenv being inadvertently inherited.

When running provision, ignore other environments, since their configuration might not be valid until running in the provisioned environment. Fix tox-dev#2862

Now that the loader is seeded via state.conf.memory_seed_loaders, the provision loader no longer needs to be passed through this interface.

masenf · 2023-01-16T20:11:18Z

provision: seed loader with state.conf.memory_seed_loaders

Small change that fixes the base inheritance... with this change, the provisioning environment no longer implicitly inherits from testenv.

env_select: ignore other env configurations when provisioning

This change is necessary to prevent accidentally loading configuration from other environments during provisioning.

env_select: _mark_provision: remove loader argument

This commit is just a cleanup, now that the loader is being pre-seeded, it doesn't need to be passed via _mark_provision.

Hopefully with a smaller diff, this is easier to review.

This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [tox](https://togithub.com/tox-dev/tox) ([changelog](https://tox.wiki/en/latest/changelog.html)) | dev | patch | `4.3.2` -> `4.3.3` | `4.4.2` (+4) | --- ### Release Notes <details> <summary>tox-dev/tox</summary> ### [`v4.3.3`](https://togithub.com/tox-dev/tox/releases/tag/4.3.3) [Compare Source](https://togithub.com/tox-dev/tox/compare/4.3.2...4.3.3) #### What's Changed - Provision: ignore other test environments by [@masenf](https://togithub.com/masenf) in [tox-dev/tox#2865 **Full Changelog**: tox-dev/tox@4.3.2...4.3.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box  Co-authored-by: descope[bot] <descope[bot]@users.noreply.github.com>

masenf requested a review from gaborbernat as a code owner January 16, 2023 05:05

psf-chronographer bot added the bot:chronographer:provided label Jan 16, 2023

masenf changed the title ~~Provision: ignore missing runners~~ Provision: ignore other test environments Jan 16, 2023

gaborbernat requested changes Jan 16, 2023

View reviewed changes

masenf force-pushed the provision-ignore-runner-error branch from 3dad658 to cb7051a Compare January 16, 2023 18:17

masenf marked this pull request as draft January 16, 2023 18:27

gaborbernat reviewed Jan 16, 2023

View reviewed changes

masenf added 5 commits January 16, 2023 11:48

test_provision_plugin_runner: testenv depends on plugin-defined runner

1672e2f

A testenv depends on a runner supplied via a plugin that tox needs to provision. Added the plugin to demo_pkg_inline since it seemed like the easiest place.

test_provision: new test for explicit .tox env override

da55c69

provision: seed loader with state.conf.memory_seed_loaders

df48a67

Ensure that the provision environment config is available _before_ attempting to load it, otherwise `base` will take the default resulting in values from the testenv being inadvertently inherited.

env_select: ignore other env configurations when provisioning

78a4970

When running provision, ignore other environments, since their configuration might not be valid until running in the provisioned environment. Fix tox-dev#2862

env_select: _mark_provision: remove loader argument

32ec2b0

Now that the loader is seeded via state.conf.memory_seed_loaders, the provision loader no longer needs to be passed through this interface.

masenf force-pushed the provision-ignore-runner-error branch from cb7051a to 32ec2b0 Compare January 16, 2023 20:00

docs/upgrading.rst: add note about provisioning env and setenv

3eb9f4a

changelog: entry for issue 2862

4f00c15

gaborbernat approved these changes Jan 16, 2023

View reviewed changes

gaborbernat marked this pull request as ready for review January 16, 2023 22:08

gaborbernat merged commit e4c65bb into tox-dev:main Jan 16, 2023

mikelane mentioned this pull request Jan 17, 2023

Bump tox from 4.3.0 to 4.3.3 mikelane/reddit-get#135

Merged

masenf deleted the provision-ignore-runner-error branch January 18, 2023 04:25

masenf mentioned this pull request Jan 18, 2023

[testenv] install_command is also used for provision environment #2428

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provision: ignore other test environments #2865

Provision: ignore other test environments #2865

masenf commented Jan 16, 2023 •

edited

gaborbernat Jan 16, 2023

masenf Jan 16, 2023

gaborbernat Jan 16, 2023

masenf Jan 16, 2023

gaborbernat Jan 16, 2023

gaborbernat Jan 16, 2023

masenf commented Jan 16, 2023

Provision: ignore other test environments #2865

Provision: ignore other test environments #2865

Conversation

masenf commented Jan 16, 2023 • edited

Thanks for contribution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masenf commented Jan 16, 2023

masenf commented Jan 16, 2023 •

edited