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

Invalid package METADATA? #3148

Closed
2 of 3 tasks
sam-willis opened this issue Oct 8, 2020 · 12 comments
Closed
2 of 3 tasks

Invalid package METADATA? #3148

sam-willis opened this issue Oct 8, 2020 · 12 comments
Labels
status/duplicate Duplicate issues

Comments

@sam-willis
Copy link

sam-willis commented Oct 8, 2020

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

Issue

If you install a package with a path dependency like:
test_path_dep = {path = "test_path"}

When you install the package, In your dist-info METADATA, you end up with the path dependant package listed like:
Requires-Dist: test_path_dep @ test_path

It looks to me according to https://www.python.org/dev/peps/pep-0508/ the @ symbol is used for URLS only, and that this isn't valid.

This causes the pkg_resources module's Distribution's class requires method to fail with:
pkg_resources.extern.packaging.requirements.InvalidRequirement: Invalid URL: test_path

@sam-willis sam-willis added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Oct 8, 2020
@abn
Copy link
Member

abn commented Oct 8, 2020

@sam-willis are these packes used locally only?

@sam-willis
Copy link
Author

sam-willis commented Oct 8, 2020

No, in my current use case this is a subdirectory with it's own setup.py. For something like:

my_app/
    pyproject.toml
    test_path/
        __init__.py
        main.py
        setup.py  # name of package: "test_path_dep"
        setup.cfg

EDIT: I'm actually not sure what you mean by locally - thinking more about it the answer is probably yes. The packages are always local relative to each other

@abn
Copy link
Member

abn commented Oct 8, 2020

Well, let me rephrase. Do you publish these wheels to an index? The local path dependencies in non-develop mode I guess should be written as dep @ file://<absolute path to dir> in the core metadata so as to support NFS mounts etc too.

The "is used locally" relevance is if these dependencies are editable or relative to current package. There really is no supported or agreed upon standard on how to do this in a manner that is portable. The issue with your example is that there is no real mechanism in which a wheel can include that information via the metadata unless we have, perhaps, a build step puts the package in a vendored location and the root package includes this path as an included pacage.

@sam-willis
Copy link
Author

sam-willis commented Oct 9, 2020

Ah thanks for the clarification. We do not publish the wheels to an index, so locally.

I think dep @ file://<absolute path to dir> would be valid. In my case the issue is not that I want the metadata to include the path absolute or otherwise, it's that a builtin part of python throws an exception when the package is specified in the metadata as it currently is.

I don't really have a good enough idea of the nuances of packaging to suggest what the solution should be, just that what's currently there seems wrong.

@johnpaulett
Copy link

Running into this myself. [email protected] (& older) would treat as

Requires-Dist: click (>=7.1.2,<8.0.0)
Requires-Dist: mypkg.pkg1
Requires-Dist: google-cloud-pubsub (>=2.1.0,<3.0.0)

But with [email protected] has the relative file path (pyproject.toml uses new develop tag "mypkg.pkg1" = {path = "../pkg1", develop = true}:

Requires-Dist: click (>=7.1.2,<8.0.0)
Requires-Dist: mypkg.pkg1 @ ../pkg1
Requires-Dist: google-cloud-pubsub (>=2.1.0,<3.0.0)

In my use case, I run poetry build -f wheel, assemble my local wheels into a wheel directory, then install the whl's inside docker. Perhaps that itself is a corner case, but I'm not sure I see any typical value in the Requires-Dist being aware of local build-system paths, whether in @ ../pkg1 or @ file://../pkg1 notation.

@jkowalleck
Copy link

jkowalleck commented Oct 17, 2020

having a similar issue. poetry 1.1.3

the error message on loading an entry point

pkg_resources.extern.packaging.requirements.InvalidRequirement: Invalid URL: ../package-a

my setup:

package-a:

  • iterates over entry points and load them via
    import pkg_resources
    for entry in pkg_resources.iter_entry_points("xyz"):
      entry.load()  # throws exception :-(

package-b:

  • providing an entry point via tool.poetry.plugins."xyz" in pyproject.toml
  • I am having a relative dependencie in pyproject.toml ala
    [tool.poetry.dependencies]
    PackageA = { path = "../package-a/", develop = true }

@johnpaulett
Copy link

Workaround hat unzips the .whl, to re-write the METADATA. I use this in an existing script that collects the .whls into a common directory to be COPY'ed during docker image building.

from pathlib import Path
from tempfile import TemporaryDirectory
from zipfile import ZipFile


def fix_whl(whl_path: Path, output_dir: Path) -> Path:
    with TemporaryDirectory() as td:
        tmp_dir = Path(td)
        tmp_whl = tmp_dir.joinpath(whl_path.name)

        with ZipFile(whl_path, "r") as zin:
            with ZipFile(tmp_whl, "w") as zout:
                for info in zin.infolist():
                    content = zin.read(info.filename)

                    if info.filename.endswith(".dist-info/METADATA"):
                        content = "\n".join(
                            [
                                (
                                    line[: line.index(" @ ")]
                                    if line.startswith("Requires-Dist")
                                    and " @ " in line
                                    else line
                                )
                                for line in content.decode("utf8").split("\n")
                            ]
                        ).encode("utf8")

                    zout.writestr(info, content)

        return Path(shutil.copy2(tmp_whl, output_dir))

@absassi
Copy link

absassi commented Oct 30, 2020

@abn, indeed it seems there isn't a standard for distributing wheels with relative paths, but writing name @ path is wrong according to PEP 508 grammar spec.

It seems any kind of URL or even just the distribution name without the @ is enough for pkg_resources to be happy when trying to list the dependencies of a distribution. So I'd say Poetry should write either name @ file://relative/path, name @ file:///absolute/path or just name. I guess the only use case that this choice affects is installation. I'm not sure which would be better, but if file URL with relative path works when installing the wheel, that looks to me the best thing to do.

I'm affected by this bug, because I use pkg_resources to gather metadata of all dependencies of a package for licensing purposes. Now this is broken, because it can't parse the distribution requirements of my project, that has relative path dependencies.

Also, resolving #1168 would help many people (including me) to never have path dependencies in distributed wheel files, so this minimizes the issue with installation of wheels containing path dependencies. But, of course, even with that feature, Poetry must still write correct Requires-Dist entries in the generated dist-info folders when doing editable installs.

@johnpaulett
Copy link

It seems any kind of URL or even just the distribution name without the @ is enough for pkg_resources to be happy when trying to list the dependencies of a distribution. So I'd say Poetry should write either name @ file://relative/path, name @ file:///absolute/path or just name. I guess the only use case that this choice affects is installation. I'm not sure which would be better, but if file URL with relative path works when installing the wheel, that looks to me the best thing to do.

I'd lean towards just name.

name @ file:///absolute/path requires the directory structure for the host that the .whl is installed into to match the structure of the build system.

Both the absolute and relative paths, I believe, would incorrect if those dependencies are .whl's themselves -- the file would be at file://relative/path/dist/*.whl or file:///absolute/path/dist/*.whl.

@nathanrpage97
Copy link

nathanrpage97 commented Oct 30, 2020

I have another workaround. Also not sure if this is defined behavior.

Pyproject.toml

[tool.poetry.dependencies]
...
mylib = "1.1.0"

[tool.poetry.dev-dependencies]
...
mylib = { path = "mylib-1.1.0-py3-none-any.whl", develop = false }

This has the added benefit of ensuring it is the correct version installed on a user machine. If you aren't using a wheel or don't care about a version then you can switch to mylib = "*".

@neersighted
Copy link
Member

Calling this a duplicate of #5273. This is older, but the linked issue has more relevant discussion and is up-to-date for current Poetry code.

@neersighted neersighted closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2022
@neersighted neersighted added status/duplicate Duplicate issues and removed kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Oct 23, 2022
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
status/duplicate Duplicate issues
Projects
None yet
Development

No branches or pull requests

7 participants