Require setuptools, mkdocs/commands/gh_deploy.py imports pkg_resources by hroncok · Pull Request #2347 · mkdocs/mkdocs

hroncok · 2021-03-31T09:09:44Z

No description provided.

waylan · 2021-03-31T12:47:50Z

I'm not sure what the point of this is. At the top of that very file we import setuptools. In other words, the script won't even run without setuptools already installed. Listing it as a dependency provides no benefit. And if the thinking is that its needs for wheels, again you can't install a whl without the wheel package and that package requires setuptools. There is no scenario that I am aware of where you can install mkdocs without already having setuptools installed.

hroncok · 2021-03-31T13:16:31Z

There is no scenario that I am aware of where you can install mkdocs without already having setuptools installed.

You can install mkdocs with setuptools and loose them later. Or you can use setuptools from an isolated build environment.

[tmp]$ python3.9 -m venv __venv__ 
[tmp]$ . __venv__/bin/activate

(__venv__) [tmp]$ pip list
Package    Version
---------- -------
pip        20.2.2
setuptools 49.1.3

(__venv__) [tmp]$ pip uninstall setuptools
...
  Successfully uninstalled setuptools-49.1.3
  
(__venv__) [tmp]$ pip list
Package Version
------- -------
pip     20.2.2
WARNING: You are using pip version 20.2.2; however, version 21.0.1 is available.
You should consider upgrading via the '/home/churchyard/tmp/__venv__/bin/python3.9 -m pip install --upgrade pip' command.

(__venv__) [tmp]$ pip install -U pip
Collecting pip
  Using cached pip-21.0.1-py3-none-any.whl (1.5 MB)
Installing collected packages: pip
  Attempting uninstall: pip
    Found existing installation: pip 20.2.2
    Uninstalling pip-20.2.2:
      Successfully uninstalled pip-20.2.2
Successfully installed pip-21.0.1

(__venv__) [tmp]$ pip install --use-pep517 mkdocs
Collecting mkdocs
  Using cached mkdocs-1.1.2-py3-none-any.whl (6.4 MB)
Collecting PyYAML>=3.10
  Using cached PyYAML-5.4.1-cp39-cp39-manylinux1_x86_64.whl (630 kB)
Collecting tornado>=5.0
  Using cached tornado-6.1-cp39-cp39-manylinux2010_x86_64.whl (427 kB)
Collecting click>=3.3
  Using cached click-7.1.2-py2.py3-none-any.whl (82 kB)
Collecting lunr[languages]==0.5.8
  Downloading lunr-0.5.8-py2.py3-none-any.whl (2.3 MB)
     |████████████████████████████████| 2.3 MB 1.3 MB/s 
Collecting livereload>=2.5.1
  Using cached livereload-2.6.3.tar.gz (25 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting Jinja2>=2.10.1
  Using cached Jinja2-2.11.3-py2.py3-none-any.whl (125 kB)
Collecting Markdown>=3.2.1
  Using cached Markdown-3.3.4-py3-none-any.whl (97 kB)
Collecting six>=1.11.0
  Using cached six-1.15.0-py2.py3-none-any.whl (10 kB)
Collecting future>=0.16.0
  Downloading future-0.18.2.tar.gz (829 kB)
     |████████████████████████████████| 829 kB 1.8 MB/s 
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting nltk>=3.2.5
  Downloading nltk-3.5.zip (1.4 MB)
     |████████████████████████████████| 1.4 MB 2.2 MB/s 
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting MarkupSafe>=0.23
  Using cached MarkupSafe-1.1.1-cp39-cp39-manylinux2010_x86_64.whl (32 kB)
Collecting tqdm
  Downloading tqdm-4.59.0-py2.py3-none-any.whl (74 kB)
     |████████████████████████████████| 74 kB 1.3 MB/s 
Collecting joblib
  Downloading joblib-1.0.1-py3-none-any.whl (303 kB)
     |████████████████████████████████| 303 kB 2.4 MB/s 
Collecting regex
  Downloading regex-2021.3.17-cp39-cp39-manylinux2014_x86_64.whl (731 kB)
     |████████████████████████████████| 731 kB 586 kB/s 
Building wheels for collected packages: future, livereload, nltk
  Building wheel for future (PEP 517) ... done
  Created wheel for future: filename=future-0.18.2-py3-none-any.whl size=491059 sha256=3f25227456e8f2b2dda6dde720d4e90acd1cc880c9ee5469b9c05ee43df4b090
  Stored in directory: /home/churchyard/.cache/pip/wheels/2f/a0/d3/4030d9f80e6b3be787f19fc911b8e7aa462986a40ab1e4bb94
  Building wheel for livereload (PEP 517) ... done
  Created wheel for livereload: filename=livereload-2.6.3-py2.py3-none-any.whl size=24713 sha256=56639ab0c65fd6ab6d8e6e3841bc2e718eae50b0c84e44fb34bae9b30857544e
  Stored in directory: /home/churchyard/.cache/pip/wheels/2a/a6/91/40395ea3d02fd9244dc79b039776bb45a4c86560b117a8c09c
  Building wheel for nltk (PEP 517) ... done
  Created wheel for nltk: filename=nltk-3.5-py3-none-any.whl size=1434673 sha256=a3c85aae2b9b1d7b3c1476d292c5352acbc4edc208f2ceeee67b97646e9b7030
  Stored in directory: /home/churchyard/.cache/pip/wheels/13/ae/bb/5e2a232ebaa1d2f38dd5f587e9fc4cf6ccb12758d14dac14d8
Successfully built future livereload nltk
Installing collected packages: tqdm, six, regex, joblib, future, click, tornado, nltk, MarkupSafe, lunr, PyYAML, Markdown, livereload, Jinja2, mkdocs
Successfully installed Jinja2-2.11.3 Markdown-3.3.4 MarkupSafe-1.1.1 PyYAML-5.4.1 click-7.1.2 future-0.18.2 joblib-1.0.1 livereload-2.6.3 lunr-0.5.8 mkdocs-1.1.2 nltk-3.5 regex-2021.3.17 six-1.15.0 tornado-6.1 tqdm-4.59.0

(__venv__) [tmp]$ pip list
Package    Version
---------- ---------
click      7.1.2
future     0.18.2
Jinja2     2.11.3
joblib     1.0.1
livereload 2.6.3
lunr       0.5.8
Markdown   3.3.4
MarkupSafe 1.1.1
mkdocs     1.1.2
nltk       3.5
pip        21.0.1
PyYAML     5.4.1
regex      2021.3.17
six        1.15.0
tornado    6.1
tqdm       4.59.0

hroncok · 2021-03-31T13:20:53Z

again you can't install a whl without the wheel package and that package requires setuptools

Consider we already have all the transitive runtime deps installed somehow... (I don't want to dig into that: they migh require setuptools to be installed but we don't know that for sure).

(__venv__) [tmp]$ pip list
Package    Version
---------- ---------
click      7.1.2
future     0.18.2
Jinja2     2.11.3
joblib     1.0.1
livereload 2.6.3
lunr       0.5.8
Markdown   3.3.4
MarkupSafe 1.1.1
nltk       3.5
pip        21.0.1
PyYAML     5.4.1
regex      2021.3.17
six        1.15.0
tornado    6.1
tqdm       4.59.0

Installing mkdocs "normally" works without wheel installed:

$ pip install mkdocs
Collecting mkdocs
  Using cached mkdocs-1.1.2-py3-none-any.whl (6.4 MB)
Requirement already satisfied: tornado>=5.0 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (6.1)
Requirement already satisfied: Markdown>=3.2.1 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (3.3.4)
Requirement already satisfied: livereload>=2.5.1 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (2.6.3)
Requirement already satisfied: Jinja2>=2.10.1 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (2.11.3)
Requirement already satisfied: PyYAML>=3.10 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (5.4.1)
Requirement already satisfied: lunr[languages]==0.5.8 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (0.5.8)
Requirement already satisfied: click>=3.3 in ./__venv__/lib/python3.9/site-packages (from mkdocs) (7.1.2)
Requirement already satisfied: six>=1.11.0 in ./__venv__/lib/python3.9/site-packages (from lunr[languages]==0.5.8->mkdocs) (1.15.0)
Requirement already satisfied: future>=0.16.0 in ./__venv__/lib/python3.9/site-packages (from lunr[languages]==0.5.8->mkdocs) (0.18.2)
Requirement already satisfied: nltk>=3.2.5 in ./__venv__/lib/python3.9/site-packages (from lunr[languages]==0.5.8->mkdocs) (3.5)
Requirement already satisfied: MarkupSafe>=0.23 in ./__venv__/lib/python3.9/site-packages (from Jinja2>=2.10.1->mkdocs) (1.1.1)
Requirement already satisfied: joblib in ./__venv__/lib/python3.9/site-packages (from nltk>=3.2.5->lunr[languages]==0.5.8->mkdocs) (1.0.1)
Requirement already satisfied: regex in ./__venv__/lib/python3.9/site-packages (from nltk>=3.2.5->lunr[languages]==0.5.8->mkdocs) (2021.3.17)
Requirement already satisfied: tqdm in ./__venv__/lib/python3.9/site-packages (from nltk>=3.2.5->lunr[languages]==0.5.8->mkdocs) (4.59.0)
Installing collected packages: mkdocs
Successfully installed mkdocs-1.1.2

And even if it did not, wheel does not require setuptools:

(__venv__) [tmp]$ pip install wheel
Collecting wheel
  Using cached wheel-0.36.2-py2.py3-none-any.whl (35 kB)
Installing collected packages: wheel
Successfully installed wheel-0.36.2

hroncok · 2021-03-31T13:21:42Z

tl;dr setuptools required and installed on build time != setuptools required on runtime

waylan · 2021-03-31T23:25:22Z

Installing mkdocs "normally" works without wheel installed:
$ pip install mkdocs

Perhaps I should have pointed out that pip requires setuptools. A person either needs to jump through some convoluted hoops to install the package without setuptools, or they will need to have uninstalled setuptools after installing MkDocs. Either way, an error will occur when MkDocs is run. What difference does it make that the dependency is listed or not?

hroncok · 2021-03-31T23:59:26Z

First of all, pip does not require setuptools. But that's irrelevant. The difference this makes is that in Fedora, we build the package using setuptools, but we ship it to users potentially without it. We can certainly require setuptools on runtime downstream only, but I was following the "upstream first" principle. If you don't want this, feel free to close it, but I belive setuptools is needed on runtime and saying it explicitly is the right thing to do.

waylan · 2021-04-02T14:48:27Z

in Fedora, we build the package using setuptools, but we ship it to users potentially without it.

Ah, I see. You are building for a third-party package manager. I only ever install Python packages with Python tools, so I forget about this case (and get annoyed when users file bugs because they didn't use Python tools). But my view that Python tools are better for Python packages is only an opinion and the license doesn't prevent others from making packages for other systems. but I digress...

you've been arguing that maybe it's OK to not specify a dependency

Yes, because I've never seen any Python package list setuptools as a dependency. Yet, most all of them require it for their setup.py script. Maybe the difference here is that we are actually calling some code from the package itself, not just the setup.py script.

However, we are not calling setuptools internally. We are using parse_version of pkg_resources to parse two version strings and compare them (with < or >). But... pkg_resources is distributed as part of setuptools. Sigh.

Perhaps we should explore using a third-party single-purpose version parsing library. For example, Version Parser was the first result that turned up in a quick search. Although, there may be other (possibly better) options available. Unfortunately, the standard lib replacement for pkg_resources (importlib.metadata) only retrieves the version string, it doesn't include version parsing and/or comparison.

hroncok · 2021-04-03T07:30:41Z

you've been arguing that maybe it's OK to not specify a dependency

Yes, because I've never seen any Python package list setuptools as a dependency.

There are plenty such packages. When they use setuptools/pkg_resources on runtime, they specify the dependency.

Yet, most all of them require it for their setup.py script. Maybe the difference here is that we are actually calling some code from the package itself, not just the setup.py script.

Yes! That is the difference and that is the only reason I've opened this PR.

However, we are not calling setuptools internally. We are using parse_version of pkg_resources to parse two version strings and compare them (with < or >). But... pkg_resources is distributed as part of setuptools. Sigh.

For the sake of dependency declaration pkg_resources is (part of) setuptools.

Perhaps we should explore using a third-party single-purpose version parsing library. For example, Version Parser was the first result that turned up in a quick search. Although, there may be other (possibly better) options available. Unfortunately, the standard lib replacement for pkg_resources (importlib.metadata) only retrieves the version string, it doesn't include version parsing and/or comparison.

Consider packaging.

hroncok · 2021-04-03T07:33:00Z

Ah, I see. You are building for a third-party package manager. I only ever install Python packages with Python tools, so I forget about this case (and get annoyed when users file bugs because they didn't use Python tools). But my view that Python tools are better for Python packages is only an opinion and the license doesn't prevent others from making packages for other systems. but I digress...

That's totally fine by me. That's why I demonstrated the situation here using Python tools only.

waylan · 2021-04-03T18:00:22Z

Consider packaging.

Yes, thank you. I knew there was some package that did this, but I couldn't remember what it was. A PR which replaced the calls to pkg_resources with calls to packaging (and listed packaging as a dependency) would be welcome. If you wanted to update this one or open a new one is your choice.

hroncok · 2021-04-03T18:10:29Z

This projects uses pkg_resources in multiple places (I have only found out about the others now):

mkdocs/plugins.py
7:import pkg_resources
27:    plugins = pkg_resources.iter_entry_points(group='mkdocs.plugins')

mkdocs/utils/__init__.py
11:import pkg_resources
316:    builtins = pkg_resources.get_entry_map(dist='mkdocs', group='mkdocs.themes')
318:    for theme in pkg_resources.iter_entry_points(group='mkdocs.themes'):

mkdocs/commands/gh_deploy.py
5:from pkg_resources import parse_version

mkdocs/tests/plugin_tests.py
213:@mock.patch('pkg_resources.iter_entry_points', return_value=[MockEntryPoint])

mkdocs/tests/utils/utils_tests.py
197:    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
215:    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
227:    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
246:    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
247:    @mock.patch('pkg_resources.get_entry_map', autospec=True)

Plus apparently the tests mocks it as well.

I came here, clicked on an pencil bottom a added an undeclared dependency to a list, because I've noticed it was missing. I am not familiar with mkdocs codebase at all. Please don't expect me to go and migrate from one library to another. If you want to do that in the future to avoid a runtime dependency on setuptools, I suggest merging this PR first and opening a tracking issue, however I don't really see the immediate benefit of doing that. Either you depend on setuptools (which as you've noted is already installed in the majority of the cases) or you depend on packaging (which isn't) and importlib_metadata on Python < 3.8 as well.

waylan · 2021-04-04T18:29:07Z

This projects uses pkg_resources in multiple places

All of those are for collecting entry_points for extension support. They should be migrated to use importlib.metadata. Quite frankly, I thought we did that already. Apparently I was thinking of another project I work on. In any event, we should fix this correctly, and that is no not rely on pkg_resources at all. There is no point applying this "fix" as we will not be doing a new release until it is fixed correctly. I've added this to the milestone and consider it a blocker to the next release. If anyone wants to help get the next release out, they are welcome to contribute. Otherwise I'll get to when I do.

hroncok · 2021-04-04T18:30:03Z

Thanks.

This removes the final dependency in pkg_resources. Closes mkdocs#2347.

This removes the final dependency in pkg_resources. Closes #2347.

As per discussion on mkdocs#2347 our new babel code base should be migrated to not require setuptools and use importlib_metadata instead

Require setuptools, mkdocs/commands/gh_deploy.py imports pkg_resources

49aea32

This comment was marked as abuse.

Sign in to view

waylan added this to the 1.2 milestone Apr 4, 2021

waylan mentioned this pull request Apr 6, 2021

Replace pkg_resources with importlib_metadata #2354

Merged

waylan added a commit to waylan/mkdocs that referenced this pull request Apr 6, 2021

Replace pgk_resources.parse_version with packaging.version.parse

be67e91

This removes the final dependency in pkg_resources. Closes mkdocs#2347.

waylan mentioned this pull request Apr 6, 2021

Replace pgk_resources.parse_version with packaging.version.parse #2355

Merged

waylan closed this in #2355 Apr 6, 2021

waylan added a commit that referenced this pull request Apr 6, 2021

Replace pgk_resources.parse_version with packaging.version.parse

24f7e12

This removes the final dependency in pkg_resources. Closes #2347.

hrnciar mentioned this pull request May 7, 2021

Require setuptools, py3status/core.py and tests/test_user_modules.py import pkg_resources ultrabug/py3status#2043

Merged

ultrabug mentioned this pull request May 7, 2021

Replace pkg_resources with importlib_metadata wrt #2354 #2395

Closed

This comment was marked as abuse.

Sign in to view

Uh oh!

Uh oh!

Conversation

hroncok commented Mar 31, 2021

Uh oh!

waylan commented Mar 31, 2021

Uh oh!

hroncok commented Mar 31, 2021

Uh oh!

hroncok commented Mar 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hroncok commented Mar 31, 2021

Uh oh!

waylan commented Mar 31, 2021

Uh oh!

This comment was marked as abuse.

hroncok commented Mar 31, 2021

Uh oh!

waylan commented Apr 2, 2021

Uh oh!

hroncok commented Apr 3, 2021

Uh oh!

hroncok commented Apr 3, 2021

Uh oh!

waylan commented Apr 3, 2021

Uh oh!

hroncok commented Apr 3, 2021

Uh oh!

waylan commented Apr 4, 2021

Uh oh!

hroncok commented Apr 4, 2021

Uh oh!

This comment was marked as abuse.

This comment was marked as abuse.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hroncok commented Mar 31, 2021 •

edited

Loading