diff --git a/src/python/pants/commands/goal.py b/src/python/pants/commands/goal.py index 2f1dfafaa759..3c74336b5f6f 100644 --- a/src/python/pants/commands/goal.py +++ b/src/python/pants/commands/goal.py @@ -7,6 +7,7 @@ from contextlib import contextmanager import inspect +import logging import os import sys import traceback @@ -19,6 +20,7 @@ from pants.backend.core.tasks.console_task import ConsoleTask from pants.backend.core.tasks.task import Task +from pants.backend.jvm.tasks.nailgun_task import NailgunTask # XXX(pl) from pants.base.address import BuildFileAddress, parse_spec from pants.base.build_environment import get_buildroot from pants.base.build_file import BuildFile @@ -35,7 +37,6 @@ from pants.goal.help import print_help from pants.goal.initialize_reporting import update_reporting from pants.goal.option_helpers import add_global_options -from pants.backend.jvm.tasks.nailgun_task import NailgunTask # XXX(pl) StringIO = Compatibility.StringIO @@ -52,9 +53,34 @@ def parse_args(args): goals = OrderedSet() specs = OrderedSet() explicit_multi = False + logger = logging.getLogger(__name__) + has_double_dash = u'--' in args + goal_names = [phase.name for phase, goal in Phase.all()] + if not goal_names: + raise GoalError( + 'Arguments cannot be parsed before the list of goals from Phase.all() is populated.') def is_spec(spec): - return os.sep in spec or ':' in spec + if os.sep in spec or ':' in spec: + return True # Definitely not a goal. + if not (spec in goal_names): + return True # Definitely not a (known) goal. + if has_double_dash: + # This means that we're parsing the half of the expression before a --, so assume it's a + # goal without warning. + return False + # Here, it's possible we have a goal and target with the same name. For now, always give + # priority to the goal, but give a warning if they might have meant the target (if the BUILD + # file exists). + try: + BuildFile(get_buildroot(), spec) + msg = (' Command-line argument "{spec}" is ambiguous, and was assumed to be a goal.' + ' If this is incorrect, disambiguate it with the "--" argument to separate goals' + ' from targets.') + logger.warning(msg.format(spec=spec)) + except IOError: pass # Awesome, it's unambiguous. + return False + for i, arg in enumerate(args): if not arg.startswith('-'): diff --git a/tests/python/pants_test/commands/BUILD b/tests/python/pants_test/commands/BUILD index 84357d0ecd69..67dfcf223658 100644 --- a/tests/python/pants_test/commands/BUILD +++ b/tests/python/pants_test/commands/BUILD @@ -20,7 +20,9 @@ python_tests( name = 'test_goal', sources = [ 'test_goal.py' ], dependencies = [ + pants('src/python/pants/backend/jvm:plugin'), pants('src/python/pants/commands:goal'), + pants('src/python/pants/goal:phase'), pants('tests/python/pants_test:base_test'), ] ) diff --git a/tests/python/pants_test/commands/test_goal.py b/tests/python/pants_test/commands/test_goal.py index 6bc2a620d8ad..dc6136f55ca0 100644 --- a/tests/python/pants_test/commands/test_goal.py +++ b/tests/python/pants_test/commands/test_goal.py @@ -7,25 +7,69 @@ import unittest +from pants.backend.jvm.register import register_goals from pants.commands.goal import Goal, GoalError +from pants.goal import GoalError +from pants.goal.phase import Phase +from pants_test.base_test import BaseTest + + +class GoalTest(BaseTest): + + def assert_result(self, goals, specs, args): + g, s = Goal.parse_args(args) + self.assertEquals((goals, specs), (list(g), list(s))) + + def test_top_level_dir(self): + register_goals() + + self.create_dir('topleveldir') + self.create_file('topleveldir/BUILD', contents='') + self.assert_result(goals=[], specs=['topleveldir'], args=['topleveldir']) + + Phase.clear() + + def test_arg_ambiguity(self): + register_goals() + + self.create_dir('compile') + self.create_file('compile/BUILD', contents='') + self.assert_result(goals=['compile'], specs=[], args=['compile']) + # Only end up with one 'compile' here, because goals and specs are sets. + self.assert_result(goals=['compile'], specs=[], args=['compile', 'compile']) + self.assert_result(goals=['compile'], specs=['compile'], args=['compile', '--', 'compile']) + + error = '' + try: + self.assert_result(goals=['compile'], specs=[], args=['compile', 'spec:', '--', 'compile']) + except GoalError as e: + error = e.message + # We should have gotten an error for intermixing specs and goals behind a --. + self.assertTrue(error.find('intermix') > 0) + + self.assert_result(goals=['bundle', 'compile'], specs=['run'], + args=['bundle', 'compile', '--', 'run']) + + Phase.clear() # Clean up after ourselves. -class GoalTest(unittest.TestCase): def test_parse_args(self): - def assert_result(goals, specs, args): - g, s = Goal.parse_args(args) - self.assertEquals((goals, specs), (list(g), list(s))) + # Have to load in goals, because parse_args requires Phase.all() now. + # Fortunately, we only have to load in the goals we're actually using below, so the goals in the + # core register are sufficient. + register_goals() - assert_result(goals=[], specs=[], args=[]) - assert_result(goals=[], specs=[], args=['--']) - assert_result(goals=[], specs=[], args=['-v', '--help']) + self.assert_result(goals=[], specs=[], args=[]) + self.assert_result(goals=[], specs=[], args=['--']) + self.assert_result(goals=[], specs=[], args=['-v', '--help']) - assert_result(goals=['compile'], specs=[], args=['compile', '--log']) - assert_result(goals=['compile', 'test'], specs=[], args=['compile', 'test']) - assert_result(goals=['compile', 'test'], specs=[], args=['compile', '-v', 'test']) + self.assert_result(goals=['compile'], specs=[], args=['compile', '--log']) + self.assert_result(goals=['compile', 'test'], specs=[], args=['compile', 'test']) + self.assert_result(goals=['compile', 'test'], specs=[], args=['compile', '-v', 'test']) - assert_result(goals=[], specs=['resolve'], args=['--', 'resolve', '--ivy-open']) - assert_result(goals=['test'], specs=['resolve'], args=['test', '--', 'resolve', '--ivy-open']) + self.assert_result(goals=[], specs=['resolve'], args=['--', 'resolve', '--ivy-open']) + self.assert_result(goals=['test'], specs=['resolve'], args=['test', '--', 'resolve', + '--ivy-open']) try: Goal.parse_args(['test', 'lib:all', '--', 'resolve']) @@ -41,7 +85,10 @@ def assert_result(goals, specs, args): except GoalError: pass # expected - assert_result(goals=['test'], specs=['lib:all'], args=['lib:all', '-v', 'test']) - assert_result(goals=['test'], specs=['lib/'], args=['-v', 'test', 'lib/']) - assert_result(goals=['test'], specs=['lib/io:sound'], args=['test', '-v', 'lib/io:sound']) - assert_result(goals=['test'], specs=['lib:all'], args=['-h', 'test', '-v', 'lib:all', '-x']) + self.assert_result(goals=['test'], specs=['lib:all'], args=['lib:all', '-v', 'test']) + self.assert_result(goals=['test'], specs=['lib/'], args=['-v', 'test', 'lib/']) + self.assert_result(goals=['test'], specs=['lib/io:sound'], args=['test', '-v', 'lib/io:sound']) + self.assert_result(goals=['test'], specs=['lib:all'], args=['-h', 'test', '-v', 'lib:all', + '-x']) + + Phase.clear() # Clean up after ourselves.