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

multipart dependency conflicts with python-multipart, making impossible to install treq and starlette on the same system #399

Closed
mgorny opened this issue Sep 19, 2024 · 20 comments · Fixed by #400

Comments

@mgorny
Copy link

mgorny commented Sep 19, 2024

There are currently two multipart implementations on PyPI: multipart and python-multipart. Both install the same multipart package, and therefore cannot be installed simultaneously.

Both starlette and synapse use (apparently more popular) python-multipart. This is probably understandable, given that prior to June, the last release of multipart was in 2021. However, treq now started using multipart, which means it is no longer possible to install treq and starlette on the same system.

Please consider using python-multipart for the sake of compatibility instead.

@glyph
Copy link
Member

glyph commented Sep 19, 2024

Perhaps python-multipart should change its import name, since it postdates multipart? The apache license isn't a dealbreaker, but it's nice that multipart is MIT, has more recent commits, has a push parser in progress, and has currently-passing CI.

Neither appears to have any documentation, which isn't great, but multipart's README is more comprehensive and doesn't contain broken links

@mgorny
Copy link
Author

mgorny commented Sep 19, 2024

python-multipart changing at this point would break all its reverse dependencies.

Honestly, this isn't my fight. All I'm pointing out that this change is making synapse not correctly installable, since it simultaneously depends on python-multipart and treq (which now depends on multipart).

Furthermore, since multipart is just a .py file, while python-multipart is a full package, if both are installed (pip doesn't do anything to prevent that) the latter will take precedence.

Yes, Python ecosystem is completely broken by design.

@glyph
Copy link
Member

glyph commented Sep 19, 2024

python-multipart changing at this point would break all its reverse dependencies.

Honestly, this isn't my fight. All I'm pointing out that this change is making synapse not correctly installable, since it simultaneously depends on python-multipart and treq (which now depends on multipart).

Yeah, thanks for letting us know.

Furthermore, since multipart is just a .py file, while python-multipart is a full package, if both are installed (pip doesn't do anything to prevent that) the latter will take precedence.

OK, good to know.

Yes, Python ecosystem is completely broken by design.

"completely" is a strong word there, but this is certainly a flaw.

@glyph
Copy link
Member

glyph commented Sep 19, 2024

Given that things are in a broken state currently, I am going to do the following:

@glyph
Copy link
Member

glyph commented Sep 19, 2024

@mgorny OK, vendoring branch done. Perhaps if @twm has some time we can get a review & release soon.

@defnull
Copy link

defnull commented Sep 19, 2024

I answered the request for renaming multipart.py in the issue linked above. tl;dr: no

Vendoring seems to be a sensible workaround for now, given how easy it is with a single-file no-dependency module. I'd suggest watching the github project for new releases though, as we have a pretty nice new sansio (non-blockling) parser almost ready that is significantly faster than the current implementation (or werkzeug) and also more predictable in memory and CPU consumption. Documentation will also improve.

@glyph
Copy link
Member

glyph commented Sep 19, 2024

@defnull For what it's worth, I'm inclined to agree with your reasoning, but we are in this situation due to decisions of others that I have relatively limited influence over :). We'll see what the python-multipart maintainers have to say. Best-case scenario here would be to merge the projects, since it's not clear to me that there's a great advantage of one over the other?

@twm twm closed this as completed in #400 Sep 19, 2024
@defnull
Copy link

defnull commented Sep 20, 2024

Merging both projects is also not an option, as the APIs are completely different. The only solution I can think of is for python-multipart to give up their conflicting package name and release a new version (with a major version bump, because it's a breaking change), then deprecate the old release and package. But no one can force anyone to do anything in this ecosystem.

Anyway, multipart is in a way better shape now. I'm actually happy enough with the current state that I'm planning a 1.0 release. That might change the 'popularity' aspect in the future a bit. Maybe. I'm really bad at promoting my own projects, so we'll see.

@Kludex
Copy link

Kludex commented Sep 21, 2024

This issue looks like a competition to see which project is doing better... "Which one has better documentation?" "Which one has a better parse?"

If it's possible to do what python-multipart did, then the problem is with the ecosystem itself. 🤷‍♂️

Anyway, it looks like this was solved. @mgorny we've interacted on many issues already, and I think I've always tried to be helpful. If you need something else here, let me know. 👍

@twm
Copy link
Contributor

twm commented Sep 21, 2024

Hi @Kludex, treq maintainer here.

Anyway, it looks like this was solved. @mgorny we've interacted on many issues already, and I think I've always tried to be helpful. If you need something else here, let me know. 👍

To be clear, this has only been solved temporarily, for treq in the PyPI ecosystem. Vendoring multipart is not a long-term solution (there has already been a release — our vendored copy is out of date!). Nor is it a complete solution: when Debian goes to package treq I expect that they will un-vendor multipart and we'll be right back to the same conflict.

Can you offer some timeline for when you plan to merge python-multipart into Starlette? I don't see an open issue for doing so but I'd like to know when I can un-vendor multipart.

@Kludex
Copy link

Kludex commented Sep 21, 2024

I can't offer timeline. Sorry.

@glyph
Copy link
Member

glyph commented Sep 22, 2024

@Kludex I understand that you can't give a timeline, but if we did the work to vendor python-multipart into startlette, or port starlette to multipart instead, would you accept that PR?

@Kludex
Copy link

Kludex commented Sep 22, 2024

We need to clear the list of PRs in python-multipart, and make it easy to just copy paste onto starlette i.e. increase test coverage to 100%, fully type check, and match the lint rules.

Moving to multipart doesn't really makes any sense... I don't understand that suggestion.

@eli-schwartz
Copy link

If it's possible to do what python-multipart did, then the problem is with the ecosystem itself. 🤷‍♂️

Not really sure how you concluded this. Do you think it is a problematic aspect of the python ecosystem, that:

  • a single wheel can install multiple modules?
  • packages are permitted to provide import names that differ from the PyPI name?
    • packages can be available as sdist, and install modules based on running python code, which PyPI cannot check regardless and can therefore be used to circumvent any technical measure designed to enforce the previous point?

Why would it be a good outcome for PyPI to desire to enforce any such thing? It's a community cooperation issue. There are plenty of good reasons to allow it as a possibility for people who believe they have good reasons to reuse the same import name. Common reasons why a project might choose to do so include:

  • renaming a project when you own both the old and new PyPI names
  • forking a project whose author died to provide continuity
  • forking a project whose author dropped the project entirely, and invited the community to fork under a new name due to concerns about dependency trust which preclude giving someone they do not know, control over a trusted name
  • offering an API-compatible drop-in replacement for a package with enhancements that haven't been accepted by the original project
  • providing an alternative policy implementation for a policy-focused package such as certifi

As for python-multipart:

I don't think anyone here doubts that python-multipart knew, back when it was first published, that it was conflicting with the import name of an existing project. This acknowledgment is implicit in the fact that python-multipart was unable to name itself "multipart" and chose to upload with the prefix "python-" indicating "python-multipart is written in the python programming language" to distinguish itself from "multipart: a module uploaded to the index of python software".

Of course, python-multipart could have chosen to name its module as "python_multipart" (or perhaps pymultipart). But it did not do so, for whatever reason. I guess you're saying it was your right to do so in a manner that doesn't really seem to fit into any of the options I mentioned above, because PyPI didn't have a technological restriction preventing you from doing so? That is definitely one way of looking at it.

Anyway, it looks like this was solved. @mgorny we've interacted on many issues already, and I think I've always tried to be helpful. If you need something else here, let me know. 👍

I don't know the context here but from a naive perspective this looks oddly patronizing, especially given that as far as I can tell your attempt to be helpful consists of confirming that you do not intend to do anything to solve the problem up to and including providing a timeline for decommissioning the package and merging it into your other project -- which I'd typically understand to mean that you do not want other people to use python-multipart at all and it exists as an internal implementation detail of starlette.

So I really wonder why it isn't a sufficient solution, to rename the module to import _starlette_multipart and solve this headache for everyone, including the people for whom bundling an old and outdated version of multipart isn't a solution, but rather an additional (vetting and compliance) problem, such as the aforementioned Debian.

...

To be clear: I'm not saying you owe me something as an open source project and that I demand an action of you. I just find it super weird that you popped up on this issue purely to state for the record that you "always try to be helpful". It feels like some kind of PR stunt but I am having a hard time unwrapping the meaning contained within it.

@defnull
Copy link

defnull commented Sep 22, 2024

We need to clear the list of PRs in python-multipart, and make it easy to just copy paste onto starlette i.e. increase test coverage to 100%, fully type check, and match the lint rules.

If python-multipart is really just for starlette and not intended to be used as a dependency by others, then please rename the package and add a clear warning to its README. You control both projects, changing the import name to starlette_multipart should be possible within a single patch release of starlette. There is no reason to wait another 6 years.

If python-multipart is intended to be used by not just starlette, then also, please rename it. Breaking changes are fine for a 0.0.x library, most dependant packages should have an exact version pinned and not break without warning. The fix for dependent packages would be trivial. Fixing the current name conflict situation, however, is almost impossible without vendoring one of the two.

sandhose added a commit to element-hq/synapse that referenced this issue Sep 25, 2024
Bumps [treq](https:/twisted/treq) from 23.11.0 to 24.9.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https:/twisted/treq/releases">treq's
releases</a>.</em></p>
<blockquote>
<h2>Treq 24.9.0</h2>
<h2>Features</h2>
<ul>
<li>treq now ships type annotations. (<a
href="https://redirect.github.com/twisted/treq/issues/366">#366</a>)</li>
<li>The new <code>treq.cookies</code> module provides helper functions
for working with <code>http.cookiejar.Cookie</code> and
<code>CookieJar</code> objects. (<a
href="https://redirect.github.com/twisted/treq/issues/384">#384</a>)</li>
<li>Python 3.13 is now supported. (<a
href="https://redirect.github.com/twisted/treq/issues/391">#391</a>)</li>
</ul>
<h2>Bugfixes</h2>
<ul>
<li><code>treq.content.text_content()</code> no longer generates
deprecation warnings due to use of the <code>cgi</code> module. (<a
href="https://redirect.github.com/twisted/treq/issues/355">#355</a>)</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Mixing the <em>json</em> argument with <em>files</em> or
<em>data</em> now raises <code>TypeError</code>. (<a
href="https://redirect.github.com/twisted/treq/issues/297">#297</a>)</li>
<li>Passing non-string (<code>str</code> or <code>bytes</code>) values
as part of a dict to the <em>headers</em> argument now results in a
<code>TypeError</code>, as does passing any collection other than a
<code>dict</code> or <code>Headers</code> instance. (<a
href="https://redirect.github.com/twisted/treq/issues/302">#302</a>)</li>
<li>Support for Python 3.7 and PyPy 3.8, which have reached end of
support, has been dropped. (<a
href="https://redirect.github.com/twisted/treq/issues/378">#378</a>)</li>
</ul>
<h2>Misc</h2>
<ul>
<li><a
href="https://redirect.github.com/twisted/treq/issues/336">#336</a>, <a
href="https://redirect.github.com/twisted/treq/issues/382">#382</a>, <a
href="https://redirect.github.com/twisted/treq/issues/395">#395</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https:/twisted/treq/blob/trunk/CHANGELOG.rst">treq's
changelog</a>.</em></p>
<blockquote>
<h1>24.9.1 (2024-09-19)</h1>
<h2>Bugfixes</h2>
<ul>
<li>treq has vendored its dependency on the <code>multipart</code>
library to avoid import
conflicts with <code>python-multipart</code>; it should now be
installable alongside
that library. (<code>[#399](twisted/treq#399)
&lt;https:/twisted/treq/issues/399&gt;</code>__)</li>
</ul>
<h1>24.9.0 (2024-09-17)</h1>
<h2>Features</h2>
<ul>
<li>treq now ships type annotations.
(<code>[#366](twisted/treq#366)
&lt;https:/twisted/treq/issues/366&gt;</code>__)</li>
<li>The new :mod:<code>treq.cookies</code> module provides helper
functions for working with <code>http.cookiejar.Cookie</code> and
<code>CookieJar</code> objects.
(<code>[#384](twisted/treq#384)
&lt;https:/twisted/treq/issues/384&gt;</code>__)</li>
<li>Python 3.13 is now supported.
(<code>[#391](twisted/treq#391)
&lt;https:/twisted/treq/issues/391&gt;</code>__)</li>
</ul>
<h2>Bugfixes</h2>
<ul>
<li>:mod:<code>treq.content.text_content()</code> no longer generates
deprecation warnings due to use of the <code>cgi</code> module.
(<code>[#355](twisted/treq#355)
&lt;https:/twisted/treq/issues/355&gt;</code>__)</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Mixing the <em>json</em> argument with <em>files</em> or
<em>data</em> now raises <code>TypeError</code>.
(<code>[#297](twisted/treq#297)
&lt;https:/twisted/treq/issues/297&gt;</code>__)</li>
<li>Passing non-string (<code>str</code> or <code>bytes</code>) values
as part of a dict to the <em>headers</em> argument now results in a
<code>TypeError</code>, as does passing any collection other than a
<code>dict</code> or <code>Headers</code> instance.
(<code>[#302](twisted/treq#302)
&lt;https:/twisted/treq/issues/302&gt;</code>__)</li>
<li>Support for Python 3.7 and PyPy 3.8, which have reached end of
support, has been dropped.
(<code>[#378](twisted/treq#378)
&lt;https:/twisted/treq/issues/378&gt;</code>__)</li>
</ul>
<h2>Misc</h2>
<ul>
<li><code>[#336](twisted/treq#336)
&lt;https:/twisted/treq/issues/336&gt;</code><strong>,
<code>[#382](twisted/treq#382)
&lt;https:/twisted/treq/issues/382&gt;</code></strong>,
<code>[#395](twisted/treq#395)
&lt;https:/twisted/treq/issues/395&gt;</code>__</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https:/twisted/treq/commit/caaf9fcb62992de47ad5ebcb628cce5106b8d1b1"><code>caaf9fc</code></a>
Release 24.9.1</li>
<li><a
href="https:/twisted/treq/commit/9cedb088b40e5d756f1196defb46b5a7e41bf1c8"><code>9cedb08</code></a>
Merge pull request <a
href="https://redirect.github.com/twisted/treq/issues/400">#400</a> from
twisted/vendor-multipart-for-now</li>
<li><a
href="https:/twisted/treq/commit/4aa1ee8a3ca5461c165ea380b1cbd0ea5b41cce4"><code>4aa1ee8</code></a>
news fragment</li>
<li><a
href="https:/twisted/treq/commit/d7c16de8f522c5fc10cf2108371afce635d39e4e"><code>d7c16de</code></a>
octothorpes rise up</li>
<li><a
href="https:/twisted/treq/commit/4fd3c842c21a3fa45560dc7eb41767fcbb4e653a"><code>4fd3c84</code></a>
try to make the linter happy</li>
<li><a
href="https:/twisted/treq/commit/f0a5148cba2c983335758dd34ab78bff46f2dc6b"><code>f0a5148</code></a>
fix import, switch to <code>from</code></li>
<li><a
href="https:/twisted/treq/commit/7f16b87f0a2574a2ef67a50e6bf89ad9941fcf4c"><code>7f16b87</code></a>
correct import</li>
<li><a
href="https:/twisted/treq/commit/1526431a37745bb33982f79bb38d1d4e4554907d"><code>1526431</code></a>
add a lightly-modified vendored version of <a
href="https:/defnull/multipa">https:/defnull/multipa</a>...</li>
<li><a
href="https:/twisted/treq/commit/7c52d4917f41291da271fd5cebf2e69e73dcee32"><code>7c52d49</code></a>
remove dependency on <code>multipart</code> package</li>
<li><a
href="https:/twisted/treq/commit/ca3966f57a34fa4a3c0b3eb1a90e3f1cc1951bf3"><code>ca3966f</code></a>
Merge pull request <a
href="https://redirect.github.com/twisted/treq/issues/398">#398</a> from
twisted/397-release-24.9.0</li>
<li>Additional commits viewable in <a
href="https:/twisted/treq/compare/release-23.11.0...treq-24.9.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=treq&package-manager=pip&previous-version=23.11.0&new-version=24.9.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Quentin Gliech <[email protected]>
@mgorny
Copy link
Author

mgorny commented Oct 21, 2024

Just I wanted to say once again, a big "thank you" to you for pushing towards resolving the bigger issue. With python-multipart finally renaming itself, things are hopefully going to settle sometime soon.

@glyph
Copy link
Member

glyph commented Oct 21, 2024

With python-multipart finally renaming itself

Is this officially decided? Kludex/python-multipart#16 looks like it's still up in the air

@merwok
Copy link

merwok commented Oct 21, 2024

Yes, a new module name was added: Kludex/python-multipart#166

(The backward compat causes issues with installing and reloading in same process without invalidating importlib caches, so there’s a second PR: Kludex/python-multipart#168)

@glyph
Copy link
Member

glyph commented Oct 21, 2024

I saw the second PR but didn't realize the context. Thank you, this is great!

@Kludex
Copy link

Kludex commented Oct 22, 2024

I had to yank the release that contained the first approach.

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 a pull request may close this issue.

7 participants