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

use liblzma.dylib for xz on osx and add platform-specific testing to the rust osx shard #5936

Merged
merged 16 commits into from
Jun 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f607ea3
vary the xz shared lib filename depending on the current platform
cosmicexplorer Jun 8, 2018
3b4e07c
use the correct key 'mac' instead of 'darwin' into the platform
cosmicexplorer Jun 8, 2018
7a9f397
add `platform_specific_behavior` tag to python native code tests and …
cosmicexplorer Jun 8, 2018
b34dbb1
use self._liblzma_shared_lib_filename to make xz error messages more …
cosmicexplorer Jun 8, 2018
633a2b6
don't re-run platform-specific test sources twice in linux
cosmicexplorer Jun 8, 2018
0b493d5
add the bootstrap phase to the platform-specific python testing in th…
cosmicexplorer Jun 8, 2018
a1617a9
don't re-run platform-specific integration tests either on linux
cosmicexplorer Jun 8, 2018
4f78bbe
increase the timeout on platform-specific testing
cosmicexplorer Jun 8, 2018
34bd769
use the correct platform-specific library search path for the xz exe
cosmicexplorer Jun 9, 2018
62892ca
fix library path handling by using get_joined_path()
cosmicexplorer Jun 11, 2018
fe240da
remove library path manipulation thanks to pantsbuild/binaries#71
cosmicexplorer Jun 11, 2018
ff93b84
bump default_version and add comment
cosmicexplorer Jun 12, 2018
b43003b
set our member variable which was accidentally removed
cosmicexplorer Jun 12, 2018
bc5c0e0
remove pants-side xz execution wrapping
cosmicexplorer Jun 12, 2018
cead478
bump default xz version to match binary archive with wrapper script
cosmicexplorer Jun 12, 2018
1a38e92
sync up with pantsbuild/binaries#74
cosmicexplorer Jun 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -430,14 +430,15 @@ matrix:
osx_image: xcode8.3
language: generic
env:
- SHARD="Rust Tests OSX"
- SHARD="Rust + Platform-specific Tests OSX"
before_install:
- brew tap caskroom/cask && brew update && brew cask install osxfuse
before_script:
- ulimit -c unlimited
- ulimit -n 8192
script:
- ./build-support/bin/ci.sh -bcfjklmnprtx
# Platform-specific tests currently need a pants pex, so we bootstrap here (no -b) and set -z to run the platform-specific tests.
- ./build-support/bin/ci.sh -cfjklmnprtxz

deploy:
# See: https://docs.travis-ci.com/user/deployment/s3/
Expand Down
79 changes: 48 additions & 31 deletions build-support/bin/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,40 @@ cd ${REPO_ROOT}
source build-support/common.sh

function usage() {
echo "Runs commons tests for local or hosted CI."
echo
echo "Usage: $0 (-h|-fxbkmsrjlpuyncia)"
echo " -h print out this help message"
echo " -f skip python code formatting checks"
echo " -x skip bootstrap clean-all (assume bootstrapping from a"
echo " fresh clone)"
echo " -b skip bootstraping pants from local sources"
echo " -k skip bootstrapped pants self compile check"
echo " -m skip sanity checks of bootstrapped pants and repo BUILD"
echo " files"
echo " -r skip doc generation tests"
echo " -j skip core jvm tests"
echo " -l skip internal backends python tests"
echo " -p skip core python tests"
echo " -u SHARD_NUMBER/TOTAL_SHARDS"
echo " if running core python tests, divide them into"
echo " TOTAL_SHARDS shards and just run those in SHARD_NUMBER"
echo " to run only even tests: '-u 0/2', odd: '-u 1/2'"
echo " -n skip contrib python tests"
echo " -e skip rust tests"
echo " -y SHARD_NUMBER/TOTAL_SHARDS"
echo " if running contrib python tests, divide them into"
echo " TOTAL_SHARDS shards and just run those in SHARD_NUMBER"
echo " to run only even tests: '-u 0/2', odd: '-u 1/2'"
echo " -c skip pants integration tests (includes examples and testprojects)"
echo " -i SHARD_NUMBER/TOTAL_SHARDS"
echo " if running integration tests, divide them into"
echo " TOTAL_SHARDS shards and just run those in SHARD_NUMBER"
echo " to run only even tests: '-i 0/2', odd: '-i 1/2'"
cat <<EOF
Runs commons tests for local or hosted CI.

Usage: $0 (-h|-fxbkmsrjlpuyncia)
-h print out this help message
-f skip python code formatting checks
-x skip bootstrap clean-all (assume bootstrapping from a
fresh clone)
-b skip bootstraping pants from local sources
-k skip bootstrapped pants self compile check
-m skip sanity checks of bootstrapped pants and repo BUILD
files
-r skip doc generation tests
-j skip core jvm tests
-l skip internal backends python tests
-p skip core python tests
-u SHARD_NUMBER/TOTAL_SHARDS
if running core python tests, divide them into
TOTAL_SHARDS shards and just run those in SHARD_NUMBER
to run only even tests: '-u 0/2', odd: '-u 1/2'
-n skip contrib python tests
-e skip rust tests
-y SHARD_NUMBER/TOTAL_SHARDS
if running contrib python tests, divide them into
TOTAL_SHARDS shards and just run those in SHARD_NUMBER
to run only even tests: '-u 0/2', odd: '-u 1/2'
-c skip pants integration tests (includes examples and testprojects)
-i SHARD_NUMBER/TOTAL_SHARDS
if running integration tests, divide them into
TOTAL_SHARDS shards and just run those in SHARD_NUMBER
to run only even tests: '-i 0/2', odd: '-i 1/2'
-t skip lint
-z test platform-specific behavior
EOF
if (( $# > 0 )); then
die "$@"
else
Expand All @@ -57,7 +61,7 @@ python_unit_shard="0/1"
python_contrib_shard="0/1"
python_intg_shard="0/1"

while getopts "hfxbkmsrjlpeu:ny:ci:at" opt; do
while getopts "hfxbkmrjlpeu:ny:ci:tz" opt; do
case ${opt} in
h) usage ;;
f) skip_pre_commit_checks="true" ;;
Expand All @@ -76,6 +80,7 @@ while getopts "hfxbkmsrjlpeu:ny:ci:at" opt; do
c) skip_integration="true" ;;
i) python_intg_shard=${OPTARG} ;;
t) skip_lint="true" ;;
z) test_platform_specific_behavior="true" ;;
*) usage "Invalid option: -${OPTARG}" ;;
esac
done
Expand Down Expand Up @@ -224,6 +229,18 @@ if [[ "${skip_rust_tests:-false}" == "false" ]]; then
end_travis_section
fi

# NB: this only tests python tests right now -- the command needs to be edited if test targets in
# other languages are tagged with 'platform_specific_behavior' in the future.
if [[ "${test_platform_specific_behavior:-false}" == 'true' ]]; then
start_travis_section "Platform-specific tests" \
"Running platform-specific testing on platform: $(uname)"
(
./pants.pex ${PANTS_ARGS[@]} --tag='+platform_specific_behavior' test \
tests/python:: -- ${PYTEST_PASSTHRU_ARGS}
) || die "Pants platform-specific test failure"
end_travis_section
fi


if [[ "${skip_integration:-false}" == "false" ]]; then
if [[ "0/1" != "${python_intg_shard}" ]]; then
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/native/subsystems/llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ class LLVMReleaseUrlGenerator(BinaryToolUrlGenerator):

_ARCHIVE_BASE_FMT = 'clang+llvm-{version}-x86_64-{system_id}'

# TODO(cosmicexplorer): Give a more useful error message than KeyError if the host platform was
# not recognized (and make it easy for other BinaryTool subclasses to do this as well).
# TODO: Give a more useful error message than KeyError if the host platform was not recognized
# (and make it easy for other BinaryTool subclasses to do this as well).
_SYSTEM_ID = {
'darwin': 'apple-darwin',
'mac': 'apple-darwin',
'linux': 'linux-gnu-ubuntu-16.04',
}

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/pex_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ def get_local_platform():
# TODO(John Sirois): Kill some or all usages when https://github.com/pantsbuild/pex/issues/511
# is fixed.
current_platform = Platform.current()
return current_platform.platform
return current_platform.platform
15 changes: 7 additions & 8 deletions src/python/pants/binaries/binary_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ class BinaryToolBase(Subsystem):
def subsystem_dependencies(cls):
sub_deps = super(BinaryToolBase, cls).subsystem_dependencies() + (BinaryUtil.Factory,)

# TODO(cosmicexplorer): if we need to do more conditional subsystem dependencies, do it
# declaratively with a dict class field so that we only try to create or access it if we
# declared a dependency on it.
# TODO: if we need to do more conditional subsystem dependencies, do it declaratively with a
# dict class field so that we only try to create or access it if we declared a dependency on it.
if cls.archive_type == 'txz':
sub_deps = sub_deps + (XZ.scoped(cls),)

Expand All @@ -71,6 +70,9 @@ def _get_archiver(self):
if not self.archive_type:
return None

# This forces downloading and extracting the `XZ` archive if any BinaryTool with a 'txz'
# archive_type is used, but that's fine, because unless the cache is manually changed we won't
# do more work than necessary.
if self.archive_type == 'txz':
return self._xz.tar_xz_extractor

Expand Down Expand Up @@ -217,15 +219,12 @@ def path_entries(self):

class XZ(NativeTool):
options_scope = 'xz'
default_version = '5.2.4'
default_version = '5.2.4-3'
archive_type = 'tgz'

@memoized_property
def tar_xz_extractor(self):
return XZCompressedTarArchiver(self._executable_location(), self._lib_dir())
return XZCompressedTarArchiver(self._executable_location())

def _executable_location(self):
return os.path.join(self.select(), 'bin', 'xz')

def _lib_dir(self):
return os.path.join(self.select(), 'lib')
55 changes: 14 additions & 41 deletions src/python/pants/fs/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
from zipfile import ZIP_DEFLATED

from pants.util.contextutil import open_tar, open_zip, temporary_dir
from pants.util.dirutil import (is_executable, safe_concurrent_rename, safe_walk,
split_basename_and_dirname)
from pants.util.dirutil import is_executable, safe_concurrent_rename, safe_walk
from pants.util.meta import AbstractClass
from pants.util.process_handler import subprocess
from pants.util.strutil import ensure_text
Expand Down Expand Up @@ -96,16 +95,13 @@ class XZCompressedTarArchiver(TarArchiver):

Invokes an xz executable to decompress a .tar.xz into a tar stream, which is piped into the
extract() method.

NB: This class will raise an error if used to create an archive! This class can only currently be
used to extract from xz archives.
"""

class XZArchiverError(Exception): pass

def __init__(self, xz_binary_path, xz_library_path):
def __init__(self, xz_binary_path):

# TODO(cosmicexplorer): test these exceptions somewhere!
# TODO: test this exception somewhere!
if not is_executable(xz_binary_path):
raise self.XZArchiverError(
"The path {} does not name an existing executable file. An xz executable must be provided "
Expand All @@ -114,21 +110,6 @@ def __init__(self, xz_binary_path, xz_library_path):

self._xz_binary_path = xz_binary_path

if not os.path.isdir(xz_library_path):
raise self.XZArchiverError(
"The path {} does not name an existing directory. A directory containing liblzma.so must "
"be provided to decompress xz archives."
.format(xz_library_path))

lib_lzma_dylib = os.path.join(xz_library_path, 'liblzma.so')
if not os.path.isfile(lib_lzma_dylib):
raise self.XZArchiverError(
"The path {} names an existing directory, but it does not contain liblzma.so. A directory "
"containing liblzma.so must be provided to decompress xz archives."
.format(xz_library_path))

self._xz_library_path = xz_library_path

super(XZCompressedTarArchiver, self).__init__('r|', 'tar.xz')

@contextmanager
Expand All @@ -138,29 +119,21 @@ def _invoke_xz(self, xz_input_file):
This allows streaming the decompressed tar archive directly into a tar decompression stream,
which is significantly faster in practice than making a temporary file.
"""
(xz_bin_dir, xz_filename) = split_basename_and_dirname(self._xz_binary_path)

# TODO(cosmicexplorer): --threads=0 is supposed to use "the number of processor cores on the
# machine", but I see no more than 100% cpu used at any point. This seems like it could be a
# bug? If performance is an issue, investigate further.
cmd = [xz_filename, '--decompress', '--stdout', '--keep', '--threads=0', xz_input_file]
env = {
# Isolate the path so we know we're using our provided version of xz.
'PATH': xz_bin_dir,
# Only allow our xz's lib directory to resolve the liblzma.so dependency at runtime.
'LD_LIBRARY_PATH': self._xz_library_path,
}
# FIXME: --threads=0 is supposed to use "the number of processor cores on the machine", but I
# see no more than 100% cpu used at any point. This seems like it could be a bug? If performance
# is an issue, investigate further.
cmd = [self._xz_binary_path, '--decompress', '--stdout', '--keep', '--threads=0', xz_input_file]

try:
# Pipe stderr to our own stderr, but leave stdout open so we can yield it.
process = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stderr=sys.stderr,
env=env)
stderr=sys.stderr)
except OSError as e:
raise self.XZArchiverError(
"Error invoking xz with command {} and environment {} for input file {}: {}"
.format(cmd, env, xz_input_file, e),
"Error invoking xz with command {} for input file {}: {}"
.format(cmd, xz_input_file, e),
e)

# This is a file object.
Expand All @@ -169,15 +142,15 @@ def _invoke_xz(self, xz_input_file):
rc = process.wait()
if rc != 0:
raise self.XZArchiverError(
"Error decompressing xz input with command {} and environment {} for input file {}. "
"Exit code was: {}. "
.format(cmd, env, xz_input_file, rc))
"Error decompressing xz input with command {} for input file {}. Exit code was: {}. "
.format(cmd, xz_input_file, rc))

def _extract(self, path, outdir):
with self._invoke_xz(path) as xz_decompressed_tar_stream:
return super(XZCompressedTarArchiver, self)._extract(
xz_decompressed_tar_stream, outdir, mode=self.mode)

# TODO: implement this method, if we ever need it.
def create(self, *args, **kwargs):
"""
:raises: :class:`NotImplementedError`
Expand Down
29 changes: 27 additions & 2 deletions tests/python/pants_test/backend/python/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,33 @@ python_library(
]
)

python_native_code_test_files = [
# TODO: This test does not have any platform-specific testing right now, but that will be
# changed when #5815 is merged.
'test_build_local_python_distributions.py',
'test_python_distribution_integration.py',
]

python_tests(
name='python_native_code_testing',
sources=python_native_code_test_files,
dependencies=[
':python_task_test_base',
'src/python/pants/backend/native',
'src/python/pants/backend/python:plugin',
'src/python/pants/backend/python/targets',
'src/python/pants/backend/python/tasks',
'src/python/pants/util:contextutil',
'src/python/pants/util:process_handler',
'tests/python/pants_test:int-test',
'tests/python/pants_test/engine:scheduler_test_base',
],
tags={'platform_specific_behavior'},
timeout=2400,
)

python_tests(
sources=globs('test_*.py', exclude=[globs('*_integration.py')]),
sources=globs('test_*.py', exclude=([globs('*_integration.py')] + python_native_code_test_files)),
dependencies=[
':python_task_test_base',
'3rdparty/python/twitter/commons:twitter.common.collections',
Expand Down Expand Up @@ -55,7 +80,7 @@ python_tests(

python_tests(
name='integration',
sources=globs('*_integration.py'),
sources=globs('*_integration.py', exclude=python_native_code_test_files),
dependencies=[
'3rdparty/python:pex',
'src/python/pants/util:contextutil',
Expand Down