Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate markers under environment with empty "extra" #550

Merged
merged 3 commits into from Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.rst
Expand Up @@ -4,7 +4,9 @@ Changelog
*unreleased*
~~~~~~~~~~~~

No unreleased changes.
* ``Marker.evaluate`` will now assume evaluation environment with empty ``extra``.
Evaluating markers like ``"extra == 'xyz'"`` without passing any extra in the
``environment`` will no longer raise an exception.

21.3 - 2021-11-17
~~~~~~~~~~~~~~~~~
Expand Down
11 changes: 0 additions & 11 deletions docs/markers.rst
Expand Up @@ -34,17 +34,6 @@ Usage
>>> # Markers can be also used with extras, to pull in dependencies if
>>> # a certain extra is being installed
>>> extra = Marker('extra == "bar"')
>>> # Evaluating an extra marker with no environment is an error
>>> try:
... extra.evaluate()
... except UndefinedEnvironmentName:
... pass
>>> extra_environment = {'extra': ''}
>>> extra.evaluate(environment=extra_environment)
False
>>> extra_environment['extra'] = 'bar'
>>> extra.evaluate(environment=extra_environment)
True
>>> # You can do simple comparisons between marker objects:
>>> Marker("python_version > '3.6'") == Marker("python_version > '3.6'")
True
Expand Down
23 changes: 3 additions & 20 deletions packaging/markers.py
Expand Up @@ -215,24 +215,6 @@ def _eval_op(lhs: str, op: Op, rhs: str) -> bool:
return oper(lhs, rhs)


class Undefined:
pass


_undefined = Undefined()


def _get_env(environment: Dict[str, str], name: str) -> str:
value: Union[str, Undefined] = environment.get(name, _undefined)

if isinstance(value, Undefined):
raise UndefinedEnvironmentName(
f"{name!r} does not exist in evaluation environment."
)

return value
MrMino marked this conversation as resolved.
Show resolved Hide resolved


def _normalize(*values: str, key: str) -> Tuple[str, ...]:
# PEP 685 – Comparison of extra names for optional distribution dependencies
# https://peps.python.org/pep-0685/
Expand All @@ -258,12 +240,12 @@ def _evaluate_markers(markers: List[Any], environment: Dict[str, str]) -> bool:

if isinstance(lhs, Variable):
environment_key = lhs.value
lhs_value = _get_env(environment, environment_key)
lhs_value = environment[environment_key]
rhs_value = rhs.value
else:
lhs_value = lhs.value
environment_key = rhs.value
rhs_value = _get_env(environment, environment_key)
rhs_value = environment[environment_key]

lhs_value, rhs_value = _normalize(lhs_value, rhs_value, key=environment_key)
groups[-1].append(_eval_op(lhs_value, op, rhs_value))
Expand All @@ -287,6 +269,7 @@ def default_environment() -> Dict[str, str]:
iver = format_full_version(sys.implementation.version)
implementation_name = sys.implementation.name
return {
"extra": "",
"implementation_name": implementation_name,
"implementation_version": iver,
"os_name": os.name,
Expand Down
9 changes: 3 additions & 6 deletions tests/test_markers.py
Expand Up @@ -15,7 +15,6 @@
Marker,
Node,
UndefinedComparison,
UndefinedEnvironmentName,
default_environment,
format_full_version,
)
Expand Down Expand Up @@ -110,6 +109,7 @@ def test_matches_expected(self):
)

assert environment == {
"extra": "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since default_environment is part of the public API (pending documentation #503), I wonder if we should avoid adding extra to it.

"implementation_name": sys.implementation.name,
"implementation_version": iver,
"os_name": os.name,
Expand Down Expand Up @@ -253,11 +253,8 @@ def test_compare_markers_to_other_objects(self):
# Markers should not be comparable to other kinds of objects.
assert Marker("os_name == 'nt'") != "os_name == 'nt'"

def test_extra_with_no_extra_in_environment(self):
# We can't evaluate an extra if no extra is passed into the environment
m = Marker("extra == 'security'")
with pytest.raises(UndefinedEnvironmentName):
m.evaluate()
def test_environment_assumes_empty_extra(self):
assert Marker('extra == "im_valid"').evaluate() is False

@pytest.mark.parametrize(
("marker_string", "environment", "expected"),
Expand Down