Skip to content

Conversation

@uranusjr
Copy link

Flet uses pkg_resources in code, but does not declare the package that provides it as a dependency. While setuptools is populated by default in most virtual environment implementations now, it is continuously being disfavored and is already not present in some tools. This patch properly declares setuptools as a dependency and avoids Flet from crashing in such environments.

Since Flet's usage of pkg_resources is quite minimal, it may be worthwhile to eventually move to packaging instead (which pkg_resources vendors), or implement a more lightweight version sorting logic to replace the dependency.

The lower version bound is chosen pretty much arbitrarily. Version 56.0.0 is the default setuptools vendored by the standard library's ensurepip module.

Flet uses pkg_resources in code, but does not declare the package that
provides it as a dependency. While setuptools is populated by default
in most virtual environment implementations now, it is continuously
being disfavored and is already not present in some tools. This patch
properly declares setuptools as a dependency and avoids Flet from
crashing in such environments.

Since Flet's usage of pkg_resources is quite minimal, it may be
worthwhile to eventually move to packaging instead (which pkg_resources
vendors), or implement a more lightweight version sorting logic to
replace the dependency.

The lower version bound is chosen pretty much arbitrarily. Version
56.0.0 is the default setuptools vendored by the standard library's
ensurepip module.
@CLAassistant
Copy link

CLAassistant commented Nov 25, 2022

CLA assistant check
All committers have signed the CLA.

@FeodorFitsner
Copy link
Contributor

You are right, instead of adding pkg_resources as a dependency it's better to rewrite to use packaging.version.parse: https://stackoverflow.com/a/11887885

FeodorFitsner added a commit that referenced this pull request Nov 28, 2022
@uranusjr uranusjr deleted the declare-setuptools-dependency branch December 9, 2022 03:08
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