Skip to content

Commit

Permalink
Find custom manifests in added directories.
Browse files Browse the repository at this point in the history
Previously a `Jar` would only detect custom jar manifests directly added
as strings or files. This change fixes `Jar` to also detect custom
manifests added from dirs and expands test coverage.

Testing Done:
Locally green:
```
./pants --tag=-integration test \
  tests/python/pants_test/backend/jvm/tasks::
```

CI went green here:
  http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3438/2/

Bugs closed: 3433, 3438

Reviewed at https://rbcommons.com/s/twitter/r/3886/
  • Loading branch information
jsirois committed May 15, 2016
1 parent 6387414 commit 0a49de6
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 10 deletions.
45 changes: 41 additions & 4 deletions src/python/pants/backend/jvm/tasks/jar_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
unicode_literals, with_statement)

import os
import shutil
import tempfile
from abc import abstractmethod
from contextlib import contextmanager
Expand All @@ -24,6 +25,7 @@
from pants.java.jar.manifest import Manifest
from pants.java.util import relativize_classpath
from pants.util.contextutil import temporary_dir
from pants.util.dirutil import safe_mkdtemp
from pants.util.meta import AbstractClass


Expand Down Expand Up @@ -51,6 +53,20 @@ def dest(self):
"""The destination path of the entry in the jar."""
return self._dest

def split_manifest(self):
"""Splits this entry into a jar non-manifest part and a manifest part.
Some entries represent a manifest, some do not, and others have both a manifest entry and
non-manifest entries; as such, callers must be prepared to handle ``None`` entries.
:returns: A tuple of (non-manifest Entry, manifest Entry).
:rtype: tuple of (:class:`Jar.Entry`, :class:`Jar.Entry`)
"""
if self.dest == Manifest.PATH:
return None, self
else:
return self, None

@abstractmethod
def materialize(self, scratch_dir):
"""Materialize this entry's source data into a filesystem path.
Expand All @@ -68,6 +84,26 @@ def __init__(self, src, dest=None):
super(Jar.FileSystemEntry, self).__init__(dest)
self._src = src

def split_manifest(self):
if not os.path.isdir(self._src):
return super(Jar.FileSystemEntry, self).split_manifest()

if self.dest and self.dest == os.path.commonprefix([self.dest, Manifest.PATH]):
manifest_relpath = os.path.relpath(Manifest.PATH, self.dest)
else:
manifest_relpath = Manifest.PATH

manifest_path = os.path.join(self._src, manifest_relpath)
if os.path.isfile(manifest_path):
manifest_entry = Jar.FileSystemEntry(manifest_path, dest=Manifest.PATH)

non_manifest_chroot = os.path.join(safe_mkdtemp(), 'chroot')
shutil.copytree(self._src, non_manifest_chroot)
os.unlink(os.path.join(non_manifest_chroot, manifest_relpath))
return Jar.FileSystemEntry(non_manifest_chroot), manifest_entry
else:
return self, None

def materialize(self, _):
return self._src

Expand Down Expand Up @@ -160,10 +196,11 @@ def writestr(self, path, contents):
self._add_entry(self.MemoryEntry(path, contents))

def _add_entry(self, entry):
if Manifest.PATH == entry.dest:
self._manifest_entry = entry
else:
self._entries.append(entry)
non_manifest, manifest = entry.split_manifest()
if manifest:
self._manifest_entry = manifest
if non_manifest:
self._entries.append(non_manifest)

def writejar(self, jar):
"""Schedules all entries from the given ``jar``'s to be added to this jar save for the manifest.
Expand Down
37 changes: 31 additions & 6 deletions tests/python/pants_test/backend/jvm/tasks/test_jar_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from pants.backend.jvm.tasks.jar_task import JarBuilderTask, JarTask
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.util.contextutil import open_zip, temporary_dir, temporary_file
from pants.util.dirutil import safe_mkdir, safe_mkdtemp, safe_rmtree
from pants.util.dirutil import safe_mkdir, safe_mkdtemp, safe_open, safe_rmtree
from pants_test.jvm.jar_task_test_base import JarTaskTestBase


Expand Down Expand Up @@ -127,8 +127,9 @@ def test_overwrite_writestr(self):
self.assert_listing(jar, 'README')
self.assertEquals('42', jar.read('README'))

def test_custom_manifest(self):
contents = b'Manifest-Version: 1.0\r\nCreated-By: test\r\n\r\n'
@contextmanager
def _test_custom_manifest(self):
manifest_contents = b'Manifest-Version: 1.0\r\nCreated-By: test\r\n\r\n'

with self.jarfile() as existing_jarfile:
with self.jar_task.open_jar(existing_jarfile, overwrite=True) as jar:
Expand All @@ -137,15 +138,39 @@ def test_custom_manifest(self):
with open_zip(existing_jarfile) as jar:
self.assert_listing(jar, 'README')
self.assertEquals('42', jar.read('README'))
self.assertNotEqual(contents, jar.read('META-INF/MANIFEST.MF'))
self.assertNotEqual(manifest_contents, jar.read('META-INF/MANIFEST.MF'))

with self.jar_task.open_jar(existing_jarfile, overwrite=False) as jar:
jar.writestr('META-INF/MANIFEST.MF', contents)
yield jar, manifest_contents

with open_zip(existing_jarfile) as jar:
self.assert_listing(jar, 'README')
self.assertEquals('42', jar.read('README'))
self.assertEquals(contents, jar.read('META-INF/MANIFEST.MF'))
self.assertEquals(manifest_contents, jar.read('META-INF/MANIFEST.MF'))

def test_custom_manifest_str(self):
with self._test_custom_manifest() as (jar, manifest_contents):
jar.writestr('META-INF/MANIFEST.MF', manifest_contents)

def test_custom_manifest_file(self):
with self._test_custom_manifest() as (jar, manifest_contents):
with safe_open(os.path.join(safe_mkdtemp(), 'any_source_file'), 'w') as fp:
fp.write(manifest_contents)
jar.write(fp.name, dest='META-INF/MANIFEST.MF')

def test_custom_manifest_dir(self):
with self._test_custom_manifest() as (jar, manifest_contents):
basedir = safe_mkdtemp()
with safe_open(os.path.join(basedir, 'META-INF/MANIFEST.MF'), 'w') as fp:
fp.write(manifest_contents)
jar.write(basedir)

def test_custom_manifest_dir_custom_dest(self):
with self._test_custom_manifest() as (jar, manifest_contents):
basedir = safe_mkdtemp()
with safe_open(os.path.join(basedir, 'MANIFEST.MF'), 'w') as fp:
fp.write(manifest_contents)
jar.write(basedir, dest='META-INF')

def test_classpath(self):
def manifest_content(classpath):
Expand Down

0 comments on commit 0a49de6

Please sign in to comment.