Adding methods for building environment in the right order

tests/core/environment-precedence/data/.fmf/version

@@ -0,0 +1 @@
+1

tests/core/environment-precedence/data/dump-envvars.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+LINE_PREFIX="${LINE_PREFIX:-}"
+
+for line in $(env -0 | sort -z | tr '\0' '\n'); do echo "${LINE_PREFIX}: ${line}"; done

tests/core/environment-precedence/data/main.fmf

@@ -0,0 +1,62 @@
+# In general, we need variables that are defined by multiple sources,
+# and we observe who overwrites who. We also need variables that stay
+# not overwritten, so we could verify all sources contributed.
+#
+# VAR1: claimed by `test[].environment`
+# VAR2: claimed by `plan.environment-file` key
+# VAR3: claimed by `plan.environment` key
+# VAR4: claimed by `provision[].environment` key
+# VAR5: claimed by plan environment file
+# VAR6: claimed by `tmt run --environment-file` CLI option
+# VAR7: claimed by `tmt run --environment` CLI option
+
+/test:
+    test: LINE_PREFIX=execute.0 ./dump-envvars.sh
+
+    environment:
+        VAR1: test.environment
+        VAR2: test.environment
+        VAR3: test.environment
+        VAR4: test.environment
+        VAR5: test.environment
+        VAR6: test.environment
+        VAR7: test.environment
+
+/plan:
+    environment:
+      VAR3: plan.environment
+      VAR4: plan.environment
+      VAR5: plan.environment
+      VAR6: plan.environment
+      VAR7: plan.environment
+
+    environment-file:
+        - plan-environment.env
+
+    discover:
+        how: fmf
+
+    provision:
+        how: local
+        environment:
+            VAR4: provision.environment
+            VAR5: provision.environment
+            VAR6: provision.environment
+            VAR7: provision.environment
+
+    prepare:
+        - how: shell
+          script: LINE_PREFIX=prepare.0 ./dump-envvars.sh
+
+        - how: shell
+          script: ./populate-plan-environment-file.sh
+
+        - how: shell
+        - script: LINE_PREFIX=prepare.2 ./dump-envvars.sh
+
+    execute:
+        how: tmt
+
+    finish:
+        how: shell
+        script: LINE_PREFIX=finish.0 ./dump-envvars.sh

tests/core/environment-precedence/data/plan-environment.env

@@ -0,0 +1,6 @@
+VAR2=plan.environment-file
+VAR3=plan.environment-file
+VAR4=plan.environment-file
+VAR5=plan.environment-file
+VAR6=plan.environment-file
+VAR7=plan.environment-file

tests/core/environment-precedence/data/populate-plan-environment-file.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+echo "VAR5=plan.plan-environment-file" >> $TMT_PLAN_ENVIRONMENT_FILE
+echo "VAR6=plan.plan-environment-file" >> $TMT_PLAN_ENVIRONMENT_FILE
+echo "VAR7=plan.plan-environment-file" >> $TMT_PLAN_ENVIRONMENT_FILE

tests/core/environment-precedence/data/run-environment.env

@@ -0,0 +1,2 @@
+VAR6=cli.run.environment-file
+VAR7=cli.run.environment-file

tests/core/environment-precedence/main.fmf

@@ -0,0 +1,4 @@
+summary: Verify environment variable precedence
+description: |
+    Verify whether various sources of environment variables are correctly
+    incorporated into the final environment.

tests/core/environment-precedence/test.sh

@@ -0,0 +1,48 @@
+#!/bin/bash
+. /usr/share/beakerlib/beakerlib.sh || exit 1
+
+rlJournalStart
+    rlPhaseStartSetup
+        rlRun 'pushd data'
+    rlPhaseEnd
+
+    rlPhaseStartTest 'Check environment variable precedence'
+        rlRun -s "tmt -vvv --feeling-safe run -a --environment VAR7=cli.run.environment --environment-file run-environment.env"
+
+        rlAssertNotGrep "prepare.0: VAR1=test.environment"              $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR2=plan.environment-file"         $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR3=plan.environment"              $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR4=provision.environment"         $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR5=provision.environment"         $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR6=cli.run.environment-file"      $rlRun_LOG
+        rlAssertGrep    "prepare.0: VAR7=cli.run.environment"           $rlRun_LOG
+
+        rlAssertNotGrep "prepare.2: VAR1=test.environment"              $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR2=plan.environment-file"         $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR3=plan.environment"              $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR4=provision.environment"         $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR5=plan.plan-environment-file"    $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR6=cli.run.environment-file"      $rlRun_LOG
+        rlAssertGrep    "prepare.2: VAR7=cli.run.environment"           $rlRun_LOG
+
+        rlAssertGrep    "execute.0: VAR1=test.environment"              $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR2=plan.environment-file"         $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR3=plan.environment"              $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR4=provision.environment"         $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR5=plan.plan-environment-file"    $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR6=cli.run.environment-file"      $rlRun_LOG
+        rlAssertGrep    "execute.0: VAR7=cli.run.environment"           $rlRun_LOG
+
+        rlAssertNotGrep "finish.0: VAR1=test.environment"               $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR2=plan.environment-file"          $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR3=plan.environment"               $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR4=provision.environment"          $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR5=plan.plan-environment-file"     $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR6=cli.run.environment-file"       $rlRun_LOG
+        rlAssertGrep    "finish.0: VAR7=cli.run.environment"            $rlRun_LOG
+    rlPhaseEnd
+
+    rlPhaseStartCleanup
+        rlRun 'popd'
+    rlPhaseEnd
+rlJournalEnd

tests/core/plan-env-file/data/main.fmf

@@ -22,20 +22,6 @@
             how: shell
             script: ./setup_env.sh
 
-    /env-file-override:
-        prepare:
-            how: shell
-            script: echo "MYVAR1=MYVAR1_VALUE" >> $TMT_PLAN_ENVIRONMENT_FILE
-        environment-file:
-            - override.env
-
-    /env-override:
-        prepare:
-            how: shell
-            script: echo "MYVAR1=MYVAR1_VALUE" >> $TMT_PLAN_ENVIRONMENT_FILE
-        environment:
-            MYVAR1: MYVAR1_OVERRIDE
-
     /execute-override:
         discover:
             how: shell

tests/core/plan-env-file/main.fmf

@@ -2,5 +2,4 @@ summary: Verify proper application of TMT_PLAN_ENVIRONMENT_FILE variables
 description:
     Verify proper application of environment variables provided
     by 'TMT_PLAN_ENVIRONMENT_FILE' and that they are available
-    during the execute and finish phase. Also, confirm that the
-    variables have a lower priority than the 'environment-file' key.
+    during the execute and finish phase.

tests/core/plan-env-file/test.sh

@@ -25,29 +25,6 @@ rlJournalStart
         rlAssertGrep 'total: 1 test passed' $rlRun_LOG
     rlPhaseEnd
 
-    rlPhaseStartTest 'Check environment variable is overridden by "environment-file" key'
-        rlRun -s 'tmt run -rvvv plan --name /plan/env-file-override'
-        rlAssertGrep 'MYVAR1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'FINISH1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'total: 1 test passed' $rlRun_LOG
-    rlPhaseEnd
-
-    rlPhaseStartTest 'Check environment variable is overridden by "environment" key'
-        rlRun -s 'tmt run -rvvv plan --name /plan/env-override'
-        rlAssertGrep 'MYVAR1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'FINISH1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'total: 1 test passed' $rlRun_LOG
-    rlPhaseEnd
-
-    rlPhaseStartTest 'Check environment variable is overridden by command line'
-        rlRun -s 'tmt run -rvvv -e MYVAR1=MYVAR1_OVERRIDE plan --name /plan/multiple'
-        rlAssertGrep 'MYVAR1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'MYVAR2="MYVAR2_VALUE"' $rlRun_LOG
-        rlAssertGrep 'FINISH1="MYVAR1_OVERRIDE"' $rlRun_LOG
-        rlAssertGrep 'FINISH2="MYVAR2_VALUE"' $rlRun_LOG
-        rlAssertGrep 'total: 1 test passed' $rlRun_LOG
-    rlPhaseEnd
-
     rlPhaseStartTest 'Check "execute" step can override environment variable'
         rlRun -s 'tmt run -rvvv plan --name /plan/execute-override'
         rlAssertGrep 'FINISH1="MYVAR1_OVERRIDE"' $rlRun_LOG

tmt/base/plan.py

@@ -336,11 +336,9 @@ def __init__(
                 self._initialize_worktree()
 
         # Expand all environment and context variables in the node
-        environment = Environment()
-
-        environment.update(self.environment, self.intrinsic_environment)
-
-        with environment.as_environ():
+        with Environment.build_environment(
+            plan=self, run=self.my_run, logger=self._logger
+        ).as_environ():
             expand_node_data(node.data, self.fmf_context)  # pyright: ignore[reportUnknownVariableType, reportUnknownArgumentType]
 
         # Initialize test steps
@@ -549,7 +547,6 @@ def environment(self) -> Environment:
                     **self._environment_from_fmf,
                     **self._environment_from_importing,
                     **self._environment_from_cli,
-                    **self.my_run.environment,
                 }
             )
 

tmt/guest/__init__.py

@@ -2263,17 +2263,12 @@ def _prepare_command_environment(
         """
 
         if environment is None:
-            environment = Environment()
-
-            environment.update(self.environment)
-
-            if isinstance(self.parent, tmt.steps.Step):
-                environment.update(self.parent.plan.environment)
-
-            environment.update(self.intrinsic_environment)
-
-            if isinstance(self.parent, tmt.steps.Step):
-                environment.update(self.parent.plan.intrinsic_environment)
+            environment = Environment.build_environment(
+                plan=self.parent.plan if isinstance(self.parent, tmt.steps.Step) else None,
+                run=self.parent.plan.my_run if isinstance(self.parent, tmt.steps.Step) else None,
+                guest=self,
+                logger=self._logger,
+            )
 
         else:
             # Create a copy of given environment - this prevents any

tmt/steps/context/__init__.py

@@ -1,5 +1,9 @@
+import abc
 from typing import TYPE_CHECKING, Optional
 
+from tmt.container import container
+from tmt.utils.environment import HasIntrinsicEnvironment
+
 if TYPE_CHECKING:
     from tmt.steps.context.reboot import RebootContext
     from tmt.steps.context.restart import RestartContext
@@ -15,3 +19,8 @@ def is_guest_healthy(
         return False
 
     return True
+
+
+@container
+class StepContext(HasIntrinsicEnvironment, abc.ABC):
+    pass

tmt/steps/context/abort.py

@@ -4,8 +4,9 @@
 import tmt.steps.scripts
 import tmt.utils
 from tmt.container import container
+from tmt.steps.context import StepContext
 from tmt.utils import Path
-from tmt.utils.environment import Environment, HasIntrinsicEnvironment
+from tmt.utils.environment import Environment
 
 
 class AbortStep(tmt.utils.GeneralError):
@@ -15,7 +16,7 @@ class AbortStep(tmt.utils.GeneralError):
 
 
 @container
-class AbortContext(HasIntrinsicEnvironment):
+class AbortContext(StepContext):
     """
     Provides API for handling a phase-requested abort of a step.
     """

tmt/steps/context/pidfile.py

@@ -83,8 +83,9 @@
 from tmt.container import container
 from tmt.guest import Guest, TransferOptions
 from tmt.steps import safe_filename
+from tmt.steps.context import StepContext
 from tmt.utils import Path, ShellScript
-from tmt.utils.environment import Environment, EnvVarValue, HasIntrinsicEnvironment
+from tmt.utils.environment import Environment, EnvVarValue
 from tmt.utils.templates import render_template
 
 TEST_PIDFILE_FILENAME = 'tmt-test.pid'
@@ -193,7 +194,7 @@ def effective_pidfile_root() -> Path:
 
 
 @container
-class PidFileContext(HasIntrinsicEnvironment):
+class PidFileContext(StepContext):
     #: Phase owning this context.
     phase: tmt.steps.BasePlugin[Any, Any]
 

tmt/steps/context/reboot.py

@@ -8,8 +8,9 @@
 import tmt.utils
 from tmt.container import MetadataContainer, container
 from tmt.guest import Guest, RebootMode, SoftRebootModes
+from tmt.steps.context import StepContext
 from tmt.utils import Path, ShellScript
-from tmt.utils.environment import Environment, EnvVarValue, HasIntrinsicEnvironment
+from tmt.utils.environment import Environment, EnvVarValue
 from tmt.utils.wait import Deadline, Waiting
 
 if TYPE_CHECKING:
@@ -27,7 +28,7 @@ class RebootData(MetadataContainer):
 
 
 @container
-class RebootContext(HasIntrinsicEnvironment):
+class RebootContext(StepContext):
     """
     Tracks information about guest reboots.
     """

tmt/steps/context/restart.py

@@ -4,14 +4,15 @@
 import tmt.utils
 from tmt.container import container
 from tmt.guest import Guest
-from tmt.utils.environment import Environment, EnvVarValue, HasIntrinsicEnvironment
+from tmt.steps.context import StepContext
+from tmt.utils.environment import Environment, EnvVarValue
 
 if TYPE_CHECKING:
     from tmt.steps.context.reboot import RebootContext
 
 
 @container
-class RestartContext(HasIntrinsicEnvironment):
+class RestartContext(StepContext):
     """
     Tracks information about restarts of an action, e.g. a test script.
     """

tmt/steps/context/restraint.py

@@ -2,11 +2,12 @@
 
 import tmt.log
 from tmt.container import container
-from tmt.utils.environment import Environment, EnvVarValue, HasIntrinsicEnvironment
+from tmt.steps.context import StepContext
+from tmt.utils.environment import Environment, EnvVarValue
 
 
 @container
-class RestraintContext(HasIntrinsicEnvironment):
+class RestraintContext(StepContext):
     """
     Provides restraint-related context for execution.
     """

tmt/steps/execute/__init__.py

@@ -392,26 +392,23 @@ def intrinsic_environment(self) -> Environment:
     @property
     def environment(self) -> Environment:
         if self._environment is None:
-            # narrow type
-            assert isinstance(self.phase.step.plan.my_run, tmt.base.run.Run)
-
-            environment = Environment()
-
-            environment.update(
-                self.guest.environment,
-                self.test.environment,
-                self.guest.plan_environment,
-                self.phase.step.plan.environment,
+            self._environment = Environment.build_environment(
+                test=self.test,
+                plan=self.phase.step.plan,
+                run=self.phase.step.plan.my_run,
+                guest=self.guest,
+                test_invocation=self,
+                logger=self.logger,
             )
 
-        else:
-            environment = self._environment
-
-        environment.update(self.intrinsic_environment)
-
-        self._environment = environment
-
-        return environment
+        return self._environment.refresh_intrinsics(
+            test=self.test,
+            plan=self.phase.step.plan,
+            run=self.phase.step.plan.my_run,
+            guest=self.guest,
+            test_invocation=self,
+            logger=self.logger,
+        )
 
     def invoke_check(self, event: CheckEvent, check: Check) -> list[CheckResult]:
         results, exc, timer = Stopwatch.measure(