python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.81k stars 2.28k forks source link

poetry export -f requirements.txt adds extras to the extras. #1313

Closed hvdklauw closed 5 years ago

hvdklauw commented 5 years ago

Issue

When you have a dependency with extra's in your pyproject.toml and you export it to a requirements.txt file the extras argument get's added to the dependency itself:

pyproject.toml

django-anymail = {version = "^6.0", extras = ["sparkpost"]}
django-phonenumber-field = {version = "^3.0", extras = ["phonenumbers"]}

Then when I do a poetry export -f requirements.txt the output is:

...
phonenumbers==8.10.17; extra == "phonenumbers" \
    --hash=sha256:b913023be4f99a210038efd3cef5212b3d11fdf7829a0b75a0031cff3eae2d9e \
    --hash=sha256:e9752dda6abb076d528d954d16c8bc7f8682df6f42c837862d0dea88c7667908
...
sparkpost==1.3.6; extra == "sparkpost" \
    --hash=sha256:75d1a408c7a581377ac9cc2cfef1de1249e49cb631921033cc3ff115b08c2c81 \
    --hash=sha256:abaf5045d04d821f9c29c965d97173f52f6acc2051debca6f122ea7456cc1733
...

Which then when I try to pip install it results in:

Ignoring phonenumbers: markers 'extra == "phonenumbers"' don't match your environment
Ignoring sparkpost: markers 'extra == "sparkpost"' don't match your environment

This does not happen in 1.0.0a5, so I reverted back for now.

hvdklauw commented 5 years ago

in #1293 @tommilligan mentioned that the issue was introduced somewhere between 442ff52 and e987f01.

hvdklauw commented 5 years ago

Looking at the changes, it might be because of the changes in #1277

tommilligan commented 5 years ago

This following patch adds a failing test against develop, although I don't know what the expected behaviour should be.

Can be run with pytest -xk test_exporter_exports_requirements_txt_with_extras

diff --git a/tests/utils/test_exporter.py b/tests/utils/test_exporter.py
index 0ca7b1a..04b25ed 100644
--- a/tests/utils/test_exporter.py
+++ b/tests/utils/test_exporter.py
@@ -309,6 +309,58 @@ foo==1.2.3 \\

     assert expected == content

+def test_exporter_exports_requirements_txt_with_extras(tmp_dir, poetry):
+    poetry.locker.mock_lock_data(
+        {
+            "package": [
+                {
+                    "name": "foo",
+                    "version": "1.2.3",
+                    "category": "main",
+                    "optional": False,
+                    "python-versions": "*",
+                    "extras": {"feature_bar": ["bar (>=4.5.0)"]},
+                    "dependencies": {
+                        "bar": {"optional": True, "version": ">=4.5.0"}
+                    },
+                },
+                {
+                    "name": "bar",
+                    "version": "4.5.6",
+                    "category": "main",
+                    "optional": False,
+                    "python-versions": "*",
+                    "marker": "extra == \"bar\""
+                },
+            ],
+            "metadata": {
+                "python-versions": "*",
+                "content-hash": "123456789",
+                "hashes": {"foo": ["12345"], "bar": ["67890"]},
+            },
+        }
+    )
+    exporter = Exporter(poetry)
+
+    exporter.export(
+        "requirements.txt",
+        Path(tmp_dir),
+        "requirements.txt",
+        dev=False,
+    )
+
+    with (Path(tmp_dir) / "requirements.txt").open(encoding="utf-8") as f:
+        content = f.read()
+
+    expected = """\
+bar==4.5.6 \\
+    --hash=sha256:67890
+foo==1.2.3 \\
+    --hash=sha256:12345
+"""
+
+    assert expected == content
+

 def test_exporter_can_export_requirements_txt_with_git_packages(tmp_dir, poetry):
     poetry.locker.mock_lock_data(
hvdklauw commented 5 years ago

Seeing your test made me think about the lock file:

[[package]]
category = "main"
description = "An international phone number field for django models."
name = "django-phonenumber-field"
optional = false
python-versions = ">=3.5"
version = "3.0.1"

[package.dependencies]
Django = ">=1.11.3"
babel = "*"

[package.dependencies.phonenumbers]
optional = true
version = ">=7.0.2"

[package.extras]
phonenumbers = ["phonenumbers (>=7.0.2)"]
phonenumberslite = ["phonenumberslite (>=7.0.2)"]

[[package]]
category = "main"
description = "Python version of Google's common library for parsing, formatting, storing and validating international phone numbers."
marker = "extra == \"phonenumbers\""
name = "phonenumbers"
optional = false
python-versions = "*"
version = "8.10.17"

[[package]]
category = "main"
description = "Django email integration for Amazon SES, Mailgun, Mailjet, Postmark, SendGrid, SendinBlue, SparkPost and other transactional ESPs"
name = "django-anymail"
optional = false
python-versions = "*"
version = "6.1.0"

[package.dependencies]
django = ">=1.11"
requests = ">=2.4.3"
six = "*"

[package.dependencies.sparkpost]
optional = true
version = "*"

[package.extras]
amazon_ses = ["boto3"]
sparkpost = ["sparkpost"]

[[package]]
category = "main"
description = "SparkPost Python API client"
marker = "extra == \"sparkpost\""
name = "sparkpost"
optional = false
python-versions = "*"
version = "1.3.6"

So maybe the bug is that the lock file has the extra marker there for the thing that is the extra instead of the package where is was given?

hvdklauw commented 5 years ago

Bit sad that 1.0.0b2 still didn't fix the extra marker on the extra in the lock file. (talikng about the marker = "extra == \"phonenumbers\" and the marker = "extra == \"sparkpost\"" in the file above which shouldn't be there but on the original package.)

The marker is for the original package, not for the package that gets installed because of it.

tonysyu commented 5 years ago

FWIW, this still appears to be an issue in 1.0.0b3

hvdklauw commented 5 years ago

I did a lot of thinking on this.

The extras are just there to tell the package which other optional dependencies to install and let the package decide on the versions.

But when installing from the lock file or exported requirements.txt it should be ignored because those optional dependencies are already resolved and added, so the extra's should just be removed.

As a fix for now I moved those extra dependencies to my own pyproject.toml file, but this means I need to manage those myself when the main package updates.

github-actions[bot] commented 8 months ago

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.