Skip to content

Commit

Permalink
Fix most issues with backend jvm
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric Arellano committed Aug 17, 2018
1 parent 62bc2a4 commit 9a1b3b7
Show file tree
Hide file tree
Showing 22 changed files with 69 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/argfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def create_argfile(f):
if delete and os.path.exists(argfile):
os.unlink(argfile)
else:
with temporary_file(cleanup=delete) as fp:
with temporary_file(cleanup=delete, binary_mode=False) as fp:
yield create_argfile(fp)
else:
yield args
4 changes: 2 additions & 2 deletions src/python/pants/backend/jvm/ivy_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,8 +539,8 @@ def __eq__(self, other):
# TODO(python3port): Return NotImplemented if other does not have attributes
def __lt__(self, other):
# We can't just re-use __repr__ or __str_ because we want to order rev last
return ((self.org, self.name, self.classifier, self.ext, self.rev) <
(other.org, other.name, other.classifier, other.ext, other.rev))
return ((self.org, self.name, self.classifier or '', self.ext, self.rev) <
(other.org, other.name, other.classifier or '', other.ext, other.rev))

def __hash__(self):
return hash(self._id)
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/jvm/targets/jvm_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ def rules(self):

def fingerprint(self):
hasher = sha1()
hasher.update(self.payload.fingerprint())
hasher.update(self.payload.fingerprint().encode('utf-8'))
for rule in self.rules:
hasher.update(rule.fingerprint())
hasher.update(rule.fingerprint().encode('utf-8'))
return hasher.hexdigest() if PY3 else hasher.hexdigest().decode('utf-8')

@property
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/coursier_resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ def compute_fingerprint(self, target):
hasher.update(target.payload.fingerprint().encode('utf-8'))

for conf in self._confs:
hasher.update(conf)
hasher.update(conf.encode('utf-8'))

for element in hash_elements_for_target:
hasher.update(element.encode('utf-8'))
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/coverage/cobertura.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def instrument(self, output_dir):
for pattern in self._exclude_classes:
args += ["--excludeClasses", pattern]

with temporary_file() as tmp_file:
with temporary_file(binary_mode=False) as tmp_file:
tmp_file.write("\n".join(unique_files))
tmp_file.flush()

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/jvm/tasks/jar_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ def exportable(tgt):

def entry_fingerprint(self, target, fingerprint_internal):
sha = hashlib.sha1()
sha.update(target.invalidation_hash())
sha.update(target.invalidation_hash().encode('utf-8'))

# TODO(Tejal Desai): pantsbuild/pants/65: Remove java_sources attribute for ScalaLibrary
if isinstance(target, ScalaLibrary):
Expand All @@ -892,7 +892,7 @@ def entry_fingerprint(self, target, fingerprint_internal):
internal_dependencies = sorted(target_internal_dependencies(target), key=lambda t: t.id)
for internal_target in internal_dependencies:
fingerprint = fingerprint_internal(internal_target)
sha.update(fingerprint)
sha.update(fingerprint.encode('utf-8'))

return sha.hexdigest() if PY3 else sha.hexdigest().decode('utf-8')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class PlatformFingerprintStrategy(FingerprintStrategy):
def compute_fingerprint(self, target):
hasher = sha1()
if hasattr(target, 'platform'):
hasher.update(str(tuple(target.platform)))
hasher.update(str(tuple(target.platform)).encode('utf-8'))
return hasher.hexdigest() if PY3 else hasher.hexdigest().decode('utf-8')

def __eq__(self, other):
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/build_graph/app_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class BundleField(tuple, PayloadField):
@staticmethod
def _hash_bundle(bundle):
hasher = sha1()
hasher.update(bundle.rel_path)
hasher.update(bundle.rel_path.encode('utf-8'))
for abs_path in sorted(bundle.filemap.keys()):
buildroot_relative_path = os.path.relpath(abs_path, get_buildroot()).encode('utf-8')
hasher.update(buildroot_relative_path)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/java/jar/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def _wrap(text):
chunk = fp.read(69)
if not chunk:
return
yield ' {}'.format(chunk)
yield b' ' + chunk

PATH = 'META-INF/MANIFEST.MF'

Expand Down
38 changes: 19 additions & 19 deletions src/python/pants/task/scm_publish_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import traceback
from abc import abstractmethod
from builtins import object, range
from functools import total_ordering

from pants.base.exceptions import TaskError
from pants.scm.scm import Scm
Expand All @@ -28,6 +29,7 @@ def version(self):
"""Returns the string representation of this Version."""


@total_ordering
class Namedver(Version):
"""A less restrictive versioning scheme that does not conflict with Semver
Expand Down Expand Up @@ -65,13 +67,14 @@ def version(self):
def __eq__(self, other):
return self._version == other._version

def __cmp__(self, other):
def __lt__(self, other):
raise ValueError("{0} is not comparable to {1}".format(self, other))

def __repr__(self):
return 'Namedver({0})'.format(self.version())


@total_ordering
class Semver(Version):
"""Semantic versioning. See http://semver.org"""

Expand Down Expand Up @@ -109,22 +112,18 @@ def version(self):
('{}-SNAPSHOT'.format(self.patch)) if self.snapshot else self.patch)

def __eq__(self, other):
return self.__cmp__(other) == 0
return (self.major, self.minor, self.patch, self.snapshot) == \
(other.major, other.minor, other.patch, other.snapshot)

def __cmp__(self, other):
def __lt__(self, other):
# import pdb; pdb.set_trace()
diff = self.major - other.major
if not diff:
diff = self.minor - other.minor
if not diff:
diff = self.patch - other.patch
if not diff:
if self.snapshot and not other.snapshot:
diff = 1
elif not self.snapshot and other.snapshot:
diff = -1
else:
diff = 0
return diff
if diff:
return self.major < other.major
diff = self.minor - other.minor
if diff:
return self.patch < other.patch
return self.snapshot < other.snapshot

def __repr__(self):
return 'Semver({})'.format(self.version())
Expand Down Expand Up @@ -231,13 +230,14 @@ def _push_and_tag_changes(self, tag_name, tag_message):

@staticmethod
def _push_with_retry(scm, log, attempts):
scm_exception = None
global_scm_exception = None
for attempt in range(attempts):
try:
log.debug("Trying scm push")
scm.push()
break # success
except Scm.RemoteException as scm_exception:
global_scm_exception = scm_exception
log.debug("Scm push failed, trying to refresh.")
# This might fail in the event that there is a real conflict, throwing
# a Scm.LocalException (in case of a rebase failure) or a Scm.RemoteException
Expand All @@ -249,9 +249,9 @@ def _push_with_retry(scm, log, attempts):
try:
scm.refresh(leave_clean=True)
except Scm.LocalException as local_exception:
exc = traceback.format_exc(scm_exception)
exc = traceback.format_exc()
log.debug("SCM exception while pushing: {}".format(exc))
raise local_exception

else:
raise scm_exception
else: # no more retries
raise global_scm_exception
1 change: 1 addition & 0 deletions tests/python/pants_test/backend/jvm/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ python_tests(
'src/python/pants/build_graph',
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'tests/python/pants_test/jvm:jar_task_test_base',
'tests/python/pants_test/jvm:jvm_tool_task_test_base',
]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def determine_version(self, path):
p = Popen(['file', path], stdout=PIPE, stderr=PIPE)
out, err = p.communicate()
self.assertEqual(0, p.returncode, 'Failed to run file on {}.'.format(path))
match = re.search(r'version (\d+[.]\d+)', out)
match = re.search(r'version (\d+[.]\d+)', out.decode('utf-8'))
self.assertTrue(match is not None, 'Could not determine version for {}'.format(path))
return version_map[match.group(1)]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ def test_agent_dependency(self):
jar = "dist/manifest-with-agent.jar"
with open_zip(jar, mode='r') as j:
with j.open("META-INF/MANIFEST.MF") as jar_entry:
entries = {tuple(line.strip().split(": ", 2)) for line in jar_entry.readlines() if line.strip()}
normalized_lines = (line.strip().decode('utf-8') for line in jar_entry.readlines() if line.strip())
entries = {tuple(line.split(": ", 2)) for line in normalized_lines}
self.assertIn(('Agent-Class', 'org.pantsbuild.testproject.manifest.Agent'), entries)

def test_deploy_excludes(self):
Expand Down Expand Up @@ -130,6 +131,8 @@ def run_java(self, java_args, expected_returncode=0, expected_output=None, cwd=N
stderr=subprocess.PIPE,
cwd=cwd)
stdout, stderr = process.communicate()
stdout = stdout.decode('utf-8')
stderr = stderr.decode('utf-8')

self.assertEqual(expected_returncode, process.returncode,
('Expected exit code {} from command `{}` but got {}:\n'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ def execute_tool(self, classpath, main, args=None):
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
out, err = process.communicate()
self.assertEqual(0, process.returncode)
self.assertEqual('', err.strip())
yield out
self.assertEqual('', err.strip().decode('utf-8'))
yield out.decode('utf-8')


class BootstrapJvmToolsShadingTest(BootstrapJvmToolsTestBase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def add_consolidated_bundle(self, context, tgt, files_dict):
entry_path = safe_mkdtemp(dir=target_dir)
classpath_dir = safe_mkdtemp(dir=target_dir)
for rel_path, content in files_dict.items():
safe_file_dump(os.path.join(entry_path, rel_path), content)
safe_file_dump(os.path.join(entry_path, rel_path), content, binary_mode=False)

# Create Jar to mimic consolidate classpath behavior.
jarpath = os.path.join(classpath_dir, 'output-0.jar')
Expand Down Expand Up @@ -71,12 +71,12 @@ def setUp(self):
JarDependency(org='org.gnu', name='gary', rev='4.0.0',
ext='tar.gz')])

safe_file_dump(os.path.join(self.build_root, 'resources/foo/file'), '// dummy content')
safe_file_dump(os.path.join(self.build_root, 'resources/foo/file'), '// dummy content', binary_mode=False)
self.resources_target = self.make_target('//resources:foo-resources', Resources,
sources=['foo/file'])

# This is so that payload fingerprint can be computed.
safe_file_dump(os.path.join(self.build_root, 'foo/Foo.java'), '// dummy content')
safe_file_dump(os.path.join(self.build_root, 'foo/Foo.java'), '// dummy content', binary_mode=False)
self.java_lib_target = self.make_target('//foo:foo-library', JavaLibrary, sources=['Foo.java'])

self.binary_target = self.make_target(spec='//foo:foo-binary',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ def setUp(self):
JarDependency(org='org.gnu', name='gary', rev='4.0.0',
ext='tar.gz')])

safe_file_dump(os.path.join(self.build_root, 'resources/foo/file'), '// dummy content')
safe_file_dump(os.path.join(self.build_root, 'resources/foo/file'), '// dummy content', binary_mode=False)
self.resources_target = self.make_target('//resources:foo-resources', Resources,
sources=['foo/file'])

# This is so that payload fingerprint can be computed.
safe_file_dump(os.path.join(self.build_root, 'foo/Foo.java'), '// dummy content')
safe_file_dump(os.path.join(self.build_root, 'foo/Foo.java'), '// dummy content', binary_mode=False)
self.java_lib_target = self.make_target('//foo:foo-library', JavaLibrary, sources=['Foo.java'])

self.binary_target = self.make_target(spec='//foo:foo-binary',
Expand Down Expand Up @@ -94,7 +94,7 @@ def test_remove_raw_deps(self):
)

# Confirm that we haven't destroyed deps.
expected_non_deps = set(['output-0.jar', 'Foo.class', 'foo.txt', 'file'])
expected_non_deps = {'output-0.jar', 'Foo.class', 'foo.txt', 'file'}
found = set(os.listdir(self.pants_workdir))
print(expected_non_deps - found)
self.assertTrue(expected_non_deps - found == expected_non_deps)
Expand All @@ -119,9 +119,9 @@ def test_consolidate_classpath(self):
)

# Confirm that we haven't destroyed deps.
expected_deps = set(['org.apache-baz-3.0.0-tests.jar',
'org.example-foo-1.0.0.jar',
'org.gnu-gary-4.0.0.tar.gz',
'org.pantsbuild-bar-2.0.0.zip'])
expected_deps = {'org.apache-baz-3.0.0-tests.jar',
'org.example-foo-1.0.0.jar',
'org.gnu-gary-4.0.0.tar.gz',
'org.pantsbuild-bar-2.0.0.zip'}
found = set(os.listdir(self.pants_workdir))
self.assertTrue(expected_deps - found == set())
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from builtins import str
from contextlib import contextmanager

from future.utils import PY3
from mock import MagicMock
from psutil.tests import safe_remove

Expand Down Expand Up @@ -130,7 +131,9 @@ def test_resolve_ignores_jars_with_rev_left_off(self):

self.resolve([lib])
self.assertEqual(
'Undefined revs for jars unsupported by Coursier. "jar(org=u\'com.google.guava\', name=u\'guava\', rev=None, classifier=None, ext=u\'jar\')"',
"Undefined revs for jars unsupported by Coursier. "
"\"jar(org={unicode_literal}'com.google.guava', name={unicode_literal}'guava', "
"rev=None, classifier=None, ext={unicode_literal}'jar')\"".format(unicode_literal='' if PY3 else 'u'),
str(cm.exception))

def test_resolve_multiple_artifacts(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_export_classpath_file_with_excludes(self):

with open_zip(manifest_jar_path) as synthetic_jar:
self.assertListEqual([Manifest.PATH], synthetic_jar.namelist())
oneline_classpath = synthetic_jar.read(Manifest.PATH).replace('\n', '').replace(' ', '')
oneline_classpath = synthetic_jar.read(Manifest.PATH).decode('utf-8').replace('\n', '').replace(' ', '')
self.assertNotIn('sbt-interface', oneline_classpath)
self.assertIn('foo', oneline_classpath)
self.assertIn('baz', oneline_classpath)
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def assert_jar_contents(self, context, product_type, target, *contents):
for content in content_set:
if not content.endswith('/'):
with closing(jar.open(content)) as fp:
self.assertEqual(os.path.basename(content), fp.read())
self.assertEqual(os.path.basename(content), fp.read().decode('utf-8'))

@ensure_cached(JarCreate)
def test_classfile_jar_contents(self):
Expand Down
Loading

0 comments on commit 9a1b3b7

Please sign in to comment.