Skip to content

Commit

Permalink
Fix built-in macros for the mutable ParseContext (pantsbuild#4583)
Browse files Browse the repository at this point in the history
### Problem

The `v2` engine mutates the `ParseContext` in order to avoid re-creating all macros and context object factories for every build file path. But some built in macros without adequate integration test coverage (or which are instantiated via different codepaths in `v2`, as is the case with file globs and bundles) escaped being updated.

### Solution

Update the remaining macros that attempted to inspect the `parse_context.rel_path` during construction rather than during usage. Add tests/examples to cover two of the broken macros.
  • Loading branch information
Stu Hood authored May 13, 2017
1 parent df19f57 commit 7b8500e
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 11 deletions.
7 changes: 4 additions & 3 deletions src/python/pants/backend/jvm/targets/jvm_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class Bundle(object):
"""

def __init__(self, parse_context):
self._rel_path = parse_context.rel_path
self._parse_context = parse_context

def __call__(self, rel_path=None, mapper=None, relative_to=None, fileset=None):
"""
Expand All @@ -128,9 +128,10 @@ def __call__(self, rel_path=None, mapper=None, relative_to=None, fileset=None):
filenames, or a Fileset object (e.g. globs()).
E.g., ``relative_to='common'`` removes that prefix from all files in the application bundle.
"""

if fileset is None:
raise ValueError("In {}:\n Bare bundle() declarations without a `fileset=` parameter "
"are no longer supported.".format(self._rel_path))
"are no longer supported.".format(self._parse_context.rel_path))

if mapper and relative_to:
raise ValueError("Must specify exactly one of 'mapper' or 'relative_to'")
Expand All @@ -147,7 +148,7 @@ def __call__(self, rel_path=None, mapper=None, relative_to=None, fileset=None):
else:
fileset = assert_list(fileset, key_arg='fileset')

real_rel_path = rel_path or self._rel_path
real_rel_path = rel_path or self._parse_context.rel_path

if relative_to:
base = os.path.join(get_buildroot(), real_rel_path, relative_to)
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/build_graph/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@

class BuildFilePath(object):
def __init__(self, parse_context):
self.rel_path = parse_context.rel_path
self._parse_context = parse_context

def __call__(self):
"""
:returns: The absolute path of this BUILD file.
"""
return os.path.join(get_buildroot(), self.rel_path)
return os.path.join(get_buildroot(), self._parse_context.rel_path)


def build_file_aliases():
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/java/jar/jar_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __init__(self, parse_context):
"""
:param parse_context: The BUILD file parse context.
"""
self._rel_path = parse_context.rel_path
self._parse_context = parse_context

def __call__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs=None,
classifier=None, mutable=None, intransitive=False, excludes=None):
Expand All @@ -66,7 +66,7 @@ def __call__(self, org, name, rev=None, force=False, ext=None, url=None, apidocs
:type excludes: list of :class:`pants.backend.jvm.targets.exclude.Exclude`
"""
return JarDependency(org, name, rev, force, ext, url, apidocs, classifier, mutable, intransitive,
excludes, self._rel_path)
excludes, self._parse_context.rel_path)


class JarDependency(datatype('JarDependency', [
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/source/wrapped_globs.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ def __init__(self, parse_context):
"""
:param parse_context: The BUILD file parse context.
"""
self._rel_path = parse_context.rel_path
self._parse_context = parse_context

def __call__(self, *patterns, **kwargs):
return self.create_fileset_with_spec(self._rel_path, *patterns, **kwargs)
return self.create_fileset_with_spec(self._parse_context.rel_path, *patterns, **kwargs)

@classmethod
def create_fileset_with_spec(cls, rel_path, *patterns, **kwargs):
Expand Down
11 changes: 11 additions & 0 deletions testprojects/3rdparty/org/pantsbuild/testprojects/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
jar_library(
jars = [
# An example of a relative jar URL referring to the current directory.
jar(
org='org.pantsbuild.testprojects',
name='relative-jar-url-example',
rev='0.0.1',
url='file:relative-jar-url-example.jar',
),
]
)
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
target(
dependencies=[
'testprojects/src/resources/org/pantsbuild/testproject/ordering'
],
description='''This target exists at path {}.'''.format(buildfile_path()),
)
9 changes: 9 additions & 0 deletions tests/python/pants_test/backend/jvm/targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ python_tests(
]
)

python_tests(
name = 'jar_dependency_integration',
sources = ['test_jar_dependency_integration.py'],
dependencies = [
'tests/python/pants_test:int-test',
],
tags = {'integration'},
)

python_tests(
name='jar_library',
sources=['test_jar_library.py'],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# coding=utf-8
# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import os
import unittest

from pants_test.pants_run_integration_test import PantsRunIntegrationTest


class JarDependencyIntegrationTest(PantsRunIntegrationTest):

def test_resolve_relative(self):
pants_run = self.run_pants(['resolve', 'testprojects/3rdparty/org/pantsbuild/testprojects'])
self.assert_success(pants_run)
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@

class BuildGraphIntegrationTest(PantsRunIntegrationTest):

@unittest.skip('https://github.com/pantsbuild/pants/issues/4358')
def test_cycle(self):
prefix = 'testprojects/src/java/org/pantsbuild/testproject'
with self.file_renamed(os.path.join(prefix, 'cycle1'), 'TEST_BUILD', 'BUILD'):
with self.file_renamed(os.path.join(prefix, 'cycle2'), 'TEST_BUILD', 'BUILD'):
pants_run = self.run_pants(['compile', os.path.join(prefix, 'cycle1')])
self.assert_failure(pants_run)
self.assertIn('contained a cycle', pants_run.stderr_data)
self.assertIn('Cycle detected', pants_run.stderr_data)

0 comments on commit 7b8500e

Please sign in to comment.