Skip to content

Commit

Permalink
remove outer (pants_exe) lock and serialized cmd
Browse files Browse the repository at this point in the history
All two subcalsses of command (eg GoalRunner and SetupPy) do not override `serialized` to return `True` and thus do not require the outer lock.
Removing it simplifies pants_exe and reduces the number of places we depend on dirutil.Lock and shutil.

Testing Done:
https://travis-ci.org/pantsbuild/pants/builds/41782185

Bugs closed: 817

Reviewed at https://rbcommons.com/s/twitter/r/1388/
  • Loading branch information
dt committed Nov 23, 2014
1 parent 4914f5d commit d59ea8e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 44 deletions.
22 changes: 1 addition & 21 deletions src/python/pants/bin/pants_exe.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
import sys
import traceback

import psutil
from twitter.common.dirutil import Lock

from pants.backend.jvm.tasks.nailgun_task import NailgunTask
from pants.base.build_environment import get_buildroot, pants_version
from pants.base.build_file_address_mapper import BuildFileAddressMapper
Expand Down Expand Up @@ -89,11 +86,6 @@ def _parse_command(root_dir, args):
command, args = _synthesize_command(root_dir, args)
return Command.get_command(command), args


def _process_info(pid):
process = psutil.Process(pid)
return '%d (%s)' % (pid, ' '.join(process.cmdline))

def _run():
# Place the registration of the unhandled exception hook as early as possible in the code.
sys.excepthook = _unhandled_exception_hook
Expand Down Expand Up @@ -158,18 +150,8 @@ def _run():
address_mapper,
build_graph)
try:
if command.serialized():
def onwait(pid):
process = psutil.Process(pid)
print('Waiting on pants process %d (%s) to complete' %
(pid, ' '.join(process.cmdline)), file=sys.stderr)
return True
runfile = os.path.join(root_dir, '.pants.run')
lock = Lock.acquire(runfile, onwait=onwait)
else:
lock = Lock.unlocked()
try:
result = command.run(lock)
result = command.run()
if result:
run_tracker.set_root_outcome(WorkUnit.FAILURE)
_do_exit(result)
Expand All @@ -179,8 +161,6 @@ def onwait(pid):
except Exception:
run_tracker.set_root_outcome(WorkUnit.FAILURE)
raise
finally:
lock.release()
finally:
run_tracker.end()
# Must kill nailguns only after run_tracker.end() is called, because there may still
Expand Down
20 changes: 8 additions & 12 deletions src/python/pants/commands/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ def _register(cls):
if command_name:
Command._commands[command_name] = cls

@classmethod
def serialized(cls):
return False

def __init__(self,
run_tracker,
root_dir,
Expand Down Expand Up @@ -105,18 +101,18 @@ def setup_parser(self, parser, args):
def error(self, message=None, show_help=True):
"""Reports the error message, optionally followed by pants help, and then exits."""

def run(self, lock):
"""Subcommands that are serialized() should override if they need the ability to interact with
the global command lock.
The value returned should be an int, 0 indicating success and any other value indicating
def run(self):
"""The value returned should be an int, 0 indicating success and any other value indicating
failure."""
return self.execute()

def execute(self):
"""Subcommands that do not require serialization should override to perform the command action.
The value returned should be an int, 0 indicating success and any other value indicating
failure."""
raise NotImplementedError('Either run(lock) or execute() must be over-ridden.')
"""The value returned should be an int, 0 indicating success and any other value indicating
failure.
This method is duplicative with `run` but remains for legacy compat until Command is removed.
"""
raise NotImplementedError('Either run() or execute() must be over-ridden.')

def cleanup(self):
"""Called on SIGINT (e.g., when the user hits ctrl-c).
Expand Down
14 changes: 3 additions & 11 deletions src/python/pants/commands/goal_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,6 @@ class IntermixedArgumentsError(GoalError):
__command__ = 'goal'
output = None

# TODO(John Sirois): revisit wholesale locking when we move py support into pants new
@classmethod
def serialized(cls):
# Goal serialization is now handled in goal execution during group processing.
# The goal command doesn't need to hold the serialization lock; individual goals will
# acquire the lock if they need to be serialized.
return False

def __init__(self, *args, **kwargs):
self.targets = []
known_scopes = ['']
Expand Down Expand Up @@ -180,7 +172,7 @@ def setup_parser(self, parser, args):
args[:] = augmented_args
sys.stderr.write("(using pantsrc expansion: pants goal %s)\n" % ' '.join(augmented_args))

def run(self, lock):
def run(self):
# TODO(John Sirois): Consider moving to straight python logging. The divide between the
# context/work-unit logging and standard python logging doesn't buy us anything.

Expand Down Expand Up @@ -260,8 +252,8 @@ def targets_by_pattern(targets, patterns):
build_graph=self.build_graph,
build_file_parser=self.build_file_parser,
address_mapper=self.address_mapper,
spec_excludes=self.get_spec_excludes(),
lock=lock)
spec_excludes=self.get_spec_excludes()
)

unknown = []
for goal in self.goals:
Expand Down

0 comments on commit d59ea8e

Please sign in to comment.