Skip to content

Commit

Permalink
pw_build: Python package fixes
Browse files Browse the repository at this point in the history
- Better error messaging when a python package can't be found.
- Sort pw_python_package listing in the generated requirements output.
- Switch the loop order when merging python packages as part of
  pw_create_python_source_tree. Before all package build output shared
  the same /tmp location which lead to problems if directories with
  the same name exist in multiple python packages. Now each
  python package gets its own /tmp build dir.
- Merge unique options.package_data entries that share the same
  package name. This prevents merged packages from clobbering
  eachothers entries.
- Add import error handling to pw_build_mcuxpresso. It is often run
  during gn gen stage when it may not be installed as a Python package.
- Remove python_dep on $dir_pw_protobuf_compiler/py for proto ._gen
  targets. This isn't required for running generate_protos.py and was
  propagating unnecessary deps.

Change-Id: I8b66423e464e1e1f7144841194d73a966b6391d6
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/103480
Reviewed-by: Armando Montanez <amontanez@google.com>
Commit-Queue: Anthony DiGirolamo <tonymd@google.com>
Pigweed-Auto-Submit: Anthony DiGirolamo <tonymd@google.com>
  • Loading branch information
AnthonyDiGirolamo authored and CQ Bot Account committed Jul 30, 2022
1 parent 4441396 commit cdb1629
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 20 deletions.
22 changes: 12 additions & 10 deletions pw_build/py/pw_build/create_python_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,13 @@ def update_config_with_packages(
# Collect package_data
if pkg.config.has_section('options.package_data'):
for key, value in pkg.config['options.package_data'].items():
config['options.package_data'][key] = value
existing_values = config['options.package_data'].get(
key, '').splitlines()
new_value = '\n'.join(
sorted(set(existing_values + value.splitlines())))
# Remove any empty lines
new_value = new_value.replace('\n\n', '\n')
config['options.package_data'][key] = new_value

# Collect entry_points
if pkg.config.has_section('options.entry_points'):
Expand Down Expand Up @@ -257,21 +263,17 @@ def build_python_tree(python_packages: Iterable[PythonPackage],
shutil.rmtree(destination_path, ignore_errors=True)
destination_path.mkdir(exist_ok=True)

# Define a temporary location to run setup.py build in.
with tempfile.TemporaryDirectory() as build_base_name:
build_base = Path(build_base_name)
for pkg in python_packages:
# Define a temporary location to run setup.py build in.
with tempfile.TemporaryDirectory() as build_base_name:
build_base = Path(build_base_name)

for pkg in python_packages:
lib_dir_path = setuptools_build_with_base(
pkg, build_base, include_tests=include_tests)

# Move installed files from the temp build-base into
# destination_path.
for new_file in lib_dir_path.glob('*'):
# Use str(Path) since shutil.move only accepts path-like objects
# in Python 3.9 and up:
# https://docs.python.org/3/library/shutil.html#shutil.move
shutil.move(str(new_file), str(destination_path))
shutil.copytree(lib_dir_path, destination_path, dirs_exist_ok=True)

# Clean build base lib folder for next install
shutil.rmtree(lib_dir_path, ignore_errors=True)
Expand Down
4 changes: 3 additions & 1 deletion pw_build/py/pw_build/generate_python_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ def main(
'# Auto-generated requirements.txt from the following packages:\n'
'#\n')
output += '\n'.join('# ' + pkg.gn_target_name
for pkg in target_py_packages)
for pkg in sorted(target_py_packages,
key=lambda pkg: pkg.gn_target_name))

output += config['options']['install_requires']
output += '\n'
requirement.write_text(output)
Expand Down
37 changes: 30 additions & 7 deletions pw_build/py/pw_build/python_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@
import configparser
from contextlib import contextmanager
import copy
from dataclasses import dataclass
from dataclasses import dataclass, asdict
import io
import json
import os
from pathlib import Path
import pprint
import re
import shutil
from typing import Dict, List, Optional, Iterable
from typing import Any, Dict, List, Optional, Iterable

_pretty_format = pprint.PrettyPrinter(indent=1, width=120).pformat

# List of known environment markers supported by pip.
# https://peps.python.org/pep-0508/#environment-markers
Expand Down Expand Up @@ -132,25 +136,44 @@ def setup_cfg(self) -> Optional[Path]:
return None
return setup_cfg[0]

def as_dict(self) -> Dict[Any, Any]:
"""Return a dict representation of this class."""
self_dict = asdict(self)
if self.config:
# Expand self.config into text.
setup_cfg_text = io.StringIO()
self.config.write(setup_cfg_text)
self_dict['config'] = setup_cfg_text.getvalue()
return self_dict

@property
def package_name(self) -> str:
unknown_package_message = (
'Cannot determine the package_name for the Python '
f'library/package: {self.gn_target_name}\n\n'
'This could be due to a missing python dependency in GN for:\n'
f'{self.gn_target_name}\n\n')

if self.config:
return self.config['metadata']['name']
try:
name = self.config['metadata']['name']
except KeyError:
raise UnknownPythonPackageName(unknown_package_message +
_pretty_format(self.as_dict()))
return name
top_level_source_dir = self.top_level_source_dir
if top_level_source_dir:
return top_level_source_dir.name

actual_gn_target_name = self.gn_target_name.split(':')
if len(actual_gn_target_name) < 2:
raise UnknownPythonPackageName(
'Cannot determine the package_name for the Python '
f'library/package: {self}')
raise UnknownPythonPackageName(unknown_package_message)

return actual_gn_target_name[-1]

@property
def package_dir(self) -> Path:
if self.setup_cfg:
if self.setup_cfg and self.setup_cfg.is_file():
return self.setup_cfg.parent / self.package_name
root_source_dir = self.top_level_source_dir
if root_source_dir:
Expand Down
6 changes: 5 additions & 1 deletion pw_build_mcuxpresso/py/pw_build_mcuxpresso/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
import pathlib
import sys

from pw_build_mcuxpresso import components
try:
from pw_build_mcuxpresso import components
except ImportError:
# Load from this directory if pw_build_mcuxpresso is not available.
import components # type: ignore


def _parse_args() -> argparse.Namespace:
Expand Down
18 changes: 17 additions & 1 deletion pw_protobuf_compiler/proto.gni
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,23 @@ template("_pw_invoke_protoc") {
script =
"$dir_pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py"

python_deps = [ "$dir_pw_protobuf_compiler/py" ]
if (pw_build_USE_NEW_PYTHON_BUILD) {
# NOTE: A python_dep on "$dir_pw_protobuf_compiler/py" should not be
# included when using the new Python build. It triggers building that
# Python package which requires the build venv to be created. The venv
# creation will drag in many unnecessary dependencies that may not be
# available when this proto is generated.
python_deps = []

# Add pw_protobuf_compiler and its dependencies to the PYTHONPATH when
# running this action.
python_metadata_deps =
[ "$dir_pw_protobuf_compiler/py:py._package_metadata" ]
} else {
python_deps = [ "$dir_pw_protobuf_compiler/py" ]
}

python_deps = []
if (defined(invoker.python_deps)) {
python_deps += invoker.python_deps
}
Expand Down

0 comments on commit cdb1629

Please sign in to comment.