sequencer: only load project vars in adopting part

Project variables exist only in the part scope and should be ignored
in other parts' states. In the existing implementation these values
are persisted in each part, which leads to double assignment errors
when a step is executed for parts in a order that's different from
the previous execution (see example in LP #1831135).

There are different ways to address this issue, the current fix
tries to be minimally disruptive by just ignoring persisted values
when reloading the state. We can refactor this code later to have
a cleaner implementation.

Signed-off-by: Claudio Matsuoka <[email protected]>

craft_parts/infos.py

@@ -173,6 +173,11 @@ def project_name(self) -> Optional[str]:
         """Return the name of the project using craft-parts."""
         return self._project_name
 
+    @property
+    def project_vars_part_name(self) -> Optional[str]:
+        """Return the name of the part that can set project vars."""
+        return self._project_vars_part_name
+
     @property
     def project_options(self) -> Dict[str, Any]:
         """Obtain a project-wide options dictionary."""

craft_parts/sequencer.py

@@ -200,9 +200,16 @@ def _add_step_actions(
             current_step,
             action_type=ActionType.SKIP,
             reason="already ran",
-            project_vars=self._sm.project_vars(part, current_step),
+            project_vars=self._get_project_vars(part, current_step),
         )
 
+    def _get_project_vars(
+        self, part: Part, step: Step
+    ) -> Optional[Dict[str, ProjectVar]]:
+        if part.name == self._project_info.project_vars_part_name:
+            return self._sm.project_vars(part, step)
+        return None
+
     def _process_dependencies(self, part: Part, step: Step) -> None:
         prerequisite_step = steps.dependency_prerequisite_step(step)
         if not prerequisite_step:

tests/integration/lifecycle/test_craftctl.py

@@ -23,7 +23,7 @@
 import yaml
 
 import craft_parts
-from craft_parts import Action, Step, errors
+from craft_parts import Action, ActionType, Step, errors
 from tests import TESTS_DIR
 
 
@@ -356,6 +356,117 @@ def test_craftctl_set_error(new_dir, capfd, mocker):
     assert "'myvar' not in project variables" in captured.err
 
 
+def test_craftctl_set_only_once(new_dir, capfd, mocker):  # see LP #1831135
+    parts_yaml = textwrap.dedent(
+        """\
+        parts:
+          part1:
+            plugin: nil
+            override-pull: |
+              craftctl default
+              craftctl set version=xx
+
+          part2:
+            plugin: nil
+            after: [part1]
+        """
+    )
+    parts = yaml.safe_load(parts_yaml)
+
+    lf = craft_parts.LifecycleManager(
+        parts,
+        application_name="test_set",
+        cache_dir=new_dir,
+        project_vars_part_name="part1",
+        project_vars={"version": ""},
+    )
+
+    assert lf.project_info.get_project_var("version", raw_read=True) == ""
+
+    actions = lf.plan(Step.BUILD)
+    with lf.action_executor() as ctx:
+        ctx.execute(actions)
+
+    assert lf.project_info.get_project_var("version") == "xx"
+
+    # change something in part1 to make it dirty
+    parts["parts"]["part1"]["override-pull"] += "\necho foo"
+
+    # now build only part2 so that pull order will be reversed
+    lf = craft_parts.LifecycleManager(
+        parts,
+        application_name="test_set",
+        cache_dir=new_dir,
+        project_vars_part_name="part1",
+        project_vars={"version": ""},
+    )
+
+    assert lf.project_info.get_project_var("version", raw_read=True) == ""
+
+    # execution of actions must succeed
+    actions = lf.plan(Step.BUILD, part_names=["part2"])
+    with lf.action_executor() as ctx:
+        ctx.execute(actions)
+
+    assert lf.project_info.get_project_var("version") == "xx"
+
+
+def test_craftctl_update_project_vars(new_dir, capfd, mocker):
+    parts_yaml = textwrap.dedent(
+        """\
+        parts:
+          part1:
+            plugin: nil
+            override-pull: |
+              craftctl default
+              craftctl set version=xx
+
+          part2:
+            plugin: nil
+            after: [part1]
+        """
+    )
+    parts = yaml.safe_load(parts_yaml)
+
+    lf = craft_parts.LifecycleManager(
+        parts,
+        application_name="test_set",
+        cache_dir=new_dir,
+        project_vars_part_name="part1",
+        project_vars={"version": ""},
+    )
+
+    assert lf.project_info.get_project_var("version", raw_read=True) == ""
+
+    actions = lf.plan(Step.BUILD)
+    with lf.action_executor() as ctx:
+        ctx.execute(actions)
+
+    assert lf.project_info.get_project_var("version") == "xx"
+
+    # re-execute the lifecycle with no changes
+    lf = craft_parts.LifecycleManager(
+        parts,
+        application_name="test_set",
+        cache_dir=new_dir,
+        project_vars_part_name="part1",
+        project_vars={"version": ""},
+    )
+
+    assert lf.project_info.get_project_var("version", raw_read=True) == ""
+
+    actions = lf.plan(Step.BUILD)
+
+    # all actions should be skipped
+    for action in actions:
+        assert action.action_type == ActionType.SKIP
+
+    with lf.action_executor() as ctx:
+        ctx.execute(actions)
+
+    assert lf.project_info.get_project_var("version") == "xx"
+
+
 def test_craftctl_get_error(new_dir, capfd, mocker):
     parts_yaml = textwrap.dedent(
         """\

tests/unit/test_infos.py

@@ -69,6 +69,7 @@ def test_project_info(mocker, new_dir, tc_arch, tc_target_arch, tc_triplet, tc_c
         "project_vars_part_name": "adopt",
         "project_vars": {"a": ProjectVar(value="b")},
     }
+    assert x.project_vars_part_name == "adopt"
     assert x.global_environment == {}
 
     assert x.parts_dir == new_dir / "parts"