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

setup.py: Remove use of "imp" module #652

Merged
merged 3 commits into from
Jan 9, 2024
+49 −84
Merged

setup.py: Remove use of "imp" module #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53288e2
setup.py: Remove use of "imp" module
douglas-raillard-arm Oct 5, 2023
157c15e
devlib: Remove Python 2 dead code
douglas-raillard-arm Oct 5, 2023
c4ebee7
setup.py: Re-add "future" PyPI package
douglas-raillard-arm Oct 5, 2023
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
3 changes: 1 addition & 2 deletions devlib/collector/ftrace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,7 @@ def report(self, binfile, destfile):
self.logger.debug(command)
process = subprocess.Popen(command, stderr=subprocess.PIPE, shell=True)
_, error = process.communicate()
if sys.version_info[0] == 3:
error = error.decode(sys.stdout.encoding or 'utf-8', 'replace')
error = error.decode(sys.stdout.encoding or 'utf-8', 'replace')
if process.returncode:
raise TargetStableError('trace-cmd returned non-zero exit code {}'.format(process.returncode))
if error:
Expand Down
5 changes: 1 addition & 4 deletions devlib/instrument/acmecape.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,7 @@ def stop(self):
msg = 'Could not terminate iio-capture:\n{}'
raise HostError(msg.format(output))
if self.process.returncode != 15: # iio-capture exits with 15 when killed
if sys.version_info[0] == 3:
output += self.process.stdout.read().decode(sys.stdout.encoding or 'utf-8', 'replace')
else:
output += self.process.stdout.read()
output += self.process.stdout.read().decode(sys.stdout.encoding or 'utf-8', 'replace')
self.logger.info('ACME instrument encountered an error, '
'you may want to try rebooting the ACME device:\n'
' ssh root@{} reboot'.format(self.host))
Expand Down
5 changes: 2 additions & 3 deletions devlib/instrument/energy_probe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ def stop(self):
self.process.poll()
if self.process.returncode is not None:
stdout, stderr = self.process.communicate()
if sys.version_info[0] == 3:
stdout = stdout.decode(sys.stdout.encoding or 'utf-8', 'replace')
stderr = stderr.decode(sys.stdout.encoding or 'utf-8', 'replace')
stdout = stdout.decode(sys.stdout.encoding or 'utf-8', 'replace')
stderr = stderr.decode(sys.stdout.encoding or 'utf-8', 'replace')
raise HostError(
'Energy Probe: Caiman exited unexpectedly with exit code {}.\n'
'stdout:\n{}\nstderr:\n{}'.format(self.process.returncode,
Expand Down
5 changes: 2 additions & 3 deletions devlib/instrument/monsoon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ def stop(self):
process.poll()
if process.returncode is not None:
stdout, stderr = process.communicate()
if sys.version_info[0] == 3:
stdout = stdout.encode(sys.stdout.encoding or 'utf-8')
stderr = stderr.encode(sys.stdout.encoding or 'utf-8')
stdout = stdout.encode(sys.stdout.encoding or 'utf-8')
stderr = stderr.encode(sys.stdout.encoding or 'utf-8')
raise HostError(
'Monsoon script exited unexpectedly with exit code {}.\n'
'stdout:\n{}\nstderr:\n{}'.format(process.returncode,
Expand Down
5 changes: 1 addition & 4 deletions devlib/target.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2018,10 +2018,7 @@ async def capture_ui_hierarchy(self, filepath):

parsed_xml = xml.dom.minidom.parse(filepath)
with open(filepath, 'w') as f:
if sys.version_info[0] == 3:
f.write(parsed_xml.toprettyxml())
else:
f.write(parsed_xml.toprettyxml().encode('utf-8'))
f.write(parsed_xml.toprettyxml())

@asyn.asyncf
async def is_installed(self, name):
Expand Down
3 changes: 1 addition & 2 deletions devlib/utils/android.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,7 @@ def _run(self, command):
logger.debug(' '.join(command))
try:
output = subprocess.check_output(command, stderr=subprocess.STDOUT)
if sys.version_info[0] == 3:
output = output.decode(sys.stdout.encoding or 'utf-8', 'replace')
output = output.decode(sys.stdout.encoding or 'utf-8', 'replace')
except subprocess.CalledProcessError as e:
raise HostError('Error while running "{}":\n{}'
.format(command, e.output))
Expand Down
20 changes: 4 additions & 16 deletions devlib/utils/csvutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@

@contextmanager
def csvwriter(filepath, *args, **kwargs):
if sys.version_info[0] == 3:
wfh = open(filepath, 'w', newline='')
else:
wfh = open(filepath, 'wb')
wfh = open(filepath, 'w', newline='')

try:
yield csv.writer(wfh, *args, **kwargs)
Expand All @@ -73,10 +70,7 @@ def csvwriter(filepath, *args, **kwargs):

@contextmanager
def csvreader(filepath, *args, **kwargs):
if sys.version_info[0] == 3:
fh = open(filepath, 'r', newline='')
else:
fh = open(filepath, 'rb')
fh = open(filepath, 'r', newline='')

try:
yield csv.reader(fh, *args, **kwargs)
Expand All @@ -85,16 +79,10 @@ def csvreader(filepath, *args, **kwargs):


def create_writer(filepath, *args, **kwargs):
if sys.version_info[0] == 3:
wfh = open(filepath, 'w', newline='')
else:
wfh = open(filepath, 'wb')
wfh = open(filepath, 'w', newline='')
return csv.writer(wfh, *args, **kwargs), wfh


def create_reader(filepath, *args, **kwargs):
if sys.version_info[0] == 3:
fh = open(filepath, 'r', newline='')
else:
fh = open(filepath, 'rb')
fh = open(filepath, 'r', newline='')
return csv.reader(fh, *args, **kwargs), fh
6 changes: 1 addition & 5 deletions devlib/utils/misc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,11 +594,7 @@ def __str__(self):

RAND_MOD_NAME_LEN = 30
BAD_CHARS = string.punctuation + string.whitespace
# pylint: disable=no-member
if sys.version_info[0] == 3:
TRANS_TABLE = str.maketrans(BAD_CHARS, '_' * len(BAD_CHARS))
else:
TRANS_TABLE = string.maketrans(BAD_CHARS, '_' * len(BAD_CHARS))
TRANS_TABLE = str.maketrans(BAD_CHARS, '_' * len(BAD_CHARS))


def to_identifier(text):
Expand Down
5 changes: 1 addition & 4 deletions devlib/utils/rendering.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,7 @@ def __init__(self, target, period, package, header=None):
def collect_frames(self, wfh):
cmd = 'dumpsys gfxinfo {} framestats'
result = self.target.execute(cmd.format(self.package))
if sys.version_info[0] == 3:
wfh.write(result.encode('utf-8'))
else:
wfh.write(result)
wfh.write(result.encode('utf-8'))

def clear(self):
pass
Expand Down
11 changes: 2 additions & 9 deletions devlib/utils/ssh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,11 +758,7 @@ def callback(output_chunks, name, chunk):
output_chunks, exit_code = _read_paramiko_streams(stdout, stderr, select_timeout, callback, [])
# Join in one go to avoid O(N^2) concatenation
output = b''.join(output_chunks)

if sys.version_info[0] == 3:
output = output.decode(sys.stdout.encoding or 'utf-8', 'replace')
if strip_colors:
output = strip_bash_colors(output)
output = output.decode(sys.stdout.encoding or 'utf-8', 'replace')

return (exit_code, output)

Expand Down Expand Up @@ -970,10 +966,7 @@ def _execute_and_wait_for_prompt(self, command, timeout=None, as_root=False, str
logger.debug(command)
self._sendline(command)
timed_out = self._wait_for_prompt(timeout)
if sys.version_info[0] == 3:
output = process_backspaces(self.conn.before.decode(sys.stdout.encoding or 'utf-8', 'replace'))
else:
output = process_backspaces(self.conn.before)
output = process_backspaces(self.conn.before.decode(sys.stdout.encoding or 'utf-8', 'replace'))

if timed_out:
self.cancel_running_command()
Expand Down
48 changes: 19 additions & 29 deletions devlib/utils/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,35 +136,25 @@ def bitmask(value):
regex_type = type(re.compile(''))


if sys.version_info[0] == 3:
def regex(value):
if isinstance(value, regex_type):
if isinstance(value.pattern, str):
return value
return re.compile(value.pattern.decode(),
value.flags | re.UNICODE)
else:
if isinstance(value, bytes):
value = value.decode()
return re.compile(value)


def bytes_regex(value):
if isinstance(value, regex_type):
if isinstance(value.pattern, bytes):
return value
return re.compile(value.pattern.encode(sys.stdout.encoding or 'utf-8'),
value.flags & ~re.UNICODE)
else:
if isinstance(value, str):
value = value.encode(sys.stdout.encoding or 'utf-8')
return re.compile(value)
else:
def regex(value):
if isinstance(value, regex_type):
def regex(value):
if isinstance(value, regex_type):
if isinstance(value.pattern, str):
return value
else:
return re.compile(value)
return re.compile(value.pattern.decode(),
value.flags | re.UNICODE)
else:
if isinstance(value, bytes):
value = value.decode()
return re.compile(value)


bytes_regex = regex
def bytes_regex(value):
if isinstance(value, regex_type):
if isinstance(value.pattern, bytes):
return value
return re.compile(value.pattern.encode(sys.stdout.encoding or 'utf-8'),
value.flags & ~re.UNICODE)
else:
if isinstance(value, str):
value = value.encode(sys.stdout.encoding or 'utf-8')
return re.compile(value)
2 changes: 1 addition & 1 deletion devlib/utils/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def get_commit():
p.wait()
if p.returncode:
return None
if sys.version_info[0] == 3 and isinstance(std, bytes):
if isinstance(std, bytes):
return std[:8].decode(sys.stdout.encoding or 'utf-8', 'replace')
else:
return std[:8]
15 changes: 13 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
# limitations under the License.
#

import imp
import os
import sys
import warnings
from itertools import chain
import types

try:
from setuptools import setup
Expand All @@ -35,15 +35,25 @@
warnings.filterwarnings('ignore', "Unknown distribution option: 'install_requires'")
warnings.filterwarnings('ignore', "Unknown distribution option: 'extras_require'")


try:
os.remove('MANIFEST')
except OSError:
pass


def _load_path(filepath):
# Not a proper import in many, many ways but does the job for really basic
# needs
with open(filepath) as f:
globals_ = dict(__file__=filepath)
exec(f.read(), globals_)
return types.SimpleNamespace(**globals_)


vh_path = os.path.join(devlib_dir, 'utils', 'version.py')
# can load this, as it does not have any devlib imports
version_helper = imp.load_source('version_helper', vh_path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a completely naïve question without looking into actual implementation details.
I was wondering if there would be any benefit in using the example replacement provided in the Python release notes [1] both in devlib and referring to this from WA?

Otherwise I've tested the PR and this looks like it does the job.

[1] https://docs.python.org/dev/whatsnew/3.12.html#imp

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes and no. The snippet they provide is:

import importlib.util
import importlib.machinery

def load_source(modname, filename):
    loader = importlib.machinery.SourceFileLoader(modname, filename)
    spec = importlib.util.spec_from_file_location(modname, filename, loader=loader)
    module = importlib.util.module_from_spec(spec)
    # The module is always executed and not cached in sys.modules.
    # Uncomment the following line to cache the module.
    # sys.modules[module.__name__] = module
    loader.exec_module(module)
    return module

That is closer to how an import using the import statement works, but it's not even fully accurate:

  • Relative imports in the module won't work (unless you uncomment the sys.modules assignment, despite the comment only stating something about caching, it's actually "mandatory caching")
  • Circular dependencies won't work (ditto)
  • return module is actually wrong, it should use return sys.modules[module.__name__] as some modules manipulate sys.modules[__name__] and the import statement does honor that.

So all that to say that if you want a properly "compliant" way of doing it, it can be quite tricky, and on top of that these APIs have changed approximately every 5 minutes, meaning that it's unlikely this snippet will stay "the best way to do it" for more than a couple years (look for "deprecated" in https://docs.python.org/3/library/importlib.html).

OTH, the exec() will always stay wrong and simple, and if something breaks it will be easy to understand why for anyone without being familiar with the intricacies of Python import system (e.g. no __name__ defined when running the module, it can just be added in the globals like I did for __file__).

referring to this from WA?

If you mean the plugin import system in WA, I wouldn't do that. Plugins expect to be proper Python modules so they should be imported the proper way. For version_helper.py, we have full control on what the module does, and we can easily tailor its import to its content if needed, we don't have this luxury with 3rd party modules.

version_helper = _load_path(vh_path)
__version__ = version_helper.get_devlib_version()
commit = version_helper.get_commit()
if commit:
Expand Down Expand Up @@ -94,6 +104,7 @@
'pandas',
'lxml', # More robust xml parsing
'nest_asyncio', # Allows running nested asyncio loops
'future', # for the "past" Python package
],
extras_require={
'daq': ['daqpower>=2'],
Expand Down