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

Lockfile support generates an incorrect lockfile for any direct reference requirement. #14020

Closed
jsirois opened this issue Dec 29, 2021 · 2 comments
Labels
backend: Python Python backend-related issues bug

Comments

@jsirois
Copy link
Contributor

jsirois commented Dec 29, 2021

Pants, via pkg_resources.Requirement, handles direct reference requirements as described here: https://www.python.org/dev/peps/pep-0440/#direct-references

These requirements parse with no version as demonstrated here:

$ pex setuptools -- -c 'from pkg_resources import Requirement; req = Requirement.parse("darglint @ git+https://github.com/thejcannon/darglint@XYZ"); print(f"req: {req} spec: {req.specifier}")'
req: darglint@ git+https://github.com/thejcannon/darglint@XYZ spec:

This is problematic with our current lock file implementaion using Poetry since:

version=str(requirement.specifier) or None, # type: ignore[attr-defined]

metadata: dict[str, Any] = {"version": dep.version or "*"}

def create_pyproject_toml_as_dict(
raw_requirements: Iterable[str], interpreter_constraints: InterpreterConstraints
) -> dict:
python_constraint = {"python": interpreter_constraints.to_poetry_constraint()}
project_name_to_poetry_deps = defaultdict(list)
for raw_req in raw_requirements:
# WONTFIX(#12314): add error handling.
req = Requirement.parse(raw_req)
poetry_dep = PoetryDependency.from_requirement(req)

That generates a pyproject.toml with an incorrect dependency entry for every direct reference requirement we are trying to lock using poetry lock on the pyproject.toml. Instead of getting a lock on the direct reference requirement you specify and instead of failing to indicate we can't generate a proper lock for these, we silently generate a lock against the latest publically available version of the project on PyPI (since we say version = "*").

@jsirois jsirois added the bug label Dec 29, 2021
@thejcannon thejcannon added the backend: Python Python backend-related issues label Jun 7, 2022
Eric-Arellano added a commit that referenced this issue Aug 5, 2022
Poetry lockfile generation via Pants has several issues, including:

* transitive deps often have bad environment markers
* `[python-repos]` is not hooked up
* `[GLOBAL].ca_certs_path` is not hooked up
* VCS and local requirements don't work
* Poetry writes to a cache not controlled by Pants, due to their bug
* #15315
* #15171
* #14020

Some of these are fixable, whether by the upcoming Poetry 1.2 or by us investing more time. However—given the project's limited resources—it is not beneficial enough to lean into Poetry lockfile generation. Poetry was always seen as a bridge technology while we developed Pex lockfiles, which now meet all of Pantsbuild's requirements.

## Keeps manually generated lockfiles

Users can still manually generate requirements.txt-style lockfiles. This is an important escape hatch if Pex lockfiles don't work for someone, and it eases migration from e.g. Poetry projects still using poetry.lock.

We may want to revisit this decision and require Pex lockfiles in the future, but that is a later decision.

[ci skip-rust]
cczona pushed a commit to cczona/pants that referenced this issue Sep 1, 2022
Poetry lockfile generation via Pants has several issues, including:

* transitive deps often have bad environment markers
* `[python-repos]` is not hooked up
* `[GLOBAL].ca_certs_path` is not hooked up
* VCS and local requirements don't work
* Poetry writes to a cache not controlled by Pants, due to their bug
* pantsbuild#15315
* pantsbuild#15171
* pantsbuild#14020

Some of these are fixable, whether by the upcoming Poetry 1.2 or by us investing more time. However—given the project's limited resources—it is not beneficial enough to lean into Poetry lockfile generation. Poetry was always seen as a bridge technology while we developed Pex lockfiles, which now meet all of Pantsbuild's requirements.

## Keeps manually generated lockfiles

Users can still manually generate requirements.txt-style lockfiles. This is an important escape hatch if Pex lockfiles don't work for someone, and it eases migration from e.g. Poetry projects still using poetry.lock.

We may want to revisit this decision and require Pex lockfiles in the future, but that is a later decision.

[ci skip-rust]
@cognifloyd
Copy link
Member

Can this be closed now that we use pex lockfiles?

@jsirois
Copy link
Contributor Author

jsirois commented Mar 14, 2023

Yes. Pex does proper hashing of these on both the generate and consume side.

@jsirois jsirois closed this as completed Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Development

No branches or pull requests

3 participants