Skip to content

Unify PEP 508 marker handling across all dependency sources (incl. PEP 723) and fix marker passing to pip #456

Description

@gilgamezh

Summary

PEP 508 environment markers (e.g. pkg==1.0; python_version < "3.6") are not handled consistently across fades' dependency-parsing paths, and the underlying install step mangles any requirement string that contains a marker. This needs a single shared fix once the relevant PRs land.

This issue tracks the follow-up work; I'll implement it in a separate PR after #452 (PEP 723) is merged.

Background

There are currently three places that turn requirement text into packaging.requirements.Requirement objects:

Problems

1. Marker evaluation is not shared across paths

PR #446 ("Support environment markers") adds the marker-skip logic, but as a duplicated inline block in only two functions — _parse_content and _parse_requirement:

if repo == REPO_PYPI and hasattr(dependency, 'marker') and dependency.marker is not None:
    if not dependency.marker.evaluate():
        ...
        continue

The PEP 723 path (_parse_pep723, #452) builds requirements directly:

deps[REPO_PYPI] = [Requirement(dep) for dep in dependencies]

so it never goes through that logic. If both PRs merge as-is, a # /// script block such as:

# /// script
# dependencies = ["pysha3==1.0b1; python_version < '3.6'"]
# ///

would not be filtered, even though PEP 723 explicitly defines dependencies as PEP 508 strings (markers are legal and expected there).

2. Matching markers still break pip install

The marker fix only handles the False case (skip the dep). When a marker evaluates True, the Requirement still stringifies with the marker, and pipmanager.install does:

str_dep = str(dependency)
args = [self.pip_exe, "install"] + str_dep.split()

str_dep.split() (added for multiword -e deps) shreds pkg==1.0; python_version >= "3.6" on the spaces inside the marker into separate args, which breaks the install. This is unfixed in both #446 and #452.

Proposed approach

  • Factor the marker-evaluation logic into a single shared helper (e.g. inside parse_fade_requirement, or a small _keep_for_marker(dependency) used by all three call sites) instead of duplicating it — so _parse_pep723 is covered too.
  • Fix pipmanager.install so requirements containing a marker (spaces) are passed as a single argument rather than .split()-ing them, while preserving the -e multiword behavior.
  • Add tests for markers coming from each source, including a PEP 723 block.

Related

To be implemented after #452 is merged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions