Merge pull request regro#3923 from ytausch/remove-side-effects

fix: remove side effects from feedstock loading

Author: beckermr

conda_forge_tick/container_cli.py

@@ -320,7 +320,7 @@ def _parse_feedstock(
 
     name = attrs["feedstock_name"]
 
-    load_feedstock_local(
+    node_attrs = load_feedstock_local(
         name,
         attrs,
         meta_yaml=meta_yaml,
@@ -329,7 +329,7 @@ def _parse_feedstock(
         mark_not_archived=mark_not_archived,
     )
 
-    return attrs
+    return node_attrs
 
 
 def _parse_meta_yaml(

conda_forge_tick/feedstock_parser.py

@@ -234,17 +234,30 @@ def _clean_req_nones(reqs):
 
 def populate_feedstock_attributes(
     name: str,
-    sub_graph: typing.MutableMapping,
+    existing_node_attrs: typing.MutableMapping[str, typing.Any],
     meta_yaml: str | None = None,
     recipe_yaml: str | None = None,
     conda_forge_yaml: str | None = None,
     mark_not_archived: bool = False,
     feedstock_dir: str | Path | None = None,
-) -> typing.MutableMapping:
-    """Parse the various configuration information into something usable"""
+) -> dict[str, typing.Any]:
+    """
+    Parse the various configuration information into the node_attrs of a feedstock.
+
+    :param name: The name of the feedstock
+    :param existing_node_attrs: The existing node_attrs of the feedstock. Pass an empty dict if none.
+    :param meta_yaml: The meta.yaml file as a string
+    :param recipe_yaml: The recipe.yaml file as a string
+    :param conda_forge_yaml: The conda-forge.yaml file as a string
+    :param mark_not_archived: If True, forcibly mark the feedstock as not archived in the node attrs, even if it is archived.
+    :param feedstock_dir: The directory where the feedstock is located. If None, some information will not be available.
 
+    :return: A dictionary with the new node_attrs of the feedstock, with only some fields populated.
+    """
     from conda_forge_tick.chaindb import ChainDB, _convert_to_dict
 
+    node_attrs = {key: value for key, value in existing_node_attrs.items()}
+
     if isinstance(feedstock_dir, str):
         feedstock_dir = Path(feedstock_dir)
 
@@ -253,35 +266,37 @@ def populate_feedstock_attributes(
     ):
         raise ValueError("Either `meta_yaml` or `recipe_yaml` needs to be given.")
 
-    sub_graph.update({"feedstock_name": name, "parsing_error": False, "branch": "main"})
+    node_attrs.update(
+        {"feedstock_name": name, "parsing_error": False, "branch": "main"}
+    )
 
     if mark_not_archived:
-        sub_graph.update({"archived": False})
+        node_attrs.update({"archived": False})
 
     # strip out old keys - this removes old platforms when one gets disabled
-    for key in list(sub_graph.keys()):
+    for key in list(node_attrs.keys()):
         if key.endswith("meta_yaml") or key.endswith("requirements") or key == "req":
-            del sub_graph[key]
+            del node_attrs[key]
 
     if isinstance(meta_yaml, str):
-        sub_graph["raw_meta_yaml"] = meta_yaml
+        node_attrs["raw_meta_yaml"] = meta_yaml
     elif isinstance(recipe_yaml, str):
-        sub_graph["raw_meta_yaml"] = recipe_yaml
+        node_attrs["raw_meta_yaml"] = recipe_yaml
 
     # Get the conda-forge.yml
     if isinstance(conda_forge_yaml, str):
         try:
-            sub_graph["conda-forge.yml"] = {
+            node_attrs["conda-forge.yml"] = {
                 k: v for k, v in yaml.safe_load(conda_forge_yaml).items()
             }
         except Exception as e:
             import traceback
 
             trb = traceback.format_exc()
-            sub_graph["parsing_error"] = sanitize_string(
+            node_attrs["parsing_error"] = sanitize_string(
                 f"feedstock parsing error: cannot load conda-forge.yml: {e}\n{trb}"
             )
-            return sub_graph
+            return node_attrs
 
     if feedstock_dir is not None:
         logger.debug(
@@ -385,7 +400,7 @@ def populate_feedstock_attributes(
         else:
             logger.debug("doing generic parsing")
             plat_archs = [("win", "64"), ("osx", "64"), ("linux", "64")]
-            for k in set(sub_graph["conda-forge.yml"].get("provider", {})):
+            for k in set(node_attrs["conda-forge.yml"].get("provider", {})):
                 if "_" in k:
                     plat_archs.append(tuple(k.split("_")))
             if isinstance(meta_yaml, str):
@@ -401,46 +416,46 @@ def populate_feedstock_attributes(
         import traceback
 
         trb = traceback.format_exc()
-        sub_graph["parsing_error"] = sanitize_string(
+        node_attrs["parsing_error"] = sanitize_string(
             f"feedstock parsing error: cannot rendering recipe: {e}\n{trb}"
         )
         raise
 
     logger.debug("platforms: %s", plat_archs)
-    sub_graph["platforms"] = ["_".join(k) for k in plat_archs]
+    node_attrs["platforms"] = ["_".join(k) for k in plat_archs]
 
     # this makes certain that we have consistent ordering
     sorted_variant_yamls = [x for _, x in sorted(zip(plat_archs, variant_yamls))]
     yaml_dict = ChainDB(*sorted_variant_yamls)
     if not yaml_dict:
         logger.error(f"Something odd happened when parsing recipe {name}")
-        sub_graph["parsing_error"] = (
+        node_attrs["parsing_error"] = (
             "feedstock parsing error: could not combine metadata dicts across platforms"
         )
-        return sub_graph
+        return node_attrs
 
-    sub_graph["meta_yaml"] = _dedupe_meta_yaml(_convert_to_dict(yaml_dict))
-    meta_yaml = sub_graph["meta_yaml"]
+    node_attrs["meta_yaml"] = _dedupe_meta_yaml(_convert_to_dict(yaml_dict))
+    meta_yaml = node_attrs["meta_yaml"]
 
     # remove all plat-arch specific keys to remove old ones if a combination is disabled
-    for k in list(sub_graph.keys()):
+    for k in list(node_attrs.keys()):
         if k in ["raw_meta_yaml", "total_requirements"]:
             continue
         if k.endswith("_meta_yaml") or k.endswith("_requirements"):
-            sub_graph.pop(k)
+            node_attrs.pop(k)
 
     for k, v in zip(plat_archs, variant_yamls):
         plat_arch_name = "_".join(k)
-        sub_graph[f"{plat_arch_name}_meta_yaml"] = v
-        _, sub_graph[f"{plat_arch_name}_requirements"], _ = _extract_requirements(
+        node_attrs[f"{plat_arch_name}_meta_yaml"] = v
+        _, node_attrs[f"{plat_arch_name}_requirements"], _ = _extract_requirements(
             v,
             outputs_to_keep=BOOTSTRAP_MAPPINGS.get(name, None),
         )
 
     (
-        sub_graph["total_requirements"],
-        sub_graph["requirements"],
-        sub_graph["strong_exports"],
+        node_attrs["total_requirements"],
+        node_attrs["requirements"],
+        node_attrs["strong_exports"],
     ) = _extract_requirements(
         meta_yaml,
         outputs_to_keep=BOOTSTRAP_MAPPINGS.get(name, None),
@@ -455,56 +470,56 @@ def populate_feedstock_attributes(
             ),
         )
         # handle implicit meta packages
-        if "run" in sub_graph.get("meta_yaml", {}).get("requirements", {}):
+        if "run" in node_attrs.get("meta_yaml", {}).get("requirements", {}):
             outputs_names.add(meta_yaml["package"]["name"])
     # add in single package name
     else:
         outputs_names = {meta_yaml["package"]["name"]}
-    sub_graph["outputs_names"] = outputs_names
+    node_attrs["outputs_names"] = outputs_names
 
     # TODO: Write schema for dict
     # TODO: remove this
     req = _get_requirements(
         yaml_dict,
         outputs_to_keep=BOOTSTRAP_MAPPINGS.get(name, []),
     )
-    sub_graph["req"] = req
+    node_attrs["req"] = req
 
     # set name and version
     keys = [("package", "name"), ("package", "version")]
     missing_keys = [k[1] for k in keys if k[1] not in yaml_dict.get(k[0], {})]
     for k in keys:
         if k[1] not in missing_keys:
-            sub_graph[k[1]] = yaml_dict[k[0]][k[1]]
+            node_attrs[k[1]] = yaml_dict[k[0]][k[1]]
 
     # sometimes a version is not given at the top level, so we check outputs
     # we do not know which version to take, but hopefully they are all the same
     if (
-        "version" not in sub_graph
+        "version" not in node_attrs
         and "outputs" in yaml_dict
         and len(yaml_dict["outputs"]) > 0
         and "version" in yaml_dict["outputs"][0]
     ):
-        sub_graph["version"] = yaml_dict["outputs"][0]["version"]
+        node_attrs["version"] = yaml_dict["outputs"][0]["version"]
 
     # set the url and hash
-    sub_graph.pop("url", None)
-    sub_graph.pop("hash_type", None)
+    node_attrs.pop("url", None)
+    node_attrs.pop("hash_type", None)
 
     source = yaml_dict.get("source", [])
     if isinstance(source, collections.abc.Mapping):
         source = [source]
     source_keys: Set[str] = set()
     for s in source:
-        if not sub_graph.get("url"):
-            sub_graph["url"] = s.get("url")
+        if not node_attrs.get("url"):
+            node_attrs["url"] = s.get("url")
         source_keys |= s.keys()
 
     kl = list(sorted(source_keys & hashlib.algorithms_available, reverse=True))
     if kl:
-        sub_graph["hash_type"] = kl[0]
+        node_attrs["hash_type"] = kl[0]
 
-    return sub_graph
+    return node_attrs
 
 
 def load_feedstock_local(
@@ -514,7 +529,7 @@ def load_feedstock_local(
     recipe_yaml: str | None = None,
     conda_forge_yaml: str | None = None,
     mark_not_archived: bool = False,
-):
+) -> dict[str, typing.Any]:
     """Load a feedstock into subgraph based on its name. If meta_yaml and/or
     conda_forge_yaml are not provided, they will be fetched from the feedstock.
 
@@ -538,6 +553,7 @@ def load_feedstock_local(
     sub_graph : MutableMapping
         The sub_graph, now updated with the feedstock metadata
     """
+    new_sub_graph = {key: value for key, value in sub_graph.items()}
 
     if meta_yaml is not None and recipe_yaml is not None:
         raise ValueError("Only either `meta_yaml` or `recipe_yaml` can be overridden.")
@@ -552,17 +568,17 @@ def load_feedstock_local(
         # if nothing is overridden and no file is present, error out
         if meta_yaml is None and recipe_yaml is None:
             if isinstance(feedstock_dir, Response):
-                sub_graph.update(
+                new_sub_graph.update(
                     {"feedstock_name": name, "parsing_error": False, "branch": "main"}
                 )
 
                 if mark_not_archived:
-                    sub_graph.update({"archived": False})
+                    new_sub_graph.update({"archived": False})
 
-                sub_graph["parsing_error"] = sanitize_string(
+                new_sub_graph["parsing_error"] = sanitize_string(
                     f"make_graph: {feedstock_dir.status_code}"
                 )
-                return sub_graph
+                return new_sub_graph
 
             meta_yaml_path = Path(feedstock_dir).joinpath("recipe", "meta.yaml")
             recipe_yaml_path = Path(feedstock_dir).joinpath("recipe", "recipe.yaml")
@@ -580,18 +596,16 @@ def load_feedstock_local(
             if conda_forge_yaml_path.exists():
                 conda_forge_yaml = conda_forge_yaml_path.read_text()
 
-        populate_feedstock_attributes(
+        return populate_feedstock_attributes(
             name,
-            sub_graph,
+            new_sub_graph,
             meta_yaml=meta_yaml,
             recipe_yaml=recipe_yaml,
             conda_forge_yaml=conda_forge_yaml,
             mark_not_archived=mark_not_archived,
             feedstock_dir=feedstock_dir,
         )
 
-    return sub_graph
-
 
 def load_feedstock_containerized(
     name: str,

tests/conftest.py

@@ -98,6 +98,10 @@ def __exit__(
     ) -> None:
         pass
 
+    @property
+    def data(self):
+        return self
+
 
 def pytest_configure(config):
     config.addinivalue_line(

tests/test_make_graph.py

@@ -0,0 +1,54 @@
+import pytest
+from conftest import FakeLazyJson
+
+from conda_forge_tick.lazy_json_backends import LazyJson
+from conda_forge_tick.make_graph import try_load_feedstock
+
+
+@pytest.mark.parametrize("container_enabled", [True, False])
+@pytest.mark.parametrize("existing_archived", [True, False, None])
+@pytest.mark.parametrize("mark_not_archived", [True, False])
+def test_try_load_feedstock(
+    request: pytest.FixtureRequest,
+    mark_not_archived: bool,
+    existing_archived: bool | None,
+    container_enabled: bool,
+):
+    if container_enabled:
+        request.getfixturevalue("use_containers")
+
+    feedstock = "typst-test"  # archived
+
+    fake_lazy_json = FakeLazyJson()  # empty dict
+
+    with fake_lazy_json as loaded_lazy_json:
+        if existing_archived is not None:
+            loaded_lazy_json["archived"] = existing_archived
+        # FakeLazyJson is not an instance of LazyJson
+        # noinspection PyTypeChecker
+        data = try_load_feedstock(feedstock, loaded_lazy_json, mark_not_archived).data  # type: ignore
+
+    if mark_not_archived:
+        assert data["archived"] is False
+    elif existing_archived is None:
+        assert "archived" not in data
+    else:
+        assert data["archived"] is existing_archived
+
+    assert data["feedstock_name"] == feedstock
+    assert data["parsing_error"] is False
+    assert data["raw_meta_yaml"].startswith("{% set name")
+    assert isinstance(data["conda-forge.yml"], dict)
+    assert "linux_64" in data["platforms"]
+    assert data["meta_yaml"]["about"]["license"] == "MIT"
+    assert isinstance(data["linux_64_meta_yaml"], dict)
+    assert isinstance(data["linux_64_requirements"], dict)
+    assert isinstance(data["total_requirements"], dict)
+    assert data["strong_exports"] is False
+    assert data["outputs_names"] == {feedstock}
+    assert isinstance(data["req"], set)
+    assert data["name"] == feedstock
+    assert data["version"].startswith("0.")
+    assert data["url"].startswith("https://github.com/tingerrr/typst-test")
+    assert data["hash_type"] == "sha256"
+    assert isinstance(data["version_pr_info"], LazyJson)

tests/test_migrators.py

@@ -549,7 +549,7 @@ def run_test_migration(
             name = "blah"
 
         pmy = populate_feedstock_attributes(
-            name, sub_graph={}, meta_yaml=inp, conda_forge_yaml=cf_yml
+            name, existing_node_attrs={}, meta_yaml=inp, conda_forge_yaml=cf_yml
         )
 
         # these are here for legacy migrators
@@ -571,7 +571,7 @@ def run_test_migration(
 
         pmy = populate_feedstock_attributes(
             name,
-            sub_graph={},
+            existing_node_attrs={},
             recipe_yaml=inp,
             conda_forge_yaml=cf_yml,
             feedstock_dir=tmp_path,