poetry icon indicating copy to clipboard operation
poetry copied to clipboard

remove circular dependency on export plugin

Open dimbleby opened this issue 2 years ago • 15 comments

The circular dependency has been bothering me for a while, but https://github.com/python-poetry/poetry/issues/6441 was the nudge to do something about it.

poetry should not depend on the plugin: the plugin should depend on poetry.

This means that docs that talk about poetry export arguably don't belong here at all: but mostly they already say "this is actually provided by poetry-plugin-export" so I've just beefed that up slightly.

I'm about to find out what I've broken in the github workflows by removing this dependency...

dimbleby avatar Jul 09 '22 12:07 dimbleby

I'm courious how this should work. IIRC removing the plugin essentially means removing the export command from the default installation. This is something we really like to do in the future, but for now this would be a breaking change.

finswimmer avatar Jul 09 '22 14:07 finswimmer

Sure, people who want poetry export will need to install the plugin themselves.

If we want poetry export to be a default command: then move it back in-tree in this repository. But if we want it to be a plugin, then let's treat it as a plugin.

dimbleby avatar Jul 09 '22 14:07 dimbleby

I can see we're going to need consensus here... somehow.

While we're thinking about disentangling these two projects, here are some more thoughts.

I'd be inclined to transfer the bits of export-specific code that linger in this repository over to the plugin entirely. That is: get_project_dependency_packages(), and its children in the call graph.

That would allow deleting the horrid cross-repository github workflow, making it much easier to make fixes to the export function.

It would also make it easier to add enhancements if the export plugin was more involved in walking the dependency tree eg https://github.com/python-poetry/poetry-plugin-export/issues/91 would like access to information that is known inside that code.

To be clear, I don't intend to do any of that any time soon. But this seems as good a place as any to dump some notes.

dimbleby avatar Jul 14 '22 20:07 dimbleby

It's probably desirable to take a view on what the Right Thing To Do is before 1.2.0.

That doesn't necessarily require that anything change - I'd hate to block 1.2.0! - but if we're backing ourselves into a corner where we're stuck with an unwanted circular dependency that's hard to get rid of: it would be as well to do that with eyes wide open.

dimbleby avatar Jul 24 '22 19:07 dimbleby

I see three solutions to resolving the circular dependency:

  1. Remove the strict dependence on poetry-plugin-export. People who need it can install it.
  2. Vendorize poetry-plugin-export into Poetry since it is an mandatory feature anyway. I recognize that this undoes the original PR to move this to its own project.
  3. Remove the dependence of poetry-plugin-export on poetry. This is basically lying, but it would work. If you installed poetry-plugin-export all by itself, it would be broken obviously because it did not pull in Poetry.

I don't have a preference for any individual solution. I only want Poetry 1.2 to be able to be installable with conda.

drhagen avatar Aug 30 '22 17:08 drhagen

@drhagen above:

I only want Poetry 1.2 to be able to be installable with conda.

@neersighted in https://github.com/python-poetry/poetry/issues/5586#issuecomment-1231945773:

[The circular dependency issue] is known and we do not plan to address it as part of 1.2.

These two stances seem incompatible.

maresb avatar Aug 30 '22 17:08 maresb

Yes, this PR is not scoped for 1.2. @abn has a plan to migrate away from this circular dependency, re-using much of @dimbleby's work. However, it's simply too big a change for 1.2, both in terms of adding additional risk, and in terms of @abn's present bandwidth.

neersighted avatar Aug 30 '22 17:08 neersighted

@neersighted, hmm. That will potentially leave a lot of packages uninstallable on conda-forge. One option is that we can patch on conda-forge to remove poetry-plugin-export from poetry to avoid this issue until it gets resolved in a longer-term way. This is what I've done in https://github.com/conda-forge/poetry-feedstock/pull/71

xylar avatar Aug 30 '22 18:08 xylar

Is this PR still a possibility? I'm also having issues building poetry now, but with bazel instead of conda.

joshgc avatar Oct 16 '23 20:10 joshgc

Is this PR still a possibility?

Short-term: no. Mid-term: maybe. See #8562

radoering avatar Oct 20 '23 16:10 radoering

Would it be possible to remove the circular dependency but still install the plugin by default? Being able to have Poetry interact with pip through requirements.txt files is still a really nice default feature to have. For example, tools like nox-poetry depend on this:

  • https://github.com/cjolowicz/nox-poetry/blob/0612f2fabad3f3a3e661db5e0f254000341e92f1/src/nox_poetry/poetry.py#L135

Not having to install a plugin for this kind of functionality (especially since it's a different workflow based on if you are using pipx or not, warnings about Windows, etc) would be convenient if at all possible.

johnthagen avatar Nov 07 '23 17:11 johnthagen

Idea: Could the export plugin be an extras? So that a user could install it with pipx in a single convenient command with:

pipx install poetry[export-plugin]

johnthagen avatar Jan 24 '24 21:01 johnthagen

I don't think an extra changes much. We still have a circular dependency (only if the extra is used but nevertheless...). I think a general solution for plugins that are required for a specific project as demanded in #5729 might make more sense. It looks like there is already a POC for project plugins: #5740. Maybe, someone picks this up.

radoering avatar Jan 27 '24 05:01 radoering

Okay, wow yes #5729 / #5740 would be a really great way to solve this issue! The specific challenge that this PR will introduce for us is a bootstrapping problem. Whenever we share a Poetry project with another developer, they already need to:

  1. Install pipx
  2. Install Poetry
  3. Use the project (Poetry/nox etc.)

Now we'd need to inform them that they need to:

  1. Install pipx
  2. Install Poetry
  3. Install the export plugin
  4. Use the project (Poetry/nox etc)

We've found that these bootstrapping type steps are fraught with developers making small mistakes and thus, we actually avoid Poetry plugins more than we otherwise would because getting them installed everywhere correctly is tricky to document and get everyone to manually install.

Hope this feedback is useful!

johnthagen avatar Jan 27 '24 12:01 johnthagen

I'm still in favor of eliminating this circular dependency but I just wanted to point to my comment here: https://github.com/python-poetry/poetry/issues/6441#issuecomment-2058832773 It seems we can handle circular dependencies in conda packages, it's just a little awkward.

xylar avatar Apr 16 '24 11:04 xylar