Skip to content

Commit

Permalink
Move back to print for logging to console in multiplatform builder
Browse files Browse the repository at this point in the history
The log writer cannot be pickled to be used in multiprocess builds,
and therefore is broken most of the time. Using print prevents that.
  • Loading branch information
saville committed Nov 10, 2023
1 parent 601b2b3 commit 8b9909f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 12 deletions.
1 change: 0 additions & 1 deletion buildrunner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,6 @@ def run(self): # pylint: disable=too-many-statements,too-many-branches,too-many
exit_explanation = None
try: # pylint: disable=too-many-nested-blocks
with MultiplatformImageBuilder(
log=self.log,
docker_registry=self.global_config.get_docker_registry(),
keep_images=not self.cleanup_images,
temp_dir=self.global_config.get_temp_dir(),
Expand Down
17 changes: 7 additions & 10 deletions buildrunner/docker/multiplatform_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from multiprocessing import Manager, Process
from platform import machine, system
import shutil
import sys
import tempfile
import uuid
from typing import Dict, List, Optional
Expand Down Expand Up @@ -122,15 +121,13 @@ class MultiplatformImageBuilder: # pylint: disable=too-many-instance-attributes
# pylint: disable=too-many-arguments
def __init__(
self,
log=sys.stdout,
docker_registry: Optional[str] = None,
use_local_registry: bool = True,
keep_images: bool = False,
temp_dir: str = os.getcwd(),
disable_multi_platform: bool = False,
platform_builders: Optional[Dict[str, str]] = None,
):
self._log = log
self._docker_registry = docker_registry
self._mp_registry_info = None
self._use_local_registry = use_local_registry
Expand Down Expand Up @@ -319,7 +316,7 @@ def _build_single_image(
tagged_names = [f"{name}:{tag}" for tag in tags]
builder = self._platform_builders.get(platform) if self._platform_builders else None
logger.debug(f"Building tagged images {tagged_names}")
self._log.write(f"Building image for platform {platform} with {builder or 'default'} builder\n")
print(f"Building image for platform {platform} with {builder or 'default'} builder")

if inject and isinstance(inject, dict):
self._build_with_inject(
Expand Down Expand Up @@ -455,26 +452,26 @@ def get_path(file):

if self._disable_multi_platform:
platforms = [self.get_single_platform_to_build(platforms)]
self._log.write(
print(
f"{line}\n"
f"Note: Disabling multi-platform build, "
"this will only build a single-platform image.\n"
f"image: {sanitized_name} platform:{platforms[0]}\n"
f"{line}\n"
f"{line}"
)
else:
self._log.write(
print(
f"{line}\n"
f"Note: Building multi-platform images can take a long time, please be patient.\n"
"If you are running this locally, you can speed this up by using the "
"'--disable-multi-platform' CLI flag "
"or set the 'disable-multi-platform' flag in the global config file.\n"
f"{line}\n"
f"{line}"
)

processes = []
self._log.write(
f'Starting builds for {len(platforms)} platforms in {"parallel" if do_multiprocessing else "sequence"}\n'
print(
f'Starting builds for {len(platforms)} platforms in {"parallel" if do_multiprocessing else "sequence"}'
)
for platform in platforms:
platform_image_name = f"{base_image_name}-{platform.replace('/', '-')}"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_multiplatform.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_start_local_registry_on_build():
registry_name = None
volume_name = None

with MultiplatformImageBuilder(log=MagicMock()) as mp:
with MultiplatformImageBuilder() as mp:
# Check that the registry is NOT running
assert mp._mp_registry_info is None
assert mp._local_registry_is_running is False
Expand Down

0 comments on commit 8b9909f

Please sign in to comment.