Skip to content

Commit

Permalink
Add parent-build-id option to know what runs are inner runs and not t…
Browse files Browse the repository at this point in the history
…o run them with pantsd (pantsbuild#8724)

### Problem
Some pants runs call other pants runs (inner runs). This is not intended behaviour. It is also causing problems when running with pantsd. The problem is described here pantsbuild#7881 

Previous solution pantsbuild#7884 used concurrent flag to disable pantsd for inner runs. 
But the concurrent flag is a user-facing flag, so using it won't allow distinguishing pants runs where users disabled pantsd from inner runs. 

### Solution
Add a parent_build_id option (default None) and use it to propagate parent run_id to children pants runs (inner runs) by setting env var PANTS_PARENT_BUILD_ID.
  • Loading branch information
cattibrie authored and illicitonion committed Dec 10, 2019
1 parent d38fcf3 commit a88a0c4
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 11 deletions.
5 changes: 5 additions & 0 deletions src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import logging
import os
from contextlib import contextmanager

from pants.base.build_environment import get_buildroot
Expand Down Expand Up @@ -224,6 +225,10 @@ def __init__(self, build_root, options, options_bootstrapper, build_config, targ
def set_start_time(self, start_time):
# Launch RunTracker as early as possible (before .run() is called).
self._run_tracker = RunTracker.global_instance()

# Propagates parent_build_id to pants runs that may be called from this pants run.
os.environ['PANTS_PARENT_BUILD_ID'] = self._run_tracker.run_id

self._reporting = Reporting.global_instance()

self._run_start_time = start_time
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/bin/pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,17 @@ def _enable_rust_logging(self, global_bootstrap_options):
setup_logging_to_stderr(logging.getLogger(None), levelname)

def _should_run_with_pantsd(self, global_bootstrap_options):
# The parent_build_id option is set only for pants commands (inner runs)
# that were called by other pants command.
# Inner runs should never be run with pantsd.
# See https://github.com/pantsbuild/pants/issues/7881 for context.
is_inner_run = global_bootstrap_options.parent_build_id is not None

# If we want concurrent pants runs, we can't have pantsd enabled.
return global_bootstrap_options.enable_pantsd and \
not self.will_terminate_pantsd() and \
not global_bootstrap_options.concurrent
not global_bootstrap_options.concurrent and \
not is_inner_run

@staticmethod
def scrub_pythonpath():
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/goal/run_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ def initialize(self, all_options):
self.run_info = RunInfo(os.path.join(self.run_info_dir, 'info'))
self.run_info.add_basic_info(self.run_id, self._run_timestamp)
self.run_info.add_info('cmd_line', self._cmd_line)
if self.get_options().parent_build_id:
self.run_info.add_info('parent_build_id', self.get_options().parent_build_id)

# Create a 'latest' symlink, after we add_infos, so we're guaranteed that the file exists.
link_to_latest = os.path.join(os.path.dirname(self.run_info_dir), 'latest')
Expand Down
9 changes: 9 additions & 0 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,15 @@ def register_bootstrap_options(cls, register):
'start up all concurrent invocations (e.g. in other terminals) without pantsd. '
'Enabling this option requires parallel pants invocations to block on the first')

# Calling pants command (inner run) from other pants command is unusual behaviour,
# and most users should never set this flag.
# It is automatically set by pants when an inner run is detected.
# Currently, pants commands with this option set don't use pantsd,
# but this effect should not be relied upon.
# This option allows us to know who was the parent of pants inner runs for informational purposes.
register('--parent-build-id', advanced=True, default=None,
help='The build ID of the other pants run which spawned this one, if any.')

# Shutdown pantsd after the current run.
# This needs to be accessed at the same time as enable_pantsd,
# so we register it at bootstrap time.
Expand Down
5 changes: 0 additions & 5 deletions src/python/pants/pantsd/pailgun_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,6 @@ def handle(self):
self.logger.info('handling pailgun request: `{}`'.format(' '.join(arguments)))
self.logger.debug('pailgun request environment: %s', environment)

# We don't support parallel runs in pantsd, and therefore if this pailgun request
# triggers any pants command, we want it to not use pantsd at all.
# See https://github.com/pantsbuild/pants/issues/7881 for context.
environment["PANTS_CONCURRENT"] = "True"

# Execute the requested command with optional daemon-side profiling.
with maybe_profiled(environment.get('PANTSD_PROFILE')):
self._run_pants(self.request, arguments, environment)
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/testutil/pants_run_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,10 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None,
env.update(extra_env)
env.update(PYTHONPATH=os.pathsep.join(sys.path))

# Pants command that was called from the test shouldn't have a parent.
if 'PANTS_PARENT_BUILD_ID' in env:
del env['PANTS_PARENT_BUILD_ID']

# Don't overwrite the profile of this process in the called process.
# Instead, write the profile into a sibling file.
if env.get('PANTS_PROFILE'):
Expand Down
10 changes: 5 additions & 5 deletions testprojects/src/python/nested_runs/run_pants_with_pantsd.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import os
import pathlib
import subprocess
import sys


def main():
workdir = sys.argv[1]
config = pathlib.Path(workdir) / 'pants.ini'
config_path = pathlib.Path(workdir) / 'pants.ini'
config = [f'--pants-config-files={config_path}'] if os.path.isfile(config_path) else []

cmd = [
'./pants',
'./pants.pex',
'--no-pantsrc',
f'--pants-config-files={config}',
'--print-exception-stacktrace=True',
f'--pants-workdir={workdir}',
'goals'
]
] + config + ['goals']
print(f'Running pants with command {cmd}')
subprocess.run(cmd, check=True)

Expand Down
1 change: 1 addition & 0 deletions tests/python/pants_test/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ python_tests(
'src/python/pants/option',
'src/python/pants/testutil:int-test',
'testprojects:pants_plugins_directory',
'testprojects/src/python:nested_runs_directory',
],
tags = {'integration'},
)
28 changes: 28 additions & 0 deletions tests/python/pants_test/bin/test_runner_integration.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os
from pathlib import Path

from pants.base.build_environment import get_buildroot
Expand Down Expand Up @@ -37,3 +38,30 @@ def test_warning_filter(self):
})
self.assert_success(non_warning_run)
self.assertNotIn('test warning', non_warning_run.stderr_data)

def test_parent_build_id_set_only_for_pants_runs_called_by_other_pants_runs(self):
with self.temporary_workdir() as workdir:
command = [
'run',
'testprojects/src/python/nested_runs',
'--',
workdir,
]
result = self.run_pants_with_workdir(
command,
workdir,
)
self.assert_success(result)

run_tracker_dir = os.path.join(workdir, 'run-tracker')
self.assertTrue(os.path.isdir(run_tracker_dir), f'dir path {run_tracker_dir} does not exist!')
run_tracker_sub_dirs = (os.path.join(run_tracker_dir, dir_name) for dir_name in os.listdir(run_tracker_dir) if dir_name != 'latest')
for run_tracker_sub_dir in run_tracker_sub_dirs:
info_path = os.path.join(run_tracker_sub_dir, 'info')
self.assert_is_file(info_path)
with open(info_path, 'r') as info_f:
lines = dict(line.split(': ', 1) for line in info_f.readlines())
if 'goals' in lines['cmd_line']:
self.assertIn('parent_build_id', lines)
else:
self.assertNotIn('parent_build_id', lines)

0 comments on commit a88a0c4

Please sign in to comment.