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.

Sign up for GitHub

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

Introduce dependency sorting #3996

Closed
wants to merge 7 commits into from
Closed

Introduce dependency sorting #3996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
96e0fc5
command/add: introduce dependency sorting
br3ndonland Dec 19, 2021
c377365
tests: prefer actual-expected to expected-actual
br3ndonland Apr 16, 2022
c45dd9d
docs: update dependency sorting docs after review
br3ndonland Apr 16, 2022
e0b3946
command/init: introduce dependency sorting
br3ndonland Apr 17, 2022
0d123d8
command/init: revise dependency sorting
br3ndonland Apr 17, 2022
5e9f7da
command/add: rename dependency group to main
br3ndonland Jun 11, 2022
e9a353e
tests: use stable django-pendulum in sorting tests
br3ndonland Jun 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ If you encounter any problems with it, set to `true` to use the system git backe

Defaults to `false`.

### `dependencies.sort`

**Type**: boolean

Sort dependencies in `pyproject.toml` alphabetically when adding.

Defaults to `false`.

### `installer.parallel`

**Type**: boolean
Expand Down
2 changes: 2 additions & 0 deletions src/poetry/config/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def validator(cls, policy: str) -> bool:
class Config:
default_config: dict[str, Any] = {
"cache-dir": str(DEFAULT_CACHE_DIR),
"dependencies": {"sort": False},
"virtualenvs": {
"create": True,
"in-project": None,
Expand Down Expand Up @@ -260,6 +261,7 @@ def resolve_from_config(match: re.Match[str]) -> Any:
@staticmethod
def _get_normalizer(name: str) -> Callable[[str], Any]:
if name in {
"dependencies.sort",
"virtualenvs.create",
"virtualenvs.in-project",
"virtualenvs.options.always-copy",
Expand Down
10 changes: 10 additions & 0 deletions src/poetry/console/commands/add.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import contextlib

from collections import OrderedDict
from typing import Any

from cleo.helpers import argument
Expand Down Expand Up @@ -230,6 +231,15 @@ def handle(self) -> int:
)
)

if self.poetry.config.get("dependencies.sort") is True:
sorted_dependencies = OrderedDict(sorted(section.items()))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more issue here: casting section to dictionary causes loss of comments in project file. That might be critical for some projects. Please, try to fix the issue

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more issue here: casting section to dictionary causes loss of comments in project file. That might be critical for some projects. Please, try to fix the issue

If comments were preserved, where would they go? We could probably preserve inline comments, but single line comments would be more difficult.

The most common use case for comments that I have seen is breaking up dependencies into subsections, like this:

[tool.poetry.dev-dependencies]
# tests
pytest = "*"
# docs
mkdocs = "*"

If the dependencies were sorted, where would the comments go? It would probably end up like this:

[tool.poetry.dev-dependencies]
# tests
# docs
mkdocs = "*"
pytest = "*"

A more effective approach might be to remove the comments and separate these commented sections into Poetry 1.2 dependency groups.

[tool.poetry.group.test.dependencies]
pytest = "*"

[tool.poetry.group.docs.dependencies]
mkdocs = "*"

I'm happy to try preserving comments if there is a suggested data structure from TOML Kit.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also comments might be used to temporarily disable a dependency or explain a dependency directly below the comment.
In these cases, it's quite important to keep those comments. I think it would be good to make the comments stick to the next dependency line:

[tool.poetry.dependencies]
simplejson = "^3.17.5"  # used in requests to serialize decimals
# Consider to replace requests with httpx 
# to improve performance.
# httpx = "^0.23.0"
requests = "^2.26.0"
fastapi = "^1"

should turn into

[tool.poetry.dependencies]
fastapi = "^1"
# Consider to replace requests with httpx 
# to improve performance.
# httpx = "^0.23.0"
requests = "^2.26.0"
simplejson = "^3.17.5"  # used in requests to serialize decimals

Your example is about a case that can be resolved using poetry functional, as you mentioned. Users should be responsible for proper use of Poetry. So I would keep comments in this case as well

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to use tomlkit properly. But I can just share my thoughts.

It seems comment lines are stored in section.value.body list with None as a key and Comment(..) as a value. You can't get an access to the comment lines via section.items() method. I didn't find any other way to access to the comments but section.value.body. And I don't see methods to remove comment lines from the section (neither section.remove() nor section.clear() don't remove the comments). So, you should re-create an entire section and append sorted lines into it, or try to sort section.value.body in-place.

if sorted_dependencies.get("python"):
sorted_dependencies.move_to_end("python", last=False)
if group == MAIN_GROUP:
poetry_content["dependencies"] = sorted_dependencies
else:
poetry_content["group"][group]["dependencies"] = sorted_dependencies

# Refresh the locker
self.poetry.set_locker(
self.poetry.locker.__class__(self.poetry.locker.lock.path, poetry_content)
Expand Down
1 change: 1 addition & 0 deletions src/poetry/console/commands/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def unique_config_values(self) -> dict[str, tuple[Any, Any, Any]]:
lambda val: str(Path(val)),
str(DEFAULT_CACHE_DIR / "virtualenvs"),
),
"dependencies.sort": (boolean_validator, boolean_normalizer, False),
"virtualenvs.create": (boolean_validator, boolean_normalizer, True),
"virtualenvs.in-project": (boolean_validator, boolean_normalizer, False),
"virtualenvs.options.always-copy": (
Expand Down
7 changes: 7 additions & 0 deletions src/poetry/console/commands/init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import sys

from collections import OrderedDict
from pathlib import Path
from typing import TYPE_CHECKING
from typing import Any
Expand Down Expand Up @@ -73,6 +74,7 @@ def handle(self) -> int:
from poetry.core.pyproject.toml import PyProjectTOML
from poetry.core.vcs.git import GitConfig

from poetry.config.config import Config
from poetry.layouts import layout
from poetry.utils.env import SystemEnv

Expand All @@ -93,6 +95,7 @@ def handle(self) -> int:
)
return 1

config = Config()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to read the global config, instead of adding a new flag to init (#3996 (comment))

vcs_config = GitConfig()

if self.io.is_interactive():
Expand Down Expand Up @@ -208,6 +211,10 @@ def handle(self) -> int:
if self.io.is_interactive():
self.line("")

if config.get("dependencies.sort") is True:
requirements = OrderedDict(sorted(requirements.items()))
dev_requirements = OrderedDict(sorted(dev_requirements.items()))

layout_ = layout("standard")(
name,
version,
Expand Down
7 changes: 6 additions & 1 deletion tests/config/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ def get_options_based_on_normalizer(normalizer: Callable) -> str:


@pytest.mark.parametrize(
("name", "value"), [("installer.parallel", True), ("virtualenvs.create", True)]
("name", "value"),
[
("dependencies.sort", False),
("installer.parallel", True),
("virtualenvs.create", True),
],
)
def test_config_get_default_value(config: Config, name: str, value: bool):
assert config.get(name) is value
Expand Down
60 changes: 60 additions & 0 deletions tests/console/commands/test_add.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
from __future__ import annotations

import os
import sys

from collections import OrderedDict
from pathlib import Path
from typing import TYPE_CHECKING

import pytest

from poetry.core.packages.dependency_group import MAIN_GROUP
from poetry.core.semver.version import Version

from poetry.repositories.legacy_repository import LegacyRepository
Expand Down Expand Up @@ -818,6 +821,63 @@ def test_add_constraint_with_platform(
}


@pytest.mark.parametrize("group", (MAIN_GROUP, "dev", "test"))
@pytest.mark.parametrize("sort_config_value", ("true", "false", None))
def test_add_constraint_with_sort(
group: str,
sort_config_value: str | None,
app: PoetryTestApplication,
repo: TestRepository,
tester: CommandTester,
):
if sort_config_value:
os.environ["POETRY_DEPENDENCIES_SORT"] = sort_config_value
group_option = f" --group {group}" if group != MAIN_GROUP else ""

repo.add_package(get_package("pendulum", "1.4.4"))
repo.add_package(get_package("cachy", "0.2.0"))
repo.add_package(get_package("cleo", "0.6.5"))

tester.execute(f"pendulum=1.4.4 cachy=0.2.0 cleo=0.6.5{group_option}")

expected_output = """\

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 3 installs, 0 updates, 0 removals

• Installing pendulum (1.4.4)
• Installing cachy (0.2.0)
• Installing cleo (0.6.5)
"""

expected_set = set(expected_output.splitlines())
output_set = set(tester.io.fetch_output().splitlines())
assert output_set == expected_set
assert tester.command.installer.executor.installations_count == 3

expected_content = (
OrderedDict(cachy="0.2.0", cleo="0.6.5", pendulum="1.4.4")
if sort_config_value == "true"
else OrderedDict(pendulum="1.4.4", cachy="0.2.0", cleo="0.6.5")
)
if group == MAIN_GROUP:
expected_content.update(python="~2.7 || ^3.4")
expected_content.move_to_end("python", last=False)
poetry_content = OrderedDict(
app.poetry.file.read()["tool"]["poetry"]["dependencies"]
)
else:
poetry_content = OrderedDict(
app.poetry.file.read()["tool"]["poetry"]["group"][group]["dependencies"]
)

assert poetry_content == expected_content


def test_add_constraint_with_source(
app: PoetryTestApplication, poetry: Poetry, tester: CommandTester
):
Expand Down
3 changes: 3 additions & 0 deletions tests/console/commands/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def test_list_displays_default_value_if_not_set(
cache_dir = json.dumps(str(config_cache_dir))
venv_path = json.dumps(os.path.join("{cache-dir}", "virtualenvs"))
expected = f"""cache-dir = {cache_dir}
dependencies.sort = false
experimental.new-installer = true
experimental.system-git-client = false
installer.max-workers = null
Expand Down Expand Up @@ -80,6 +81,7 @@ def test_list_displays_set_get_setting(
cache_dir = json.dumps(str(config_cache_dir))
venv_path = json.dumps(os.path.join("{cache-dir}", "virtualenvs"))
expected = f"""cache-dir = {cache_dir}
dependencies.sort = false
experimental.new-installer = true
experimental.system-git-client = false
installer.max-workers = null
Expand Down Expand Up @@ -133,6 +135,7 @@ def test_list_displays_set_get_local_setting(
cache_dir = json.dumps(str(config_cache_dir))
venv_path = json.dumps(os.path.join("{cache-dir}", "virtualenvs"))
expected = f"""cache-dir = {cache_dir}
dependencies.sort = false
experimental.new-installer = true
experimental.system-git-client = false
installer.max-workers = null
Expand Down
144 changes: 144 additions & 0 deletions tests/console/commands/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,81 @@ def test_noninteractive(
assert 'pytest = "^3.6.0"' in toml_content


def test_noninteractive_with_sort(
app: PoetryTestApplication,
mocker: MockerFixture,
poetry: Poetry,
repo: TestRepository,
tmp_path: Path,
):
mocker.patch.dict(os.environ, clear=True)
os.environ["POETRY_DEPENDENCIES_SORT"] = "true"

command = app.find("init")
command._pool = poetry.pool

repo.add_package(get_package("django-pendulum", "0.1.5"))
repo.add_package(get_package("pendulum", "2.0.0"))
repo.add_package(get_package("pytest-requests", "0.2.0"))
repo.add_package(get_package("pytest", "3.6.0"))
repo.add_package(get_package("flask", "2.0.0"))

p = mocker.patch("pathlib.Path.cwd")
p.return_value = tmp_path

tester = CommandTester(command)

args = (
"--name my-package "
"--description 'This is a description' "
"--author 'Your Name <you@example.com>' "
"--python '~2.7 || ^3.6' "
"--dependency django-pendulum "
"--dependency pendulum "
"--dependency flask "
"--dev-dependency pytest-requests "
"--dev-dependency pytest "
"--license MIT"
)
tester.execute(args=args, interactive=False)
expected_output = (
"Using version ^0.1.5 for django-pendulum\n"
"Using version ^2.0.0 for pendulum\n"
"Using version ^2.0.0 for flask\n"
"Using version ^0.2.0 for pytest-requests\n"
"Using version ^3.6.0 for pytest\n"
)
assert tester.io.fetch_output() == expected_output
assert tester.io.fetch_error() == ""

expected_pyproject_content = """\
[tool.poetry]
name = "my-package"
version = "0.1.0"
description = "This is a description"
authors = ["Your Name <you@example.com>"]
license = "MIT"
readme = "README.md"
packages = [{include = "my_package"}]

[tool.poetry.dependencies]
python = "~2.7 || ^3.6"
django-pendulum = "^0.1.5"
flask = "^2.0.0"
pendulum = "^2.0.0"

[tool.poetry.group.dev.dependencies]
pytest = "^3.6.0"
pytest-requests = "^0.2.0"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
"""
pyproject_content = (tmp_path / "pyproject.toml").read_text()
assert pyproject_content == expected_pyproject_content


def test_interactive_with_dependencies(tester: CommandTester, repo: TestRepository):
repo.add_package(get_package("django-pendulum", "0.1.6-pre4"))
repo.add_package(get_package("pendulum", "2.0.0"))
Expand Down Expand Up @@ -222,6 +297,75 @@ def test_interactive_with_dependencies_and_no_selection(
assert expected in tester.io.fetch_output()


def test_interactive_with_dependencies_and_sort(
mocker: MockerFixture, repo: TestRepository, tester: CommandTester
):
mocker.patch.dict(os.environ, clear=True)
os.environ["POETRY_DEPENDENCIES_SORT"] = "true"

repo.add_package(get_package("django-pendulum", "0.1.5"))
repo.add_package(get_package("pendulum", "2.0.0"))
repo.add_package(get_package("pytest-requests", "0.2.0"))
repo.add_package(get_package("pytest", "3.6.0"))
repo.add_package(get_package("flask", "2.0.0"))

inputs = [
"my-package", # Package name
"1.2.3", # Version
"This is a description", # Description
"n", # Author
"MIT", # License
"~2.7 || ^3.6", # Python
"", # Interactive packages
"pendulu", # Search for package
"1", # Second option is pendulum
"", # Do not set constraint
"django-pendulum",
"0",
"",
"Flask",
"0",
"",
"", # Stop searching for packages
"", # Interactive dev packages
"pytest-requests",
"0",
"",
"pytest", # Search for package
"0",
"",
"",
"\n", # Generate
]
tester.execute(inputs="\n".join(inputs))

expected = """\
[tool.poetry]
name = "my-package"
version = "1.2.3"
description = "This is a description"
authors = ["Your Name <you@example.com>"]
license = "MIT"
readme = "README.md"
packages = [{include = "my_package"}]

[tool.poetry.dependencies]
python = "~2.7 || ^3.6"
django-pendulum = "^0.1.5"
flask = "^2.0.0"
pendulum = "^2.0.0"

[tool.poetry.group.dev.dependencies]
pytest = "^3.6.0"
pytest-requests = "^0.2.0"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
"""
assert expected in tester.io.fetch_output()


def test_empty_license(tester: CommandTester):
inputs = [
"my-package", # Package name
Expand Down