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

factory: merge legacy dependencies and groups #305

Merged
merged 3 commits into from
Mar 18, 2022

Conversation

abn
Copy link
Member

@abn abn commented Mar 10, 2022

This change ensures that existence of legacy "dev-dependency" section does not break when "dev" group is added for new dependencies.

root@b447a66339c6:/foobar# cat pyproject.toml 
[tool.poetry]
name = "foobar"
version = "0.1.0"
description = ""
authors = ["Your Name <you@example.com>"]
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.10"

[tool.poetry.group.dev.dependencies]
pycowsay = "^0.0.0.1"

[tool.poetry.dev-dependencies]
pytest = "*"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
root@b447a66339c6:/foobar# poetry show pycowsay
 name         : pycowsay 
 version      : 0.0.0.1  
 description  :          
root@b447a66339c6:/foobar# poetry show --tree
pytest 7.0.1 pytest: simple powerful testing with Python
├── atomicwrites >=1.0
├── attrs >=19.2.0
├── colorama *
├── iniconfig *
├── packaging *
│   └── pyparsing >=2.0.2,<3.0.5 || >3.0.5 
├── pluggy >=0.12,<2.0
├── py >=1.8.2
└── tomli >=1.0.0
root@b447a66339c6:/foobar# pip uninstall poetry-core
Found existing installation: poetry-core 1.1.0a7
..
  Successfully uninstalled poetry-core-1.1.0a7
root@b447a66339c6:/foobar# pip install -q git+https://github.com/abn/poetry-core.git@groups-merge-dev
root@b447a66339c6:/foobar# poetry show --tree
pycowsay 0.0.0.1
pytest 7.0.1 pytest: simple powerful testing with Python
├── atomicwrites >=1.0
├── attrs >=19.2.0
├── colorama *
├── iniconfig *
├── packaging *
│   └── pyparsing >=2.0.2,<3.0.5 || >3.0.5 
├── pluggy >=0.12,<2.0
├── py >=1.8.2
└── tomli >=1.0.0

@abn abn requested a review from a team March 10, 2022 13:22
src/poetry/core/factory.py Outdated Show resolved Hide resolved
tests/test_factory.py Show resolved Hide resolved
src/poetry/core/factory.py Show resolved Hide resolved
abn added 2 commits March 17, 2022 02:57
This change ensures that existence of legacy "dev-dependency" section
does not break when "dev" group is added for new dependencies.
@sonarcloud
Copy link

sonarcloud bot commented Mar 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
1.6% 1.6% Duplication

@radoering radoering merged commit e8031bb into python-poetry:master Mar 18, 2022
@abn abn deleted the groups-merge-dev branch March 19, 2022 23:49
DavidVujic pushed a commit to DavidVujic/poetry-core that referenced this pull request Mar 26, 2022
* factory: merge legacy dependencies and groups

This change ensures that existence of legacy "dev-dependency" section
does not break when "dev" group is added for new dependencies.

* package: remove redundant filtering

* fix typing for `ProjectPackage.python_versions`
@andersk
Copy link

andersk commented May 21, 2022

This commit seems to have broken Poetry’s test_solver_duplicate_dependencies_different_sources_types_are_preserved. If you update poetry-core from Git, that test now raises poetry.puzzle.exceptions.SolverProblemError: Because root depends on both demo (0.1.2 git) and demo (>=0.1.0), version solving failed.

@abn
Copy link
Member Author

abn commented May 21, 2022

This does not relate to the breakage. The tests downstream introduced a case we were previously not testing for. The breakage is related to package similarity check changes.

There already is a fix for this on Poetry.

@andersk
Copy link

andersk commented May 21, 2022

I believe you that the real bug is elsewhere. But it isn’t fixed as of now. What I’m saying is that

  • current poetry master fails with current poetry-core master;
  • current poetry master fails with poetry-core where this commit was merged;
  • current poetry master succeeds with poetry-core before this commit was merged.

@radoering
Copy link
Member

It's kind of related. This PR influences the order of dependencies that are returned by all_requires and thereby reveals a downstream bug in a test added recently in python-poetry/poetry#5640. A fix is on the way: python-poetry/poetry#5654

bostonrwalker pushed a commit to bostonrwalker/poetry-core that referenced this pull request Aug 29, 2022
* factory: merge legacy dependencies and groups

This change ensures that existence of legacy "dev-dependency" section
does not break when "dev" group is added for new dependencies.

* package: remove redundant filtering

* fix typing for `ProjectPackage.python_versions`
bostonrwalker pushed a commit to bostonrwalker/poetry-core that referenced this pull request Aug 29, 2022
* factory: merge legacy dependencies and groups

This change ensures that existence of legacy "dev-dependency" section
does not break when "dev" group is added for new dependencies.

* package: remove redundant filtering

* fix typing for `ProjectPackage.python_versions`
bostonrwalker pushed a commit to bostonrwalker/poetry-core that referenced this pull request Aug 29, 2022
* factory: merge legacy dependencies and groups

This change ensures that existence of legacy "dev-dependency" section
does not break when "dev" group is added for new dependencies.

* package: remove redundant filtering

* fix typing for `ProjectPackage.python_versions`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants