Skip to content

Commit

Permalink
Replace sys.exit calls in conda_build/metadata.py (#5371)
Browse files Browse the repository at this point in the history
* Update exceptions imports/usage in metadata.py

* Replace sys.exit call with CondaBuildUserError for select_lines() function

* Add unit test for CondaBuildUserError exception in select_lines test

* Replace sys.exit call in _git_clean() and add unit test

* Replace sys.exit call in check_bad_chrs() and add unit test

* Update import statements and remove sys.exit call from yamlize()

* Mark _get_env_path() for deprecation

* Remove sys.exit call from parse_until_resolved() and add unit test

* Remove sys.exit call from _get_contents()

* Ignore deprecation warnings in metadata.py tests, point to correct exception in test_jinja_typo in test_api_build.py

* Relax match string in error assertion for Windows

* Revert 'cleanup' code changes from #5255
  • Loading branch information
beeankha authored Jul 8, 2024
1 parent 2ba431f commit 87194f6
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 64 deletions.
122 changes: 59 additions & 63 deletions conda_build/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,27 @@
import warnings
from collections import OrderedDict
from functools import lru_cache
from os.path import isfile, join
from os.path import isdir, isfile, join
from typing import TYPE_CHECKING, NamedTuple, overload

import yaml
from bs4 import UnicodeDammit
from conda.base.context import context
from conda.base.context import locate_prefix_by_name
from conda.gateways.disk.read import compute_sum
from conda.models.match_spec import MatchSpec
from frozendict import deepfreeze

from . import exceptions, utils
from . import utils
from .config import Config, get_or_merge_config
from .deprecations import deprecated
from .exceptions import (
CondaBuildException,
CondaBuildUserError,
DependencyNeedsBuildingError,
RecipeError,
UnableToParse,
UnableToParseMissingJinja2,
)
from .features import feature_list
from .license_family import ensure_valid_license_family
from .utils import (
Expand All @@ -46,6 +55,7 @@
)

if TYPE_CHECKING:
from pathlib import Path
from typing import Any, Literal, Self

OutputDict = dict[str, Any]
Expand Down Expand Up @@ -336,12 +346,12 @@ def select_lines(text: str, namespace: dict[str, Any], variants_in_place: bool)
if value:
lines.append(line)
except Exception as e:
sys.exit(
f"Error: Invalid selector in meta.yaml line {i + 1}:\n"
f"offending line:\n"
f"{line}\n"
raise CondaBuildUserError(
f"Invalid selector in meta.yaml line {i + 1}:\n"
f"offending selector:\n"
f" [{selector}]\n"
f"exception:\n"
f"{e.__class__.__name__}: {e}\n"
f" {e.__class__.__name__}: {e}\n"
)
return "\n".join(lines) + "\n"

Expand All @@ -356,10 +366,10 @@ def yamlize(data):

jinja2 # Avoid pyflakes failure: 'jinja2' imported but unused
except ImportError:
raise exceptions.UnableToParseMissingJinja2(original=e)
raise UnableToParseMissingJinja2(original=e)
print("Problematic recipe:", file=sys.stderr)
print(data, file=sys.stderr)
raise exceptions.UnableToParse(original=e)
raise UnableToParse(original=e)


def ensure_valid_fields(meta):
Expand Down Expand Up @@ -400,9 +410,7 @@ def _trim_None_strings(meta_dict):
def ensure_valid_noarch_value(meta):
build_noarch = meta.get("build", {}).get("noarch")
if build_noarch and build_noarch not in NOARCH_TYPES:
raise exceptions.CondaBuildException(
f"Invalid value for noarch: {build_noarch}"
)
raise CondaBuildException(f"Invalid value for noarch: {build_noarch}")


def _get_all_dependencies(metadata, envs=("host", "build", "run")):
Expand Down Expand Up @@ -444,7 +452,7 @@ def check_circular_dependencies(
error = "Circular dependencies in recipe: \n"
for pair in pairs:
error += " {} <-> {}\n".format(*pair)
raise exceptions.RecipeError(error)
raise RecipeError(error)


def _check_circular_dependencies(
Expand Down Expand Up @@ -477,7 +485,7 @@ def _check_circular_dependencies(
error = "Circular dependencies in recipe: \n"
for pair in pairs:
error += " {} <-> {}\n".format(*pair)
raise exceptions.RecipeError(error)
raise RecipeError(error)


def _check_run_constrained(metadata_tuples):
Expand All @@ -495,7 +503,7 @@ def _check_run_constrained(metadata_tuples):
f"Reason: {exc}"
)
if errors:
raise exceptions.RecipeError("\n".join(["", *errors]))
raise RecipeError("\n".join(["", *errors]))


def _variants_equal(metadata, output_metadata):
Expand Down Expand Up @@ -539,7 +547,7 @@ def ensure_matching_hashes(output_metadata):
error += "Mismatching package: {} (id {}); dep: {}; consumer package: {}\n".format(
*prob
)
raise exceptions.RecipeError(
raise RecipeError(
"Mismatching hashes in recipe. Exact pins in dependencies "
"that contribute to the hash often cause this. Can you "
"change one or more exact pins to version bound constraints?\n"
Expand Down Expand Up @@ -766,19 +774,18 @@ def _git_clean(source_meta):
If more than one field is used to specified, exit
and complain.
"""

git_rev_tags_old = ("git_branch", "git_tag")
git_rev = "git_rev"

git_rev_tags = (git_rev,) + git_rev_tags_old

has_rev_tags = tuple(bool(source_meta.get(tag, "")) for tag in git_rev_tags)
if sum(has_rev_tags) > 1:
msg = "Error: multiple git_revs:"
msg += ", ".join(
f"{key}" for key, has in zip(git_rev_tags, has_rev_tags) if has
)
sys.exit(msg)

keys = [key for key in (git_rev, "git_branch", "git_tag") if key in source_meta]
if not keys:
# git_branch, git_tag, nor git_rev specified, return as-is
return source_meta
elif len(keys) > 1:
raise CondaBuildUserError(f"Multiple git_revs: {', '.join(keys)}")

# make a copy of the input so we have no side-effects
ret_meta = source_meta.copy()
Expand All @@ -801,15 +808,16 @@ def _str_version(package_meta):
return package_meta


def check_bad_chrs(s, field):
bad_chrs = "=@#$%^&*:;\"'\\|<>?/ "
def check_bad_chrs(value: str, field: str) -> None:
bad_chrs = set("=@#$%^&*:;\"'\\|<>?/ ")
if field in ("package/version", "build/string"):
bad_chrs += "-"
bad_chrs.add("-")
if field != "package/version":
bad_chrs += "!"
for c in bad_chrs:
if c in s:
sys.exit(f"Error: bad character '{c}' in {field}: {s}")
bad_chrs.add("!")
if invalid := bad_chrs.intersection(value):
raise CondaBuildUserError(
f"Bad character(s) ({''.join(sorted(invalid))}) in {field}: {value}."
)


def get_package_version_pin(build_reqs, name):
Expand Down Expand Up @@ -882,20 +890,17 @@ def build_string_from_metadata(metadata):
return build_str


# This really belongs in conda, and it is int conda.cli.common,
# but we don't presently have an API there.
def _get_env_path(env_name_or_path):
if not os.path.isdir(env_name_or_path):
for envs_dir in list(context.envs_dirs) + [os.getcwd()]:
path = os.path.join(envs_dir, env_name_or_path)
if os.path.isdir(path):
env_name_or_path = path
break
bootstrap_metadir = os.path.join(env_name_or_path, "conda-meta")
if not os.path.isdir(bootstrap_metadir):
print(f"Bootstrap environment '{env_name_or_path}' not found")
sys.exit(1)
return env_name_or_path
@deprecated(
"24.7", "24.9", addendum="Use `conda.base.context.locate_prefix_by_name` instead."
)
def _get_env_path(
env_name_or_path: str | os.PathLike | Path,
) -> str | os.PathLike | Path:
return (
env_name_or_path
if isdir(env_name_or_path)
else locate_prefix_by_name(env_name_or_path)
)


def _get_dependencies_from_environment(env_name_or_path):
Expand Down Expand Up @@ -1124,7 +1129,7 @@ def finalize_outputs_pass(
fm.name(),
deepfreeze({k: fm.config.variant[k] for k in fm.get_used_vars()}),
] = (output_d, fm)
except exceptions.DependencyNeedsBuildingError as e:
except DependencyNeedsBuildingError as e:
if not permit_unsatisfiable_variants:
raise
else:
Expand Down Expand Up @@ -1427,19 +1432,20 @@ def parse_until_resolved(
):
"""variant contains key-value mapping for additional functions and values
for jinja2 variables"""
# undefined_jinja_vars is refreshed by self.parse again
undefined_jinja_vars = ()
# store the "final" state that we think we're in. reloading the meta.yaml file
# can reset it (to True)
final = self.final
# always parse again at least once.

# always parse again at least once
self.parse_again(
permit_undefined_jinja=True,
allow_no_other_outputs=allow_no_other_outputs,
bypass_env_check=bypass_env_check,
)
self.final = final

# recursively parse again so long as each iteration has fewer undefined jinja variables
undefined_jinja_vars = ()
while set(undefined_jinja_vars) != set(self.undefined_jinja_vars):
undefined_jinja_vars = self.undefined_jinja_vars
self.parse_again(
Expand All @@ -1448,18 +1454,8 @@ def parse_until_resolved(
bypass_env_check=bypass_env_check,
)
self.final = final
if undefined_jinja_vars:
self.parse_again(
permit_undefined_jinja=False,
allow_no_other_outputs=allow_no_other_outputs,
bypass_env_check=bypass_env_check,
)
sys.exit(
f"Undefined Jinja2 variables remain ({self.undefined_jinja_vars}). Please enable "
"source downloading and try again."
)

# always parse again at the end, too.
# always parse again at the end without permit_undefined_jinja
self.parse_again(
permit_undefined_jinja=False,
allow_no_other_outputs=allow_no_other_outputs,
Expand Down Expand Up @@ -2116,8 +2112,8 @@ def _get_contents(
except jinja2.TemplateError as ex:
if "'None' has not attribute" in str(ex):
ex = "Failed to run jinja context function"
sys.exit(
f"Error: Failed to render jinja template in {self.meta_path}:\n{str(ex)}"
raise CondaBuildUserError(
f"Failed to render jinja template in {self.meta_path}:\n{str(ex)}"
)
finally:
if "CONDA_BUILD_STATE" in os.environ:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_api_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def test_recursive_fail(testing_config):

@pytest.mark.sanity
def test_jinja_typo(testing_config):
with pytest.raises(SystemExit, match="GIT_DSECRIBE_TAG"):
with pytest.raises(CondaBuildUserError, match="GIT_DSECRIBE_TAG"):
api.build(
os.path.join(fail_dir, "source_git_jinja2_oops"), config=testing_config
)
Expand Down
74 changes: 74 additions & 0 deletions tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import subprocess
import sys
from contextlib import nullcontext
from itertools import product
from typing import TYPE_CHECKING

Expand All @@ -15,12 +16,15 @@

from conda_build import api
from conda_build.config import Config
from conda_build.exceptions import CondaBuildUserError
from conda_build.metadata import (
FIELDS,
OPTIONALLY_ITERABLE_FIELDS,
MetaData,
_hash_dependencies,
check_bad_chrs,
get_selectors,
sanitize,
select_lines,
yamlize,
)
Expand All @@ -30,6 +34,8 @@
from .utils import metadata_dir, metadata_path, thisdir

if TYPE_CHECKING:
from pathlib import Path

from pytest import MonkeyPatch


Expand Down Expand Up @@ -200,6 +206,7 @@ def test_clobber_section_data(testing_metadata):


@pytest.mark.serial
@pytest.mark.filterwarnings("ignore", category=PendingDeprecationWarning)
def test_build_bootstrap_env_by_name(testing_metadata):
assert not any(
"git" in pkg for pkg in testing_metadata.meta["requirements"].get("build", [])
Expand All @@ -218,6 +225,7 @@ def test_build_bootstrap_env_by_name(testing_metadata):
subprocess.check_call(cmd.split())


@pytest.mark.filterwarnings("ignore", category=PendingDeprecationWarning)
def test_build_bootstrap_env_by_path(testing_metadata):
assert not any(
"git" in pkg for pkg in testing_metadata.meta["requirements"].get("build", [])
Expand Down Expand Up @@ -549,3 +557,69 @@ def test_get_section(testing_metadata: MetaData):
assert isinstance(section, list)
else:
assert isinstance(section, dict)


def test_select_lines_invalid():
with pytest.raises(
CondaBuildUserError,
match=r"Invalid selector in meta\.yaml",
):
select_lines("text # [{bad]", {}, variants_in_place=True)


@pytest.mark.parametrize(
"keys,expected",
[
pytest.param([], {}, id="git_tag"),
pytest.param(["git_tag"], {"git_rev": "rev"}, id="git_tag"),
pytest.param(["git_branch"], {"git_rev": "rev"}, id="git_branch"),
pytest.param(["git_rev"], {"git_rev": "rev"}, id="git_rev"),
pytest.param(["git_tag", "git_branch"], None, id="git_tag + git_branch"),
pytest.param(["git_tag", "git_rev"], None, id="git_tag + git_rev"),
pytest.param(["git_branch", "git_rev"], None, id="git_branch + git_rev"),
pytest.param(
["git_tag", "git_branch", "git_rev"],
None,
id="git_tag + git_branch + git_rev",
),
],
)
def test_sanitize_source(keys: list[str], expected: dict[str, str] | None) -> None:
with pytest.raises(
CondaBuildUserError,
match=r"Multiple git_revs:",
) if expected is None else nullcontext():
assert sanitize({"source": {key: "rev" for key in keys}}) == {
"source": expected
}


@pytest.mark.parametrize(
"value,field,invalid",
[
pytest.param("good", "field", None, id="valid field"),
pytest.param("!@d&;-", "field", "!&;@", id="invalid field"),
pytest.param("good", "package/version", None, id="valid package/version"),
pytest.param("!@d&;-", "package/version", "&-;@", id="invalid package/version"),
pytest.param("good", "build/string", None, id="valid build/string"),
pytest.param("!@d&;-", "build/string", "!&-;@", id="invalid build/string"),
],
)
def test_check_bad_chrs(value: str, field: str, invalid: str) -> None:
with pytest.raises(
CondaBuildUserError,
match=rf"Bad character\(s\) \({invalid}\) in {field}: {value}\.",
) if invalid else nullcontext():
check_bad_chrs(value, field)


def test_parse_until_resolved(testing_metadata: MetaData, tmp_path: Path) -> None:
(recipe := tmp_path / (name := "meta.yaml")).write_text("{{ UNDEFINED[:2] }}")
testing_metadata._meta_path = recipe
testing_metadata._meta_name = name

with pytest.raises(
CondaBuildUserError,
match=("Failed to render jinja template"),
):
testing_metadata.parse_until_resolved()

0 comments on commit 87194f6

Please sign in to comment.