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

replace pep425tags with packaging #346

Merged
merged 8 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/codeqa-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ jobs:
uses: actions/setup-python@v1
with:
python-version: ${{ matrix.python-version }}
- name: Upgrade setuptools
run: pip install "setuptools >= 40.9"
- name: Upgrade setuptools, pip
run: pip install "pip>=20.0.2" "setuptools >= 44" "packaging"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does pip have to be upgraded?
And why is packaging explicitly installed here even though it's an install dependency?
Why is the setuptools requirement here higher than in setup.cfg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting, let's see if it passes tests. I guess I was thinking of PyPy, which would require these.

- name: Install the project
run: "pip install --no-binary=:all: ."
- name: Install test dependencies
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ package_dir=
packages = find:
python_requires = >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
setup_requires = setuptools >= 40.9.0
install_requires = packaging
Copy link
Contributor

Choose a reason for hiding this comment

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

Will any version work or is there a minimum required version?

zip_safe = False

[options.packages.find]
Expand Down
168 changes: 151 additions & 17 deletions src/wheel/bdist_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,22 @@
import re
from collections import OrderedDict
from email.generator import Generator
import distutils
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the module level imports to the top of the group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

from distutils.core import Command
from distutils.sysconfig import get_python_version
from distutils.sysconfig import get_config_var
from distutils import log as logger
from glob import iglob
from shutil import rmtree
from warnings import warn
import warnings
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this import to the top too please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

from zipfile import ZIP_DEFLATED, ZIP_STORED

import packaging.tags as tags
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe from packaging import tags instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adopting

import pkg_resources

from .pep425tags import get_abbr_impl, get_impl_ver, get_abi_tag, get_platform
from .pkginfo import write_pkg_info
from .macosx_libfile import extract_macosx_min_system_version
from .metadata import pkginfo_to_metadata
from .wheelfile import WheelFile
from . import pep425tags
from . import __version__ as wheel_version


Expand All @@ -35,6 +36,135 @@
PY_LIMITED_API_PATTERN = r'cp3\d'


def python_tag():
return 'py{}'.format(sys.version_info[0])


def get_platform(archive_root):
"""Return our platform name 'win32', 'linux_x86_64'"""
# XXX remove distutils dependency
result = distutils.util.get_platform()
if result.startswith("macosx") and archive_root is not None:
result = calculate_macosx_platform_tag(archive_root, result)
return result.replace('.', '_').replace('-', '_')


def calculate_macosx_platform_tag(archive_root, platform_tag):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function should go into the macos specific module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"""
Calculate proper macosx platform tag basing on files which are included to wheel

Example platform tag `macosx-10.14-x86_64`
"""
prefix, base_version, suffix = platform_tag.split('-')
base_version = tuple([int(x) for x in base_version.split(".")])
if len(base_version) >= 2:
base_version = base_version[0:2]

assert len(base_version) == 2
if "MACOSX_DEPLOYMENT_TARGET" in os.environ:
deploy_target = tuple([int(x) for x in os.environ[
"MACOSX_DEPLOYMENT_TARGET"].split(".")])
if len(deploy_target) >= 2:
deploy_target = deploy_target[0:2]
if deploy_target < base_version:
sys.stderr.write(
"[WARNING] MACOSX_DEPLOYMENT_TARGET is set to a lower value ({}) than the "
"version on which the Python interpreter was compiled ({}), and will be "
"ignored.\n".format('.'.join(str(x) for x in deploy_target),
'.'.join(str(x) for x in base_version))
)
else:
base_version = deploy_target

assert len(base_version) == 2
start_version = base_version
versions_dict = {}
for (dirpath, dirnames, filenames) in os.walk(archive_root):
for filename in filenames:
if filename.endswith('.dylib') or filename.endswith('.so'):
lib_path = os.path.join(dirpath, filename)
min_ver = extract_macosx_min_system_version(lib_path)
if min_ver is not None:
versions_dict[lib_path] = min_ver[0:2]

if len(versions_dict) > 0:
base_version = max(base_version, max(versions_dict.values()))

# macosx platform tag do not support minor bugfix release
fin_base_version = "_".join([str(x) for x in base_version])
if start_version < base_version:
problematic_files = [k for k, v in versions_dict.items() if v > start_version]
problematic_files = "\n".join(problematic_files)
if len(problematic_files) == 1:
files_form = "this file"
else:
files_form = "these files"
error_message = \
"[WARNING] This wheel needs a higher macOS version than {} " \
"To silence this warning, set MACOSX_DEPLOYMENT_TARGET to at least " +\
fin_base_version + " or recreate " + files_form + " with lower " \
"MACOSX_DEPLOYMENT_TARGET: \n" + problematic_files

if "MACOSX_DEPLOYMENT_TARGET" in os.environ:
error_message = error_message.format("is set in MACOSX_DEPLOYMENT_TARGET variable.")
else:
error_message = error_message.format(
"the version your Python interpreter is compiled against.")

sys.stderr.write(error_message)

platform_tag = prefix + "_" + fin_base_version + "_" + suffix
return platform_tag


def get_flag(var, fallback, expected=True, warn=True):
"""Use a fallback value for determining SOABI flags if the needed config
var is unset or unavailable."""
val = get_config_var(var)
if val is None:
if warn:
warnings.warn("Config variable '{0}' is unset, Python ABI tag may "
"be incorrect".format(var), RuntimeWarning, 2)
return fallback
return val == expected


def get_abi_tag():
"""Return the ABI tag based on SOABI (if available) or emulate SOABI
(CPython 2, PyPy)."""
soabi = get_config_var('SOABI')
impl = tags.interpreter_name()
if not soabi and impl in ('cp', 'pp') and hasattr(sys, 'maxunicode'):
d = ''
m = ''
u = ''
if get_flag('Py_DEBUG',
hasattr(sys, 'gettotalrefcount'),
warn=(impl == 'cp')):
d = 'd'
if get_flag('WITH_PYMALLOC',
impl == 'cp',
warn=(impl == 'cp' and
sys.version_info < (3, 8))) \
and sys.version_info < (3, 8):
m = 'm'
if get_flag('Py_UNICODE_SIZE',
sys.maxunicode == 0x10ffff,
expected=4,
warn=(impl == 'cp' and
sys.version_info < (3, 3))) \
and sys.version_info < (3, 3):
u = 'u'
abi = '%s%s%s%s%s' % (impl, tags.interpreter_version(), d, m, u)
elif soabi and soabi.startswith('cpython-'):
abi = 'cp' + soabi.split('-')[1]
elif soabi:
abi = soabi.replace('.', '_').replace('-', '_')
else:
abi = None
return abi


def safer_name(name):
return safe_name(name).replace('-', '_')

Expand Down Expand Up @@ -88,7 +218,7 @@ class bdist_wheel(Command):
.format(', '.join(supported_compressions))),
('python-tag=', None,
"Python implementation compatibility tag"
" (default: py%s)" % get_impl_ver()[0]),
" (default: '%s')" % (python_tag())),
('build-number=', None,
"Build number for this particular version. "
"As specified in PEP-0427, this must start with a digit. "
Expand Down Expand Up @@ -116,7 +246,7 @@ def initialize_options(self):
self.group = None
self.universal = False
self.compression = 'deflated'
self.python_tag = 'py' + get_impl_ver()[0]
self.python_tag = python_tag()
self.build_number = None
self.py_limited_api = False
self.plat_name_supplied = False
Expand Down Expand Up @@ -178,6 +308,12 @@ def get_tag(self):
if self.plat_name and not self.plat_name.startswith("macosx"):
plat_name = self.plat_name
else:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see how the condition on line 308 can be satisfied, since self.plat_name_supplied is set to self.plat_name is not None. It would seem that self.plat_name_supplied can be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was in code before #314. But it somehow work, because I meet problem and need to create also #317. Maybe plat_name is set somehow in base class.

# on macosx always limit the platform name to comply with any
# c-extension modules in bdist_dir, since the user can specify
# a higher MACOSX_DEPLOYMENT_TARGET via tools like CMake

# on other platforms, and on macosx if there are no c-extension
# modules, use the default platform name.
plat_name = get_platform(self.bdist_dir)

if plat_name in ('linux-x86_64', 'linux_x86_64') and sys.maxsize == 2147483647:
Expand All @@ -192,8 +328,8 @@ def get_tag(self):
impl = self.python_tag
tag = (impl, 'none', plat_name)
else:
impl_name = get_abbr_impl()
impl_ver = get_impl_ver()
impl_name = tags.interpreter_name()
impl_ver = tags.interpreter_version()
impl = impl_name + impl_ver
# We don't work on CPython 3.1, 3.0.
if self.py_limited_api and (impl_name + impl_ver).startswith('cp3'):
Expand All @@ -202,12 +338,8 @@ def get_tag(self):
else:
abi_tag = str(get_abi_tag()).lower()
tag = (impl, abi_tag, plat_name)
supported_tags = pep425tags.get_supported(
self.bdist_dir,
supplied_platform=plat_name if self.plat_name_supplied else None)
# XXX switch to this alternate implementation for non-pure:
if not self.py_limited_api:
assert tag == supported_tags[0], "%s != %s" % (tag, supported_tags[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this check important? The order of tags returned from sys_tags is different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not, but I didn't write that code.

supported_tags = [(t.interpreter, t.abi, t.platform)
for t in tags.sys_tags()]
assert tag in supported_tags, "would build wheel with unsupported tag {}".format(tag)
return tag

Expand Down Expand Up @@ -286,7 +418,9 @@ def run(self):

# Add to 'Distribution.dist_files' so that the "upload" command works
getattr(self.distribution, 'dist_files', []).append(
('bdist_wheel', get_python_version(), wheel_path))
('bdist_wheel',
'.'.join(list(tags.interpreter_version())), # like 3.7
Copy link
Contributor

Choose a reason for hiding this comment

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

The list() seems unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is tags.interpreter_version ever any different from the Python version?
This will also react badly to Python 3.10.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixing via sys.version_info

wheel_path))

if not self.keep_temp:
logger.info('removing %s', self.bdist_dir)
Expand Down Expand Up @@ -330,8 +464,8 @@ def license_paths(self):
})

if 'license_file' in metadata:
warn('The "license_file" option is deprecated. Use "license_files" instead.',
DeprecationWarning)
warnings.warn('The "license_file" option is deprecated. Use '
'"license_files" instead.', DeprecationWarning)
files.add(metadata['license_file'][1])

if 'license_file' not in metadata and 'license_files' not in metadata:
Expand Down