Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Temporarily restore recursive behaviour for bundle filesets" #4625

Merged
merged 1 commit into from May 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/python/pants/backend/jvm/tasks/BUILD
Expand Up @@ -104,17 +104,16 @@ python_library(
name = 'bundle_create',
sources = ['bundle_create.py'],
dependencies = [
'3rdparty/python/twitter/commons:twitter.common.collections',
':classpath_products',
':jvm_binary_task',
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/backend/jvm/targets:jvm',
'src/python/pants/base:build_environment',
'src/python/pants/base:deprecated',
'src/python/pants/base:exceptions',
'src/python/pants/build_graph',
'src/python/pants/fs',
'src/python/pants/util:dirutil',
'src/python/pants/util:fileutil',
'src/python/pants/util:dirutil',
'src/python/pants/util:objects',
],
)
Expand Down
43 changes: 10 additions & 33 deletions src/python/pants/backend/jvm/tasks/bundle_create.py
Expand Up @@ -14,8 +14,7 @@
from pants.backend.jvm.tasks.classpath_products import ClasspathProducts
from pants.backend.jvm.tasks.jvm_binary_task import JvmBinaryTask
from pants.base.build_environment import get_buildroot
from pants.base.deprecated import deprecated_conditional
from pants.base.exceptions import TargetDefinitionException, TaskError
from pants.base.exceptions import TaskError
from pants.build_graph.target_scopes import Scopes
from pants.fs import archive
from pants.util.dirutil import absolute_symlink, safe_mkdir, safe_mkdir_for
Expand Down Expand Up @@ -209,39 +208,17 @@ def bundle(self, app, results_dir):
self.shade_jar(shading_rules=app.binary.shading_rules, jar_path=jar_path)

for bundle_counter, bundle in enumerate(app.bundles):
file_count = 0
dir_count = 0
fileset_empty = True
for path, relpath in bundle.filemap.items():
bundle_path = os.path.join(bundle_dir, relpath)
if os.path.isfile(path):
file_count += 1
elif os.path.isdir(path):
dir_count += 1
else:
continue
safe_mkdir(os.path.dirname(bundle_path))
os.symlink(path, bundle_path)

if file_count == 0 and dir_count == 0:
raise TargetDefinitionException(app.target,
'Bundle index {} of "bundles" field '
'does not match any files.'.format(bundle_counter))

if dir_count == 1 and file_count == 0:
# When this deprecation finishes, we should remove symlinking of directories into the
# bundle (which implicitly includes their contents), and instead create them using mkdir.
spec = os.path.relpath(bundle.filemap.keys()[0], get_buildroot())
deprecated_conditional(
lambda: True,
'1.5.0.dev0',
'default recursive inclusion of files in directory',
'''The bundle filespec `{spec}` corresponds to exactly one directory: if you'd like to '''
'''continue to recursively include directory contents in future versions, please switch '''
'''to a recursive glob like `{fixed_spec}`.'''.format(
spec=spec,
fixed_spec=os.path.join(spec, '**', '*'),
)
)
if os.path.exists(path):
safe_mkdir(os.path.dirname(bundle_path))
os.symlink(path, bundle_path)
fileset_empty = False

if fileset_empty:
raise TaskError('In target {}, bundle index {} of "bundles" field does not match any files.'.format(
app.address.spec, bundle_counter))

return bundle_dir

Expand Down
11 changes: 3 additions & 8 deletions src/python/pants/engine/legacy/graph.py
Expand Up @@ -344,9 +344,8 @@ def hydrate_target(target_adaptor, hydrated_fields):
tuple(target_adaptor.dependencies))


def _eager_fileset_with_spec(spec_path, filespec, snapshot, include_dirs=False):
fds = snapshot.path_stats if include_dirs else snapshot.files
files = tuple(fast_relpath(fd.path, spec_path) for fd in fds)
def _eager_fileset_with_spec(spec_path, filespec, snapshot):
files = tuple(fast_relpath(fd.path, spec_path) for fd in snapshot.files)

relpath_adjusted_filespec = FilesetRelPathWrapper.to_filespec(filespec['globs'], spec_path)
if filespec.has_key('exclude'):
Expand Down Expand Up @@ -382,13 +381,9 @@ def hydrate_bundles(bundles_field, snapshot_list):
for bundle, filespecs, snapshot in zipped:
spec_path = bundles_field.address.spec_path
kwargs = bundle.kwargs()
# NB: We `include_dirs=True` because bundle filesets frequently specify directories in order
# to trigger a (deprecated) default inclusion of their recursive contents. See the related
# deprecation in `pants.backend.jvm.tasks.bundle_create`.
kwargs['fileset'] = _eager_fileset_with_spec(getattr(bundle, 'rel_path', spec_path),
filespecs,
snapshot,
include_dirs=True)
snapshot)
bundles.append(BundleAdaptor(**kwargs))
return HydratedField('bundles', bundles)

Expand Down
11 changes: 1 addition & 10 deletions testprojects/src/java/org/pantsbuild/testproject/bundle/BUILD
Expand Up @@ -32,20 +32,11 @@ jvm_app(name='rel_path',
bundles=[
bundle(
rel_path='testprojects/src/java/org/pantsbuild/testproject/bundle/a',
fileset=['b/file1.txt'],
fileset=['b/file1.txt']
)
]
)

jvm_app(name='directory',
binary=':bundle-bin',
bundles=[
bundle(
fileset=['a/b'],
),
]
)

# The binary, the "runnable" part:

jvm_binary(name = 'bundle-bin',
Expand Down
3 changes: 1 addition & 2 deletions tests/python/pants_test/engine/legacy/BUILD
Expand Up @@ -29,8 +29,7 @@ python_tests(
name = 'bundle_integration',
sources = ['test_bundle_integration.py'],
dependencies = [
'src/python/pants/base:deprecated',
'tests/python/pants_test:int-test',
'tests/python/pants_test:int-test'
],
tags = {'integration'},
timeout = 240,
Expand Down
24 changes: 0 additions & 24 deletions tests/python/pants_test/engine/legacy/test_bundle_integration.py
Expand Up @@ -7,7 +7,6 @@

import os

from pants.base.deprecated import deprecated_conditional
from pants.util.contextutil import temporary_dir
from pants_test.pants_run_integration_test import PantsRunIntegrationTest, ensure_engine

Expand Down Expand Up @@ -56,29 +55,6 @@ def test_bundle_rel_path(self):
self.assertTrue(os.path.isfile(
'{}/{}.rel_path-bundle/b/file1.txt'.format(temp_distdir, self.TARGET_PATH.replace('/', '.'))))

@ensure_engine
def test_bundle_directory(self):
with temporary_dir() as temp_distdir:
with self.pants_results(
['-q',
'--pants-distdir={}'.format(temp_distdir),
'bundle',
'{}:directory'.format(self.TARGET_PATH)]) as pants_run:
self.assert_success(pants_run)

root = '{}/{}.directory-bundle/a/b'.format(temp_distdir, self.TARGET_PATH.replace('/', '.'))

self.assertTrue(os.path.isdir(root))

# NB: The behaviour of this test will change with the relevant deprecation
# in `pants.backend.jvm.tasks.bundle_create`.
deprecated_conditional(
lambda: os.path.isfile(os.path.join(root, 'file1.txt')),
'1.5.0.dev0',
'default recursive inclusion of files in directory',
'A non-recursive/literal glob should no longer include child paths.'
)

@ensure_engine
def test_bundle_resource_ordering(self):
"""Ensures that `resources=` ordering is respected."""
Expand Down