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

Up version to 3.1.dev0 #736

Merged
merged 8 commits into from Oct 21, 2018
Merged

Up version to 3.1.dev0 #736

merged 8 commits into from Oct 21, 2018

Conversation

waylan
Copy link
Member

@waylan waylan commented Oct 19, 2018

Also update to be PEP 440 compliant in preparation for #732.

Also update to be PEP 440 compliant in preparation for Python-Markdown#732.
@waylan
Copy link
Member Author

waylan commented Oct 19, 2018

Just a note that [pypa/packaging] officially implements PEP 404. And as of pypa/setuptools@9382fa0c, (version 8.0) setuptools and pkg_resources vendor and use pypa/packaging to do all version parsing. Finally, since 52b8da8 (version 3.0) we are using setuptools for installation.

Therefore, the differences between pkg_resources and PEP 440 listed in the PEP are presumably outdated and we should be able to use pkg_resources for all version handling without any added dependencies.

@waylan
Copy link
Member Author

waylan commented Oct 19, 2018

Hmm, not sure why we are only getting this error on Python 3.4:

Traceback (most recent call last):
  File "setup.py", line 26, in <module>
    from markdown import __version__, __version_info__
  File "/home/travis/build/Python-Markdown/markdown/markdown/__init__.py", line 26, in <module>
    from pkg_resources.extern import packaging
ImportError: No module named 'pkg_resources.extern'

@mitya57
Copy link
Collaborator

mitya57 commented Oct 20, 2018

There was a period of time when setuptools did not ship pkg_resources.extern module.

Namely, it was:

So requiring setuptools>=36 may help.

@mitya57
Copy link
Collaborator

mitya57 commented Oct 20, 2018

Finally, since 52b8da8 (version 3.0) we are using setuptools for installation.

I also want to mention that with this PR, Python-Markdown needs a fresh version of setuptools both at install time and at run time. This needs to be specified in two different places:

  • To specify install-time dependency, you can use a pyproject.toml file specified by PEP 518.

    Like this:

    [build-system]
    requires = ["setuptools>=36"]

    This will be recognized by pip.

  • To specify run-time dependency, use install_requires in setup.py (or in setup.cfg). This is needed because users may have installed Python-Markdown using a wheel package, without having setuptools at all (or having an old version).

@facelessuser
Copy link
Collaborator

facelessuser commented Oct 20, 2018

Never knew about the toml file. I've had to use setup_requires which has it's issues. It's good to know there is a better way.

@waylan
Copy link
Member Author

waylan commented Oct 20, 2018

Yeah the toml file is relatively new. I have considered switching to it, but didn't see any added value till this issue.

There was a period of time when setuptools did not ship pkg_resources.extern module.

Namely, it was:

So requiring setuptools>=36 may help.

Ah, that was the detail I was missing. Thanks for pointing that out. At least we have a clear path to a solution now.

@waylan
Copy link
Member Author

waylan commented Oct 21, 2018

Adding the pyproject.toml file didn't have any effect on the Travis test as pip isn't used for the install. The command tox uses is 'python setup.py sdist --formats=zip --dist-dir .tox/dist'. I see a few possibilities:

  1. Tell tox to use a different command. Not sure if this is possible, but might be worth exploring for other reasons as well. For example, the zip format is used, but that is the least downloaded format on PyPI (or at least it was back when PyPI kept stats). We should be testing the most used format (whl?), or even better, all three. Current releases are available in tar.gz and whl formats only. So we should be testing those.
  2. List setuptools>=36 as a dependency in the tox and/or travis config files to unsure its installed before setup.py is run.

@facelessuser
Copy link
Collaborator

I think tox supports toml https://tox.readthedocs.io/en/latest/example/package.html?highlight=toml#setuptools. Maybe I'm mistaken?

@waylan
Copy link
Member Author

waylan commented Oct 21, 2018

Finally got Travis to pass. There are multiple moving parts here:

  1. Travis needs to install setuptools=>36 next to tox so that when it builds the lib (before installing into the env) it has access to the proper version.
  2. tox.ini needs isolated_build = True to tell tox to use the pyproject.toml file.
  3. tox requires the pyproject.toml file to define build-backend = "setuptools.build_meta".
  4. tox combines the pyproject.toml requires values with the tox.ini deps values. However, you can't combine individual package names with a -r requirements.txt call. Therefore, all items need to be listed individually in deps.
  5. tox.ini supports a requires setting, but all that does is short-circuit failure of the test. It does not install anything. I left it in anyway as it provides a clear error message in the event things break in the future.

@waylan
Copy link
Member Author

waylan commented Oct 21, 2018

Looks like I got this all working. A few final notes.

Unfortunately, there does not seem to be a way to tell tox how to build the lib. There is an install_command config setting, but that is used for installing the lib after it is built. Ideally, we would be testing wheel and/or gztar builds, not zip builds. I suppose the gztar and zip builds would be similar enough, but wheels are very different and we are not verifying at all that they build properly in our tests.

Apparently we could define the tox config in the pyproject.toml file under [tool.tox.legacy_tox_ini]. However, the entire entry needs to be a single multi-line string using the ini format. I'm inclined to wait until tox natively supports the toml format (which the docs claim is in the works) before going down that road.

@waylan waylan merged commit b431bbb into Python-Markdown:master Oct 21, 2018
@waylan waylan deleted the 3-1-dev branch October 21, 2018 20:32
@mitya57
Copy link
Collaborator

mitya57 commented Oct 21, 2018

Your last changes look good to me. Thanks!

@waylan
Copy link
Member Author

waylan commented Oct 21, 2018

Unfortunately, there does not seem to be a way to tell tox how to build the lib.

See tox-dev/tox#850. Apparently, with the changes in tox-dev/tox#954, wheel support can be added via a third-party plugin (which doesn't exist yet AFAICT). The reasons for this approach are discussed in detail in tox-dev/tox#573. If/when such a plugin becomes available, we should adopt it for our tests.

@eli-schwartz
Copy link

Depending on private, internal API of setuptools results in bugs like #825

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.

None yet

4 participants