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

Add setuptools.get_version(path, field='__version__') #1679

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog.d/1679.change.rst
@@ -0,0 +1,3 @@
Add ``setuptools.get_version(path, field='__version__')``. It extracts version
info from lines like ``__version__ = '0.12'"`` in text files and Python
sources without importing them.
2 changes: 0 additions & 2 deletions setup.cfg
Expand Up @@ -25,5 +25,3 @@ universal = 1
[metadata]
license_file = LICENSE

[bumpversion:file:setup.py]

2 changes: 1 addition & 1 deletion setup.py
Expand Up @@ -89,7 +89,7 @@ def pypi_link(pkg_filename):

setup_params = dict(
name="setuptools",
version="40.8.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although you could use get_version here, this usage would be discouraged. Better would be to store the value in setup.cfg under metadata.version (even if in duplicate), although doing so leads to a limitation in bump2version. What's more concerning about this change is the issues it introduces if setuptools were to move to a declarative config (something that's definitely desirable)... and this speaks to the resistance to accepting this change--it's encouraging patterns that run contrary to the best practices.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is the ETA to introduce the best practices to pip? Why doesn't it use best practices?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for declarative config is relatively recent and I haven't spent much time evangelizing it, so it's probably no big surprise that some projects haven't adopted it. Even setuptools itself, which because it installs itself, would have had no minimum dependency, only adopted declarative config many months after it was available. It's just a matter of priorities and resources. Perhaps that team would accept a pull request to use declarative config; I don't know.

version=setuptools.get_version("setup.cfg", field="current_version"),
description=(
"Easily download, build, install, upgrade, and uninstall "
"Python packages"
Expand Down
22 changes: 21 additions & 1 deletion setuptools/__init__.py
Expand Up @@ -27,7 +27,7 @@
__all__ = [
'setup', 'Distribution', 'Feature', 'Command', 'Extension', 'Require',
'SetuptoolsDeprecationWarning',
'find_packages'
'find_packages', 'get_version'
]

if PY3:
Expand Down Expand Up @@ -224,5 +224,25 @@ def findall(dir=os.curdir):
return list(files)


def get_version(path, field='__version__'):
"""
Get version information from a text file. Version is extracted
from "key = value" format with key matched at the beginning of
a line and specified by `field` parameter. Returns string.
"""
version_file = os.path.abspath(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this line has no useful effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be. Just a safe practice, useful for debugging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, let's omit it except when debugging.

# Using binary matching to avoid possible encoding problems
# when reading arbitrary text files
if type(field) is not bytes:
field = field.encode('utf-8')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This encode should happen unconditionally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it is already bytes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should not accept bytes on Python 3. On Python 2, if field is a str, it should be decodable through the default encoding, causing an implied decoding and explicit encoding using UTF-8.

I'm actually thinking now though that this function should only support files that are UTF-8 or where an alternate encoding is specified.

for line in open(version_file, 'rb'):
if line.startswith(field):
# __version__ = "0.9"
# current_version = 4.8.0
_, value = line.split(b'=')
version = value.strip(b' \r\n\t\'\"').decode()
return version


# Apply monkey patches
monkey.patch_all()
49 changes: 49 additions & 0 deletions setuptools/tests/test_get_version.py
@@ -0,0 +1,49 @@
# -*- coding: utf-8 -*-

"""Tests for setuptools.get_version()."""
import os
import codecs
import shutil
import tempfile

import pytest

from setuptools import __version__, get_version


def test_own_version():
version = get_version('setup.cfg', field='current_version')

# `setup.py egg_info` which is run in bootstrap.py during package
# installation adds `.post` prefix to setuptools.__version__
# which becomes different from 'setup.cfg` file

# https://setuptools.readthedocs.io/en/latest/setuptools.html#egg-info

assert __version__.startswith(version + '.post')


class TestFiles:
def setup_method(self, method):
self.tmpdir = tempfile.mkdtemp()

def teardown_method(self, method):
shutil.rmtree(self.tmpdir)

def test_python_file(self):
path = os.path.join(self.tmpdir, 'version.py')
with open(path, 'w') as fp:
fp.write('__version__ = "0.23beta"\n')

version = get_version(path)
assert version == '0.23beta'

def test_non_utf8_python_file(self):
path = os.path.join(self.tmpdir, 'russian.py')
with open(path, 'wb') as fp:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use io.open(..., encoding='cp1251').

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is called non_ut8. Python doesn't know that it is russuan or not - filename can be any.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I should have asked is if you could open this file as text with an encoding (cp1251) and then write unicode text to it.

fp.write(u'# файл в русской кодировке\n\n'.encode('cp1251'))
fp.write(u'__version__ = "17.0"\n'.encode('cp1251'))

version = get_version(path)
assert version == '17.0'