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.

Sign up for GitHub

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

Jump to bottom

rfe: Deprecate distutils module #3920

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

rfe: Deprecate distutils module #3920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 25 additions & 16 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,27 @@
# Copyright: Red Hat Inc. 2013-2014
# Author: Lucas Meneghel Rodrigues <lmr@redhat.com>

# pylint: disable=E0611
import os
import shutil
from distutils.command.clean import clean
from pathlib import Path

from setuptools import find_packages, setup
from setuptools import Command, find_packages, setup

VERSION = open("VERSION", "r").read().strip()


class Clean(clean):
"""Our custom command to get rid of scratch files after build."""
class CleanCommand(Command):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incompatible with the original use of the clean command.

[before]

$ python setup.py clean --help
Common commands: (see '--help-commands' for more)

  setup.py build      will build the package underneath 'build/'
  setup.py install    will install the package

Global options:
  --verbose (-v)  run verbosely (default)
  --quiet (-q)    run quietly (turns verbosity off)
  --dry-run (-n)  don't actually do anything
  --help (-h)     show detailed help message
  --no-user-cfg   ignore pydistutils.cfg in your home directory

Options for 'Clean' command:
  --build-base (-b)  base build directory [default: 'build.build-base']
  --build-lib        build directory for all modules [default: 'build.build-
                     lib']
  --build-temp (-t)  temporary build directory [default: 'build.build-temp']
  --build-scripts    build directory for scripts [default: 'build.build-
                     scripts']
  --bdist-base       temporary directory for built distributions
  --all (-a)         remove all build output, not just temporary by-products

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

[after]

$ python setup.py clean --help                                            141 ↵
Common commands: (see '--help-commands' for more)

  setup.py build      will build the package underneath 'build/'
  setup.py install    will install the package

Global options:
  --verbose (-v)  run verbosely (default)
  --quiet (-q)    run quietly (turns verbosity off)
  --dry-run (-n)  don't actually do anything
  --help (-h)     show detailed help message
  --no-user-cfg   ignore pydistutils.cfg in your home directory

Options for 'CleanCommand' command:

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

FYI, see https://github.com/pypa/setuptools/blob/main/setuptools/_distutils/command/clean.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All clean options are wanna clean sub dir under build, however, our custom cleaning_list will clean the entire build dir, which means these options are not used for us.

avocado-vt/setup.py

Lines 33 to 43 in 4cfe8c9

cleaning_list = [
"MANIFEST",
"BUILD",
"BUILDROOT",
"SPECS",
"RPMS",
"SRPMS",
"SOURCES",
"PYPI_UPLOAD",
"./build",
]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And, after we switch to pip install in the future, we don't need to care about build dir, as it will use the tmp during build.

"""Custom command to clean up build and scratch files."""

description = "Get rid of scratch, byte files and build stuff."
description = "Custom clean command to tidy up the project root"
user_options = []

def initialize_options(self):
pass

def finalize_options(self):
pass

def run(self):
super().run()
cleaning_list = [
"MANIFEST",
"BUILD",
Expand All @@ -46,12 +49,12 @@ def run(self):
cleaning_list += list(Path(".").rglob("__pycache__"))

for e in cleaning_list:
if not os.path.exists(e):
continue
if os.path.isfile(e):
os.remove(e)
if os.path.isdir(e):
shutil.rmtree(e)
path = Path(e)
if path.exists():
if path.is_file():
path.unlink()
elif path.is_dir():
shutil.rmtree(path)


if __name__ == "__main__":
Expand Down Expand Up @@ -96,6 +99,12 @@ def run(self):
"avocado-vt = avocado_vt.plugins.vt_runner:VTTestRunner",
],
},
install_requires=["netifaces", "six", "aexpect", "avocado-framework>=82.1"],
cmdclass={"clean": Clean},
install_requires=[
"netifaces",
"packaging",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be in avocado rather than avocado-vt?

"six",
"aexpect",
"avocado-framework>=82.1",
],
cmdclass={"clean": CleanCommand},
)
9 changes: 6 additions & 3 deletions virttest/bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import re
import shutil
import sys
from distutils import dir_util # virtualenv problem pylint: disable=E0611

from avocado.utils import cpu, distro, genio, linux_modules
from avocado.utils import path as utils_path
Expand Down Expand Up @@ -910,7 +909,9 @@ def bootstrap(options, interactive=False):
LOG.info("%d - Updating test providers repo configuration from local copy", step)
tp_base_dir = data_dir.get_base_test_providers_dir()
tp_local_dir = data_dir.get_test_providers_dir()
dir_util.copy_tree(tp_base_dir, tp_local_dir)
if os.path.exists(tp_local_dir):
shutil.rmtree(tp_local_dir)
shutil.copytree(tp_base_dir, tp_local_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

shutil.copytree will raise FileExistsError against existing target dirs by default,
(there is an argument dirs_exist_ok added since Python 3.8 in favor to that)

>>> shutil.copytree('./a', './b')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.12/shutil.py", line 600, in copytree
    return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.12/shutil.py", line 498, in _copytree
    os.makedirs(dst, exist_ok=dirs_exist_ok)
  File "<frozen os>", line 225, in makedirs
FileExistsError: [Errno 17] File exists: './b'

while distutil.dir_util.copy_tree don't.

>>> dir_util.copy_tree('./a', './b')
['./b/foo']

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, as we also support python3.6, I will check the dst first, if it exists, then remove it.


not_downloaded = asset.test_providers_not_downloaded()
if not_downloaded:
Expand Down Expand Up @@ -950,7 +951,9 @@ def bootstrap(options, interactive=False):
LOG.info(
"%d - Syncing backend dirs %s -> %s", step, base_backend_dir, local_backend_dir
)
dir_util.copy_tree(base_backend_dir, local_backend_dir)
if os.path.exists(local_backend_dir):
shutil.rmtree(local_backend_dir)
shutil.copytree(base_backend_dir, local_backend_dir)

sync_download_dir(interactive)

Expand Down
11 changes: 5 additions & 6 deletions virttest/utils_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import operator
import re
from distutils.version import ( # pylint: disable=no-name-in-module,import-error
LooseVersion,
)

from packaging.version import parse


class VersionInterval(object):
Expand Down Expand Up @@ -31,8 +30,8 @@ def __init__(self, interval):
raise ValueError("Invalid string representation of an interval")
self.opening, lower, upper, self.closing = match.groups()

self.lower_bound = LooseVersion(lower) if lower else None
self.upper_bound = LooseVersion(upper) if upper else None
self.lower_bound = parse(lower) if lower else None
Copy link
Contributor

Choose a reason for hiding this comment

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

As the implementation of packaging.version.Version is quite different to distutils.version.LooseVersion's, I'm afraid we could not do the simple replacement like this, in order to keeping the compatibility.

self.upper_bound = parse(upper) if upper else None
self._check_interval()

def _check_interval(self):
Expand Down Expand Up @@ -64,7 +63,7 @@ def __contains__(self, version):
"]": operator.ge,
}
in_interval = True
version = LooseVersion(version)
version = parse(version)
if self.lower_bound:
opt = op_mapping.get(self.opening)
in_interval = opt(self.lower_bound, version)
Expand Down
Loading