Skip to content

feat(deps): Update and relax tomlkit version requirement #321

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

Merged
merged 1 commit into from
Dec 30, 2020
Merged

feat(deps): Update and relax tomlkit version requirement #321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ decli = "^0.5.2"
colorama = "^0.4.1"
termcolor = "^1.1"
packaging = ">=19,<21"
tomlkit = "^0.5.3"
tomlkit = ">=0.5.3,<1.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

If my understanding is not wrong, this is basically the same as what is was
https://python-poetry.org/docs/dependency-specification/#version-constraints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example from the documentation:
^0.2.3 | >=0.2.3 <0.3.0

Installation example:

root@38df9bc1e4e0:/# pip install poetry==1.1.4 commitizen==2.11.1
Collecting commitizen==2.11.1
  Downloading commitizen-2.11.1-py3-none-any.whl (42 kB)
     |████████████████████████████████| 42 kB 228 kB/s
Collecting poetry==1.1.4
  Downloading poetry-1.1.4-py2.py3-none-any.whl (171 kB)
     |████████████████████████████████| 171 kB 2.6 MB/s
INFO: pip is looking at multiple versions of <Python from Requires-Python> to determine which version is compatible with other requirements. This could take a while.
INFO: pip is looking at multiple versions of commitizen to determine which version is compatible with other requirements. This could take a while.
ERROR: Cannot install commitizen==2.11.1 and poetry==1.1.4 because these package versions have conflicting dependencies.

The conflict is caused by:
    commitizen 2.11.1 depends on tomlkit<0.6.0 and >=0.5.3
    poetry 1.1.4 depends on tomlkit<1.0.0 and >=0.7.0

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/user_guide/#fixing-conflicting-dependencies```

Copy link
Member

Choose a reason for hiding this comment

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

Then, maybe we should change it to tomlkit = ">=0.7.0,<1.0.0" instead?

Copy link
Contributor Author

@Woklex Woklex Dec 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Then someone might have this situation. What if someone uses commitizen and an x package that has tomlkit < 0.6.0 and >= 0.5.3 in its dependencies? If someone has toolkit < 0.6.0 and >= 0.5.3 then at least we won't have a conflict. And when installing commitzen and poetry, we will install the last package, because poetry tomlkit = >= 0.7.0, < 1.0.0 and commitzen tomlkit = >= 0.5.3, < 1.0.0

Copy link
Member

Choose a reason for hiding this comment

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

If someone has tomlkit < 0.6.0 and >= 0.5.3 then updating to a newer version is cheap, as there are no crazy breaking changes in the changelog and it's recommended to update.

But I'm okay with the proposed change or with the one from Lee tomlkit = ">=0.7.0,<1.0.0", using 0.7.0 would make it more in compliance with toml format. Which one do you prefer?

Regarding the conflicts, poetry is usually not recommended to install it on project level.

And for commitizen, if there were more reports of conflicts, we should consider using a _vendor/ folder, using something like vendy. But so far we haven't had any issues, and we try to keep deps small.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will be satisfied with both options :) I originally wanted to make a toolkit = ">=0.7.0,<1.0.0, but I thought that such changes could break someone's build. So I leave the choice to you. If you prefer >=0.7.0,<1.0.0 then write to me and I will correct the commit.

Copy link
Member

@Lee-W Lee-W Dec 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I got your point. I misunderstood the meaning of "^0.5.3". I think your solution is a better one. I'll merge it. Thanks for your contribution!

As for dependency conflict, I'd suggest using tools like pipx to install python cli-tools like poetry and commitizen. Maybe we could add it to the doc. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! It is obvious to me that pipx is needed, but I just ran into this problem because another developer did not know it. So it would be ideal to update the documentation and recommend pipx (do as in the poetry documentation). It should be easy to update the documentation and I would even do a PR, but my English is not so good :(

jinja2 = "^2.10.3"
pyyaml = ">=3.08"
argcomplete = "^1.12.1"
Expand Down