fix: only keep metadata.yaml for reactive charms (#1666)

Fixes #1654

charmcraft/services/package.py

@@ -239,10 +239,13 @@ def write_metadata(self, path: pathlib.Path) -> None:
 
         project_dict = self._project.marshal()
 
-        if (path / "metadata.yaml").exists():
+        # If there is a reactive part, defer to it for the existence of metadata.yaml.
+        plugins = {part.get("plugin") or name for name, part in self._project.parts.items()}
+        stage_dir = self._services.lifecycle.project_info.dirs.stage_dir
+        if "reactive" in plugins and (stage_dir / const.METADATA_FILENAME).exists():
             emit.debug("'metadata.yaml' generated by charm, not using original project metadata.")
         else:
-            self._write_file_or_object(self.metadata.marshal(), "metadata.yaml", path)
+            self._write_file_or_object(self.metadata.marshal(), const.METADATA_FILENAME, path)
         if actions := cast(dict | None, project_dict.get("actions")):
             self._write_file_or_object(actions, "actions.yaml", path)
         if config := cast(dict | None, project_dict.get("config")):

tests/integration/services/sample_projects/basic-reactive/prime/manifest.yaml

@@ -0,0 +1,13 @@
+charmcraft-version: 3.0-test-version
+charmcraft-started-at: '2020-03-14T00:00:00'
+bases:
+- name: ubuntu
+  channel: '22.04'
+  architectures:
+  - amd64
+analysis:
+  attributes:
+  - name: language
+    result: unknown
+  - name: framework
+    result: unknown

tests/integration/services/sample_projects/basic-reactive/prime/metadata.yaml

@@ -0,0 +1,5 @@
+description: |
+  The most basic possible valid charmcraft.yaml that doesn't need other files and gets returned to its own value.
+  Note that this means we cannot use short-form bases here because this charm is meant to be rewritable.
+name: basic-charm
+summary: The most basic valid charm

tests/integration/services/sample_projects/basic-reactive/project/charmcraft.yaml

@@ -0,0 +1,17 @@
+type: charm
+name: basic-charm
+summary: The most basic valid charm
+description: |
+  The most basic possible valid charmcraft.yaml that doesn't need other files and gets returned to its own value.
+  Note that this means we cannot use short-form bases here because this charm is meant to be rewritable.
+bases:
+- build-on:
+  - name: ubuntu
+    channel: '22.04'
+  run-on:
+  - name: ubuntu
+    channel: '22.04'
+
+parts:
+  reactive:
+    source: .

tests/integration/services/test_package.py

@@ -22,7 +22,7 @@
 import pytest_check
 
 import charmcraft
-from charmcraft import models, services
+from charmcraft import const, models, services
 from charmcraft.application.main import APP_METADATA
 
 
@@ -63,3 +63,58 @@ def test_write_metadata(monkeypatch, fs, package_service, project_path):
 
     for file in expected_prime_dir.iterdir():
         pytest_check.equal((test_prime_dir / file.name).read_text(), file.read_text())
+
+
+@pytest.mark.parametrize(
+    "project_path",
+    [
+        pytest.param(path, id=path.name)
+        for path in (pathlib.Path(__file__).parent / "sample_projects").iterdir()
+    ],
+)
+@freezegun.freeze_time(datetime.datetime(2020, 3, 14, 0, 0, 0, tzinfo=datetime.timezone.utc))
+def test_overwrite_metadata(monkeypatch, fs, package_service, project_path):
+    """Test that the metadata file gets rewritten for a charm.
+
+    Regression test for https://github.com/canonical/charmcraft/issues/1654
+    """
+    monkeypatch.setattr(charmcraft, "__version__", "3.0-test-version")
+    fs.add_real_directory(project_path)
+    test_prime_dir = pathlib.Path("/prime")
+    fs.create_dir(test_prime_dir)
+    expected_prime_dir = project_path / "prime"
+
+    project = models.CharmcraftProject.from_yaml_file(project_path / "project" / "charmcraft.yaml")
+    project._started_at = datetime.datetime.utcnow()
+    package_service._project = project
+
+    fs.create_file(test_prime_dir / const.METADATA_FILENAME, contents="INVALID!!")
+
+    package_service.write_metadata(test_prime_dir)
+
+    for file in expected_prime_dir.iterdir():
+        pytest_check.equal((test_prime_dir / file.name).read_text(), file.read_text())
+
+
+@freezegun.freeze_time(datetime.datetime(2020, 3, 14, 0, 0, 0, tzinfo=datetime.timezone.utc))
+def test_no_overwrite_reactive_metadata(monkeypatch, fs, package_service):
+    """Test that the metadata file doesn't get overwritten for a reactive charm..
+
+    Regression test for https://github.com/canonical/charmcraft/issues/1654
+    """
+    monkeypatch.setattr(charmcraft, "__version__", "3.0-test-version")
+    project_path = pathlib.Path(__file__).parent / "sample_projects" / "basic-reactive"
+    fs.add_real_directory(project_path)
+    test_prime_dir = pathlib.Path("/prime")
+    fs.create_dir(test_prime_dir)
+    test_stage_dir = pathlib.Path("/stage")
+    fs.create_dir(test_stage_dir)
+    fs.create_file(test_stage_dir / const.METADATA_FILENAME, contents="INVALID!!")
+
+    project = models.CharmcraftProject.from_yaml_file(project_path / "project" / "charmcraft.yaml")
+    project._started_at = datetime.datetime.utcnow()
+    package_service._project = project
+
+    package_service.write_metadata(test_prime_dir)
+
+    assert not (test_prime_dir / const.METADATA_FILENAME).exists()

tests/unit/services/test_package.py

@@ -112,8 +112,11 @@ def test_do_not_overwrite_metadata_yaml(
 ):
     fake_prime_dir = fake_path / "prime"
     fake_prime_dir.mkdir()
-    fake_metadata_yaml = fake_prime_dir / "metadata.yaml"
-    fake_metadata_yaml.touch()
+    fake_stage_dir = fake_path / "stage"
+    fake_stage_dir.mkdir()
+    fake_staged_metadata = fake_stage_dir / "metadata.yaml"
+    fake_staged_metadata.touch()
+    package_service._project.parts["reactive"] = {"source": "."}
 
     package_service.write_metadata(fake_prime_dir)