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

Path dependencies specialization at build time #5495

Closed
2 tasks done
mindstorm38 opened this issue Apr 26, 2022 · 6 comments
Closed
2 tasks done

Path dependencies specialization at build time #5495

mindstorm38 opened this issue Apr 26, 2022 · 6 comments
Labels
kind/feature Feature requests/implementations

Comments

@mindstorm38
Copy link

  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have searched the documentation and believe that my question is not covered.

Feature Request

The main issue I want to address is a "bug" affecting "path dependencies", this issue has already been described in #3148 if you need more context. To sum up, such dependencies need a path to another project instead of a version. When building a poetry project (e.g. for publishing), the output archives .whl and .tar.gz both contains a metadata file, named METADATA in wheels, and PKG-INFO in tarball. In this file, each poetry dependency (except python one) are translated to a Requires-Dist one. For version constraints it works as usual, but for path dependencies we can get something like this Requires-Dist: foo @ ../bar, where foo is the package's name and ../bar is its location. But in reality of production, it will never work, it's only suitable for testing (and is addressed in issue #1168).

I also know that an alternative has already been suggested using dev-dependencies with path-dependencies, and dependencies with actual versions, but for me, it remains complicated to maintain dependencies versions.

The goal is to fix this by specializing the version of path-dependencies at build time, to have a fixed Requires-Dist field in the metadata. Because multi-repos (without usage of submodules) cannot use this path-dependency feature, I'll only be talking about monorepo workspaces (which are useful for small projects), but this proposal remains compatible with multi-repos and PyPI publishing.

From current specification (from official documentation):

[tool.poetry.dependencies]
# directory
my-package = { path = "../my-package/", develop = false }
# file
my-package = { path = "../my-package/dist/my-package-0.1.0.tar.gz" }

This schema will remain valid, but will be transposed to Requires-Dist: my-package (>=0.1.0, <0.2.0) in metadata, this version can be known from the pyproject.toml file in a directory-based path, from PKG-INFO for tarball-based paths or from METADATA for wheels. By default, we use a ~ tilde constraint to allow patch-level changes, behavior for non-semver versions is not currently specified.

Devs might want to change this default constraint, for this we need to extend the path dependency schema by adding a version key. Here are some examples:

# Simple constraints:
dep = { path = "<path>", version = ">=x" }    # actual constraint: >=1
dep = { path = "<path>", version = ">=x.x" }  # actual constraint: >=1.2
dep = { path = "<path>", version = "^x.x" }   # actual constraint: ^1.2
dep = { path = "<path>", version = "^x.x, !=1.2.0" }   # we can also use classic constraints
# By default, this constraints can be used to allow only patch updates (according to sementic versionning):
dep = { path = "<path>", version = "~x.x.x" }                 # actual constraint: ~1.2.3
dep = { path = "<path>", version = ">=x.x.x, <x.{x+1}.0" }    # equivalent to the previous one, less readable, and {x+1} syntax is ugly imo
# Disable specialization:
dep = { path = "<path>", version = false }

What do you think about this idea? I can try to work on a proof-of-concept pull request if you find that useful.

@mindstorm38 mindstorm38 added kind/feature Feature requests/implementations status/triage This issue needs to be triaged labels Apr 26, 2022
@abn
Copy link
Member

abn commented Apr 26, 2022

There are few things to keep in mind when exploring this feature.

As Poetry builds on top of PEP 517, it is essential that all PEP 517 frontends produce the same artefacts on builds. In practice this means that the following commands should produce the same wheel, with the same metadata.

poetry build
python -m pip install build
python -m build .

If I am understanding the proposal correct, this requires build time path dependency metadata inspection. The proposal as is might not be able to accomplish that as poetry-core (PEP 517 backend) does not do package inspections of path dependencies to produce the metadata. Inspections are only performed by poetry (frontend + application). This is by design.

Today, when poetry-core (used in turn by poetry), builds a distribution, the Requires-Dist metadata is generated by reading what is in the [tool.poetry.dependencies] section of the pyproject.toml and serialising them into PEP 508 specifications. The lockfile nor any dynamic inspection is not involved. This is also part of the reason why relative paths do not necessarily always work as PEP 517 builds often occur in isolated build time environments.

This means that if introducing these specifications, the implementation will be limited in such a way that the end result will be similar to, if not the same as, adding an override in the development dependencies section and managing version constraints explicitly.

In theory, if poetry-core becomes aware of lockfiles (python-poetry/poetry-core#83), this could change without having dynamic inspections - ie. read version from lockfile. Alternatively, poetry-core could call build.util.project_wheel_metadata, but that introduces its own class of problems and design issues.

@mindstorm38
Copy link
Author

mindstorm38 commented Apr 26, 2022

You are right, something is missing in my proposal. The key problem, if I understand what you said, is that distributing PEP 517 source code of individual project will break them at build time because the relative path could not be found. So my proposal is only usable within mono-repo projects if we download the whole repo. It's more complicated than I thought honestly, I've to admit that overriding dev-dependencies might be a better idea.

@abn
Copy link
Member

abn commented Apr 26, 2022

That is one of the issues, yes. Even within mono repo projects, this can become an issue. First level path dependencies are okay-ish, but nested ones become an issue. Your proposal, atleast partially, can work if core starts handling lockfiles and metadata gets generated using pyproject.toml + poetry.lock. But in the interim, the overides are the best recourse.

Python packaging in general does not have good solutions for this. There are various hurdles here. And, in my view, Poetry cannot depart from the community standards either; since that will undo all the good work that has come in recent years within the packging space by creating more fragmentation.

@mindstorm38
Copy link
Author

Constraints override is better anyway, avoiding clash with PEP 517 and the past work. Python packaging is the real mess, poetry need to follow this, so now I'll support #1168.

@mkniewallner mkniewallner removed the status/triage This issue needs to be triaged label Jun 11, 2022
@adriangb
Copy link
Contributor

adriangb commented Oct 4, 2022

I think the right thing to do here would be only allowing building wheels or sdists with path dependencies that are poetry projects. That way poetry can introspect the path dependency's pyproject.toml and get its dependencies (recursively). Then all path dependencies get bundled (just like if they had been specified in packages or include) and their dependencies get combined.

If a version constraint is also specified then that version constraint is used for the built wheel/sdist.

This is analogous to what Cargo does: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations

Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Feature requests/implementations
Projects
None yet
Development

No branches or pull requests

4 participants