Skip to content

Commit

Permalink
Port engine/fs.py datatype() instances (#6103)
Browse files Browse the repository at this point in the history
This applies the principles learned in #6098 to the `engine/fs.py` classes like `DirectoryToMaterialize`. Refer to that PR for an explanation of why this is necessary.

Will fix the broken test in #6092. Part of #6062
  • Loading branch information
Eric-Arellano authored and Stu Hood committed Jul 14, 2018
1 parent f34fd98 commit a1eec56
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 36 deletions.
1 change: 1 addition & 0 deletions src/python/pants/backend/graph_info/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

python_library(
dependencies = [
'3rdparty/python:future',
'src/python/pants/backend/graph_info/subsystems',
'src/python/pants/base:build_environment',
'src/python/pants/base:cmd_line_spec_parser',
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/backend/graph_info/tasks/cloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import os

from future.utils import text_type

from pants.backend.graph_info.subsystems.cloc_binary import ClocBinary
from pants.base.workunit import WorkUnitLabel
from pants.engine.fs import FilesContent, PathGlobs, PathGlobsAndRoot
Expand Down Expand Up @@ -49,7 +51,7 @@ def console_output(self, targets):
list_file_snapshot = self.context._scheduler.capture_snapshots((
PathGlobsAndRoot(
PathGlobs(('input_files_list',)),
str(tmpdir),
text_type(tmpdir),
),
))[0]

Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/jvm/tasks/jvm_compile/javac/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
python_library(
dependencies = [
'3rdparty/python/twitter/commons:twitter.common.collections',
'3rdparty/python:future',
'3rdparty/python:six',
'src/python/pants/backend/jvm/subsystems:java',
'src/python/pants/backend/jvm/subsystems:jvm_platform',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

import logging
import os
from builtins import str

from future.utils import text_type

from pants.backend.jvm import argfile
from pants.backend.jvm.subsystems.java import Java
Expand Down Expand Up @@ -223,5 +226,5 @@ def _execute_hermetic_compile(self, cmd, ctx):
# Dump the output to the .pants.d directory where it's expected by downstream tasks.
classes_directory = ctx.classes_dir
self.context._scheduler.materialize_directories((
DirectoryToMaterialize(str(classes_directory), exec_result.output_directory_digest),
DirectoryToMaterialize(text_type(classes_directory), exec_result.output_directory_digest),
))
1 change: 1 addition & 0 deletions src/python/pants/binaries/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

python_library(
dependencies=[
'3rdparty/python:future',
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/base:build_environment',
'src/python/pants/base:exceptions',
Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/binaries/binary_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

import logging
import os
from builtins import str

from future.utils import text_type

from pants.binaries.binary_util import BinaryRequest, BinaryUtil
from pants.engine.fs import PathGlobs, PathGlobsAndRoot
Expand Down Expand Up @@ -198,7 +201,7 @@ def hackily_snapshot(self, context):
snapshot = context._scheduler.capture_snapshots((
PathGlobsAndRoot(
PathGlobs((script_relpath,)),
str(bootstrapdir),
text_type(bootstrapdir),
),
))[0]
return (script_relpath, snapshot)
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ python_library(
sources=['fs.py'],
dependencies=[
'3rdparty/python/twitter/commons:twitter.common.collections',
'3rdparty/python:future',
':rules',
':selectors',
'src/python/pants/base:project_tree',
Expand Down
10 changes: 6 additions & 4 deletions src/python/pants/engine/fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from __future__ import absolute_import, division, print_function, unicode_literals

from future.utils import text_type

from pants.base.project_tree import Dir, File
from pants.engine.rules import RootRule
from pants.option.global_options import GlobMatchErrorBehavior
Expand Down Expand Up @@ -61,11 +63,11 @@ def with_match_error_behavior(self, glob_match_error_behavior):
glob_match_error_behavior=glob_match_error_behavior)


class PathGlobsAndRoot(datatype([('path_globs', PathGlobs), ('root', str)])):
class PathGlobsAndRoot(datatype([('path_globs', PathGlobs), ('root', text_type)])):
pass


class DirectoryDigest(datatype([('fingerprint', str), ('serialized_bytes_length', int)])):
class DirectoryDigest(datatype([('fingerprint', text_type), ('serialized_bytes_length', int)])):
"""A DirectoryDigest is an opaque handle to a set of files known about by the engine.
The contents of files can be inspected by requesting a FilesContent for it.
Expand Down Expand Up @@ -114,7 +116,7 @@ def file_stats(self):
return [p.stat for p in self.files]


class DirectoryToMaterialize(datatype([('path', str), ('directory_digest', DirectoryDigest)])):
class DirectoryToMaterialize(datatype([('path', text_type), ('directory_digest', DirectoryDigest)])):
"""A request to materialize the contents of a directory digest at the provided path."""
pass

Expand All @@ -127,7 +129,7 @@ class DirectoryToMaterialize(datatype([('path', str), ('directory_digest', Direc


EMPTY_DIRECTORY_DIGEST = DirectoryDigest(
fingerprint=str(_EMPTY_FINGERPRINT),
fingerprint=text_type(_EMPTY_FINGERPRINT),
serialized_bytes_length=0
)

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/engine/native.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import cffi
import pkg_resources
import six
from future.utils import native
from future.utils import text_type

from pants.engine.selectors import Get, constraint_for
from pants.util.contextutil import temporary_dir
Expand Down Expand Up @@ -433,7 +433,7 @@ def extern_store_bytes(context_handle, bytes_ptr, bytes_len):
def extern_store_utf8(context_handle, utf8_ptr, utf8_len):
"""Given a context and UTF8 bytes, return a new Handle to represent the content."""
c = ffi.from_handle(context_handle)
return c.to_value(native(str(ffi.buffer(utf8_ptr, utf8_len))))
return c.to_value(text_type(ffi.buffer(utf8_ptr, utf8_len)))

@ffi.def_extern()
def extern_store_i64(context_handle, i64):
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/task/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

python_library(
dependencies = [
'3rdparty/python:future',
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/base:build_environment',
'src/python/pants/base:deprecated',
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/task/simple_codegen_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from abc import abstractmethod
from collections import OrderedDict

from future.utils import text_type
from twitter.common.collections import OrderedSet

from pants.base.build_environment import get_buildroot
Expand Down Expand Up @@ -293,7 +294,7 @@ def _capture_sources(self, targets_and_dirs):
to_capture.append(
PathGlobsAndRoot(
PathGlobs(buildroot_relative_globs, buildroot_relative_excludes),
str(get_buildroot()),
text_type(get_buildroot()),
)
)
results_dirs.append(results_dir_relpath)
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ impl Snapshot {
externs::unsafe_call(
&core.types.construct_directory_digest,
&[
externs::store_bytes(item.0.to_hex().as_bytes()),
externs::store_utf8(&item.0.to_hex()),
externs::store_i64(item.1 as i64),
],
)
Expand Down
3 changes: 3 additions & 0 deletions tests/python/pants_test/engine/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ python_tests(
sources=['test_fs.py'],
dependencies=[
':scheduler_test_base',
'3rdparty/python:future',
'src/python/pants/engine:fs',
'src/python/pants/engine:nodes',
'tests/python/pants_test:test_base',
Expand All @@ -87,6 +88,7 @@ python_tests(
name='isolated_process',
sources=['test_isolated_process.py'],
dependencies=[
'3rdparty/python:future',
':scheduler_test_base',
'src/python/pants/engine:fs',
'src/python/pants/engine:isolated_process',
Expand Down Expand Up @@ -136,6 +138,7 @@ python_tests(
name='build_files',
sources=['test_build_files.py'],
dependencies=[
'3rdparty/python:future',
':scheduler_test_base',
'src/python/pants/build_graph',
'src/python/pants/engine:build_files',
Expand Down
10 changes: 6 additions & 4 deletions tests/python/pants_test/engine/test_build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import os
import unittest

from future.utils import text_type

from pants.base.project_tree import Dir, File
from pants.base.specs import SiblingAddresses, SingleAddress, Specs
from pants.build_graph.address import Address
Expand All @@ -32,7 +34,7 @@ def test_empty(self):
"""Test that parsing an empty BUILD file results in an empty AddressFamily."""
address_mapper = AddressMapper(JsonParser(TestTable()))
af = run_rule(parse_address_family, address_mapper, Dir('/dev/null'), {
(Snapshot, PathGlobs): lambda _: Snapshot(DirectoryDigest(str("abc"), 10), (File('/dev/null/BUILD'),)),
(Snapshot, PathGlobs): lambda _: Snapshot(DirectoryDigest(text_type("abc"), 10), (File('/dev/null/BUILD'),)),
(FilesContent, DirectoryDigest): lambda _: FilesContent([FileContent('/dev/null/BUILD', '')]),
})
self.assertEquals(len(af.objects_by_name), 0)
Expand All @@ -43,7 +45,7 @@ def test_duplicated(self):
"""Test that matching the same Spec twice succeeds."""
address = SingleAddress('a', 'a')
address_mapper = AddressMapper(JsonParser(TestTable()))
snapshot = Snapshot(DirectoryDigest(str('xx'), 2), (Path('a/BUILD', File('a/BUILD')),))
snapshot = Snapshot(DirectoryDigest(text_type('xx'), 2), (Path('a/BUILD', File('a/BUILD')),))
address_family = AddressFamily('a', {'a': ('a/BUILD', 'this is an object!')})

bfas = run_rule(addresses_from_address_families, address_mapper, Specs([address, address]), {
Expand All @@ -58,7 +60,7 @@ def test_tag_filter(self):
"""Test that targets are filtered based on `tags`."""
spec = SiblingAddresses('root')
address_mapper = AddressMapper(JsonParser(TestTable()))
snapshot = Snapshot(DirectoryDigest(str('xx'), 2), (Path('root/BUILD', File('root/BUILD')),))
snapshot = Snapshot(DirectoryDigest(text_type('xx'), 2), (Path('root/BUILD', File('root/BUILD')),))
address_family = AddressFamily('root',
{'a': ('root/BUILD', TargetAdaptor()),
'b': ('root/BUILD', TargetAdaptor(tags={'integration'})),
Expand All @@ -79,7 +81,7 @@ def test_exclude_pattern(self):
"""Test that targets are filtered based on exclude patterns."""
spec = SiblingAddresses('root')
address_mapper = AddressMapper(JsonParser(TestTable()))
snapshot = Snapshot(DirectoryDigest(str('xx'), 2), (Path('root/BUILD', File('root/BUILD')),))
snapshot = Snapshot(DirectoryDigest(text_type('xx'), 2), (Path('root/BUILD', File('root/BUILD')),))
address_family = AddressFamily('root',
{'exclude_me': ('root/BUILD', TargetAdaptor()),
'not_me': ('root/BUILD', TargetAdaptor()),
Expand Down
39 changes: 21 additions & 18 deletions tests/python/pants_test/engine/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@
import os
import tarfile
import unittest
from builtins import str
from contextlib import contextmanager

from future.utils import text_type

from pants.base.project_tree import Dir, Link
from pants.engine.fs import (EMPTY_DIRECTORY_DIGEST, DirectoryDigest, DirectoryToMaterialize, FilesContent, PathGlobs,
PathGlobsAndRoot, Snapshot, create_fs_rules)
from pants.engine.fs import (EMPTY_DIRECTORY_DIGEST, DirectoryDigest, DirectoryToMaterialize,
FilesContent, PathGlobs, PathGlobsAndRoot, Snapshot, create_fs_rules)
from pants.util.contextutil import temporary_dir
from pants.util.meta import AbstractClass
from pants_test.engine.scheduler_test_base import SchedulerTestBase
Expand Down Expand Up @@ -272,9 +275,9 @@ def test_snapshot_from_outside_buildroot(self):
f.write("European Burmese")
scheduler = self.mk_scheduler(rules=create_fs_rules())
globs = PathGlobs(("*",), ())
snapshot = scheduler.capture_snapshots((PathGlobsAndRoot(globs, temp_dir),))[0]
snapshot = scheduler.capture_snapshots((PathGlobsAndRoot(globs, text_type(temp_dir)),))[0]
self.assert_snapshot_equals(snapshot, ["roland"], DirectoryDigest(
str("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
text_type("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
80
))

Expand All @@ -286,17 +289,17 @@ def test_multiple_snapshots_from_outside_buildroot(self):
f.write("I don't know")
scheduler = self.mk_scheduler(rules=create_fs_rules())
snapshots = scheduler.capture_snapshots((
PathGlobsAndRoot(PathGlobs(("roland",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("susannah",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("doesnotexist",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("roland",), ()), text_type(temp_dir)),
PathGlobsAndRoot(PathGlobs(("susannah",), ()), text_type(temp_dir)),
PathGlobsAndRoot(PathGlobs(("doesnotexist",), ()), text_type(temp_dir)),
))
self.assertEquals(3, len(snapshots))
self.assert_snapshot_equals(snapshots[0], ["roland"], DirectoryDigest(
str("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
text_type("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
80
))
self.assert_snapshot_equals(snapshots[1], ["susannah"], DirectoryDigest(
str("d3539cfc21eb4bab328ca9173144a8e932c515b1b9e26695454eeedbc5a95f6f"),
text_type("d3539cfc21eb4bab328ca9173144a8e932c515b1b9e26695454eeedbc5a95f6f"),
82
))
self.assert_snapshot_equals(snapshots[2], [], EMPTY_DIRECTORY_DIGEST)
Expand All @@ -306,7 +309,7 @@ def test_snapshot_from_outside_buildroot_failure(self):
scheduler = self.mk_scheduler(rules=create_fs_rules())
globs = PathGlobs(("*",), ())
with self.assertRaises(Exception) as cm:
scheduler.capture_snapshots((PathGlobsAndRoot(globs, str(os.path.join(temp_dir, "doesnotexist"))),))
scheduler.capture_snapshots((PathGlobsAndRoot(globs, text_type(os.path.join(temp_dir, "doesnotexist"))),))
self.assertIn("doesnotexist", str(cm.exception))

def assert_snapshot_equals(self, snapshot, files, directory_digest):
Expand All @@ -327,10 +330,10 @@ def test_merge_directories(self):
scheduler = self.mk_scheduler(rules=create_fs_rules())
(empty_snapshot, roland_snapshot, susannah_snapshot, both_snapshot) = (
scheduler.capture_snapshots((
PathGlobsAndRoot(PathGlobs(("doesnotmatch",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("roland",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("susannah",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("*",), ()), temp_dir),
PathGlobsAndRoot(PathGlobs(("doesnotmatch",), ()), text_type(temp_dir)),
PathGlobsAndRoot(PathGlobs(("roland",), ()), text_type(temp_dir)),
PathGlobsAndRoot(PathGlobs(("susannah",), ()), text_type(temp_dir)),
PathGlobsAndRoot(PathGlobs(("*",), ()), text_type(temp_dir)),
))
)

Expand Down Expand Up @@ -364,11 +367,11 @@ def test_materialize_directories(self):
with temporary_dir() as temp_dir:
dir_path = os.path.join(temp_dir, "containing_roland")
digest = DirectoryDigest(
str("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
text_type("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
80
)
scheduler = self.mk_scheduler(rules=create_fs_rules())
scheduler.materialize_directories((DirectoryToMaterialize(str(dir_path), digest),))
scheduler.materialize_directories((DirectoryToMaterialize(text_type(dir_path), digest),))

created_file = os.path.join(dir_path, "roland")
with open(created_file) as f:
Expand Down Expand Up @@ -426,8 +429,8 @@ def prime_store_with_roland_digest(self):
f.write("European Burmese")
scheduler = self.mk_scheduler(rules=create_fs_rules())
globs = PathGlobs(("*",), ())
snapshot = scheduler.capture_snapshots((PathGlobsAndRoot(globs, temp_dir),))[0]
snapshot = scheduler.capture_snapshots((PathGlobsAndRoot(globs, text_type(temp_dir)),))[0]
self.assert_snapshot_equals(snapshot, ["roland"], DirectoryDigest(
str("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
text_type("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
80
))
4 changes: 3 additions & 1 deletion tests/python/pants_test/engine/test_isolated_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import tarfile
import unittest

from future.utils import text_type

from pants.engine.fs import (EMPTY_DIRECTORY_DIGEST, DirectoryDigest, FileContent, FilesContent,
PathGlobs, Snapshot, create_fs_rules)
from pants.engine.isolated_process import (ExecuteProcessRequest, ExecuteProcessResult,
Expand Down Expand Up @@ -303,7 +305,7 @@ def test_write_file(self):
self.assertEquals(
execute_process_result.output_directory_digest,
DirectoryDigest(
fingerprint=str("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
fingerprint=text_type("63949aa823baf765eff07b946050d76ec0033144c785a94d3ebd82baa931cd16"),
serialized_bytes_length=80,
)
)
Expand Down
6 changes: 4 additions & 2 deletions tests/python/pants_test/source/test_payload_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from __future__ import absolute_import, division, print_function, unicode_literals

from future.utils import text_type

from pants.base.project_tree import File
from pants.engine.fs import DirectoryDigest, Path, Snapshot
from pants.source.payload_fields import SourcesField
Expand Down Expand Up @@ -80,9 +82,9 @@ def test_passes_eager_fileset_with_spec_through(self):
self.assertEqual(['foo/a.txt'], list(sf.source_paths))
self.assertEqual(['foo/foo/a.txt'], list(sf.relative_to_buildroot()))

digest = str('56001a7e48555f156420099a99da60a7a83acc90853046709341bf9f00a6f944')
digest = '56001a7e48555f156420099a99da60a7a83acc90853046709341bf9f00a6f944'
want_snapshot = Snapshot(
DirectoryDigest(digest, 77),
DirectoryDigest(text_type(digest), 77),
(Path('foo/foo/a.txt', stat=File('foo/foo/a.txt')),)
)

Expand Down

0 comments on commit a1eec56

Please sign in to comment.