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

Automated migration to groups during poetry add --group dev #6638

Open
tomwatson1024 opened this issue Sep 27, 2022 · 15 comments
Open

Automated migration to groups during poetry add --group dev #6638

tomwatson1024 opened this issue Sep 27, 2022 · 15 comments
Labels
area/pyproject Metadata/pyproject.toml-related kind/feature Feature requests/implementations

Comments

@tomwatson1024
Copy link

tomwatson1024 commented Sep 27, 2022

Running poetry add --group dev (or poetry add --dev) with a dependency that already exists in the [tool.poetry.dev-dependencies] section adds a new entry in [tool.poetry.group.dev.dependencies that may conflict with the existing entry.

It's easily fixed (updating [tool.poetry.dev-dependencies] to [tool.poetry.group.dev.dependencies], and merging the sections if the extra entry has already been added) but it makes the Poetry 1.1 -> 1.2 upgrade painful if you've got lots of repos.

# syntax=docker/dockerfile:1.3-labs
FROM python:3.8
RUN pip install poetry==1.2.1
WORKDIR /src
COPY <<"EOF" pyproject.toml
[tool.poetry]
name = ""
version = "0.1.0"
description = ""
authors = [""]

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

[tool.poetry.dev-dependencies]
tomli = "1.0.0"

EOF

# Using `poetry add --group dev` and `poetry add --dev` produce the same result.
RUN poetry add --group dev tomli@2.0.0
RUN cat pyproject.toml && poetry lock

produces

 > [6/6] RUN cat pyproject.toml && poetry lock:
#14 0.534 [tool.poetry]
#14 0.534 name = ""
#14 0.534 version = "0.1.0"
#14 0.534 description = ""
#14 0.534 authors = [""]
#14 0.534
#14 0.534 [tool.poetry.dependencies]
#14 0.534 python = "^3.8"
#14 0.534
#14 0.534 [tool.poetry.dev-dependencies]
#14 0.534 tomli = "1.0.0"
#14 0.534
#14 0.534 [[tool.poetry.source]]
#14 0.534 name = 'simple'
#14 0.534 url = 'https://pypi.org/simple'
#14 0.534 default = true
#14 0.534
#14 0.534
#14 0.534 [tool.poetry.group.dev.dependencies]
#14 0.534 tomli = "2.0.0"
#14 0.534
#14 1.122 Updating dependencies
#14 1.123 Resolving dependencies...
#14 1.133
#14 1.133 Because  depends on both tomli (2.0.0) and tomli (1.0.0), version solving failed.

edit: Removed unnecessary [tool.poetry.source] section in Dockerfile.

@tomwatson1024 tomwatson1024 added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 27, 2022
@neersighted neersighted added kind/feature Feature requests/implementations area/pyproject Metadata/pyproject.toml-related and removed kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Sep 27, 2022
@neersighted neersighted changed the title poetry add --group dev adds duplicate dependencies to as pyproject.toml with [tool.poetry.dev-dependencies] Automated migration to groups during poetry add --group dev Sep 27, 2022
@neersighted
Copy link
Member

This has nothing to do with lock files, but with project files. If you make use of groups in your project, naturally Poetry 1.1 will no longer be able to understand it, as it is a new feature. I have edited your issue to remove the link to the other issue as it is misleading.

This is currently a known drawback of the --dev switch -- one-time migration code that detects the old dev-depedencies and migrates them by rewriting the pyproject.toml to use groups before doing the 'add' would be sufficient.

However, such a feature was considered more trouble than it was worth as most users who migrate over will do so by hand-editing pyproject.toml, and what is happening is very obvious when you open the file.

I'm reclassifying this as a feature request -- it's welcome if anyone wants to write it, but was considered optional and not written by any maintainer as no one had much interest in a mild edge case like this.

@rzuckerm
Copy link

rzuckerm commented Oct 1, 2022

I noticed that the remove command seems to handle removing dependencies from dev-dependencies.

elif group == "dev" and "dev-dependencies" in poetry_content:

I'm curious as to why the add command doesn't look in dev-dependencies when checking if a dependency is already present in pyproject.toml.

@neersighted
Copy link
Member

However, such a feature was considered more trouble than it was worth as most users who migrate over will do so by hand-editing pyproject.toml, and what is happening is very obvious when you open the file.

I think I covered that -- both commands are ignorant of dev-dependencies and the only migration currently in the code is implicit conversion to a group.

Contributions are welcome, including if you think this deserves some mention in the docs or you want to introduce a migration in code. No one involved at the time thought it was worth the effort to implement and test, is all.

@rzuckerm
Copy link

rzuckerm commented Oct 1, 2022

I'm wondering if dev-dependencies should continue to be used if it exists. Maybe there could be a separate command to migrate from dev-dependencies to group.dev.dependencies?

@neersighted
Copy link
Member

dev-dependencies is implicitly used/combined right now -- I don't think we can walk back from that behavior immediately -- warning and removing it in a future version is better, I think.

@rzuckerm
Copy link

rzuckerm commented Oct 1, 2022

So, how should add work when there are dev-dependencies under the following conditions given that foo = "^1.0" is in dev-dependencies?

  1. poetry add foo
  2. poetry add foo^1.1
  3. poetry add bar

@neersighted
Copy link
Member

neersighted commented Oct 1, 2022

I assume that's add -G dev? I think that's up to us to decide -- my immediate thoughts are we could do a wholesale migration the first time -G dev is used. That is to say, migrate everything from dev-dependencies to groups.dev, write pyproject.toml, and then do the add as normal.

Yes, this forces migration for users who are using -G dev on the command line, but we already effectively do that -- this just removes the split brain situation.

@rzuckerm
Copy link

rzuckerm commented Oct 2, 2022

I've taken a swing a fixing this. I currently have a draft PR (#6677) for this. I'll add some tests and docs later. I just want to make sure that I'm at least on the right track. I've tested this locally, and it seems to work, but I'm not still a newbie on this package and am not familiar with all the different use-cases I'd need to test to make sure that this doesn't have a nasty side-effects.

@rzuckerm
Copy link

rzuckerm commented Oct 4, 2022

After discussing this @dimbleby , I'm wondering if a better solution would be to make a change to poetry-core here that would do the migration on read of the pyproject.toml. That way, any operation that writes the pyproject.toml would complete the migration.

@lanzz
Copy link

lanzz commented Dec 21, 2022

However, such a feature was considered more trouble than it was worth as most users who migrate over will do so by hand-editing pyproject.toml, and what is happening is very obvious when you open the file.

Those bits about "hand-editing pyproject.toml" and "very obvious" might be bit biased. I'm not familiar enough with the internals of Poetry's dependency management, and wouldn't directly assume that I can just hand-edit my pyproject.toml and move stuff around between sections, especially considering this is the first time the very concept of groups has popped up and Poetry itself isn't presenting any obvious upgrade path. I would hope the average developer wouldn't just blindly start moving stuff around in "obvious" ways hoping they will just luck out in figuring out how exactly this new concept works — to me at least this represents a hard interrupt of my workflow forcing me into researching how exactly this thing works before fiddling with it.

How is this not a major-version breaking change, if "naturally Poetry 1.1 will no longer be able to understand it, as it is a new feature"?

@neersighted
Copy link
Member

neersighted commented Dec 21, 2022

The pyproject.toml is front-and-center in Poetry's design and documentation. I'm sorry that you were under the impression it was opaque/only to be managed using the CLI. Please point out deficiencies in the README/docs if you find them/send patches improving them.

Regarding the addition of new features, there is a detailed introduction in the release announcement, good documentation for this new feature and its semantics, and backwards compatibility as dev-dependencies is supported for those who don't want to move to the syntax exclusive to Poetry 1.2+. That being said, there is no way to implement new features in a way that previous versions will understand. This is equivalent to objecting to adding a new module to the Python standard library as you can't use it on previous versions.

This issue is for an automated migration to smooth over the transition for CLI-only users, such as yourself. Please keep discussion on-topic in this issue; if you wish to discuss Poetry's development process and how you believe the project can be better, please do so on Discord, via Discussions, or contribute to the project and help shape these decisions with us.

@lanzz
Copy link

lanzz commented Dec 21, 2022

If Poetry itself introduces a change to pyproject.toml that isn't backwards compatible, I would expect it to bump the requires = ["poetry-core>=1.0.0"] line to something that would understand the new feature, and older versions to notice they're not satisfying the pyproject.toml requirement and to provide meaningful upgrade path directions. Otherwise people get

$ poetry install

  RuntimeError

  The Poetry configuration is invalid:
    - Additional properties are not allowed ('group' was unexpected)

which is pretty opaque.

@neersighted
Copy link
Member

You are instead looking for #3316. Please note that the poetry-core requirement has nothing to do with anything being discussed here, including #3316. That is because it is only used to inform other PEP 517 frontends (e.g. pip, build) where to find the build-system that can interpret a Poetry pyproject.toml.

@lanzz

This comment was marked as off-topic.

@neersighted

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/pyproject Metadata/pyproject.toml-related kind/feature Feature requests/implementations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants