Skip to content

Commit

Permalink
Fix native tasks.
Browse files Browse the repository at this point in the history
Running a `./pants lint` across the whole repo revealed an issue
accessing a non-existant `self.linker.platform` attribute in
`LinkSharedLibraries`. Add basic unit tests to exercise task execution
with a full cache miss and then a full hit.
  • Loading branch information
jsirois committed Jul 18, 2018
1 parent d3f2005 commit 55a4f4e
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 12 deletions.
18 changes: 11 additions & 7 deletions src/python/pants/backend/native/tasks/link_shared_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ def _toolchain(self):
def linker(self):
return self._request_single(Linker, self._toolchain)

@memoized_property
def platform(self):
return Platform.create()

def _retrieve_single_product_at_target_base(self, product_mapping, target):
self.context.log.debug("product_mapping: {}".format(product_mapping))
self.context.log.debug("target: {}".format(target))
Expand Down Expand Up @@ -105,7 +109,7 @@ def execute(self):
def _retrieve_shared_lib_from_cache(self, vt):
native_artifact = vt.target.ctypes_native_library
path_to_cached_lib = os.path.join(
vt.results_dir, native_artifact.as_shared_lib(self.linker.platform))
vt.results_dir, native_artifact.as_shared_lib(self.platform))
if not os.path.isfile(path_to_cached_lib):
raise self.LinkSharedLibrariesError("The shared library at {} does not exist!"
.format(path_to_cached_lib))
Expand Down Expand Up @@ -137,8 +141,8 @@ def _make_link_request(self, vt, compiled_objects_product, native_target_deps_pr
'linux': lambda: ['-shared'],
}

def _get_shared_lib_cmdline_args(self, platform):
return platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS)
def _get_shared_lib_cmdline_args(self):
return self.platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS)

def _execute_link_request(self, link_request):
object_files = link_request.object_files
Expand All @@ -147,14 +151,14 @@ def _execute_link_request(self, link_request):
raise self.LinkSharedLibrariesError("No object files were provided in request {}!"
.format(link_request))

platform = Platform.create()
linker = link_request.linker
native_artifact = link_request.native_artifact
output_dir = link_request.output_dir
resulting_shared_lib_path = os.path.join(output_dir, native_artifact.as_shared_lib(platform))
resulting_shared_lib_path = os.path.join(output_dir,
native_artifact.as_shared_lib(self.platform))
# We are executing in the results_dir, so get absolute paths for everything.
cmd = ([linker.exe_filename] +
self._get_shared_lib_cmdline_args(platform) +
self._get_shared_lib_cmdline_args() +
['-o', os.path.abspath(resulting_shared_lib_path)] +
[os.path.abspath(obj) for obj in object_files])

Expand All @@ -166,7 +170,7 @@ def _execute_link_request(self, link_request):
cwd=output_dir,
stdout=workunit.output('stdout'),
stderr=workunit.output('stderr'),
env=linker.get_invocation_environment_dict(platform))
env=linker.get_invocation_environment_dict(self.platform))
except OSError as e:
workunit.set_outcome(WorkUnit.FAILURE)
raise self.LinkSharedLibrariesError(
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/native/tasks/native_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pants.backend.native.config.environment import Executable, Platform
from pants.backend.native.targets.native_library import NativeLibrary
from pants.backend.native.tasks.native_task import NativeTask
from pants.base.build_environment import get_buildroot
from pants.base.exceptions import TaskError
from pants.base.workunit import WorkUnit, WorkUnitLabel
from pants.build_graph.dependency_context import DependencyContext
Expand Down Expand Up @@ -134,7 +135,7 @@ def execute(self):
# This may be calculated many times for a target, so we memoize it.
@memoized_method
def _include_dirs_for_target(self, target):
return target.sources_relative_to_target_base().rel_root
return os.path.join(get_buildroot(), target.target_base)

class NativeSourcesByType(datatype(['rel_root', 'headers', 'sources'])): pass

Expand Down Expand Up @@ -168,7 +169,7 @@ def get_sources_headers_for_target(self, target):
"Conflicting filenames:\n{}"
.format(target.address.spec, target.alias(), '\n'.join(duplicate_filename_err_msgs)))

return [os.path.join(rel_root, src) for src in target_relative_sources]
return [os.path.join(get_buildroot(), rel_root, src) for src in target_relative_sources]

# FIXME(#5951): expand `Executable` to cover argv generation (where an `Executable` is subclassed
# to modify or extend the argument list, as declaratively as possible) to remove
Expand Down Expand Up @@ -266,7 +267,7 @@ def _compile(self, compile_request):
workunit.set_outcome(WorkUnit.FAILURE)
raise self.NativeCompileError(
"Error in '{section_name}' with command {cmd} for request {req}. Exit code was: {rc}."
.format(section_name=self.workunit_name, cmd=argv, req=compile_request, rc=rc))
.format(section_name=self.workunit_label, cmd=argv, req=compile_request, rc=rc))

def collect_cached_objects(self, versioned_target):
"""Scan `versioned_target`'s results directory and return the output files from that directory.
Expand Down
18 changes: 18 additions & 0 deletions tests/python/pants_test/backend/native/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
python_tests(
dependencies=[
':native_task_test_base',
'src/python/pants/backend/native/targets',
'src/python/pants/backend/native/tasks',
],
tags={'platform_specific_behavior'},
)

python_library(
name='native_task_test_base',
sources=['native_task_test_base.py'],
dependencies=[
'src/python/pants/backend/native',
'src/python/pants/backend/native/targets',
'tests/python/pants_test:task_test_base',
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from textwrap import dedent

from pants.backend.native import register
from pants.backend.native.targets.native_library import CppLibrary
from pants_test.task_test_base import TaskTestBase


class NativeTaskTestBase(TaskTestBase):

@classmethod
def rules(cls):
return super(NativeTaskTestBase, cls).rules() + register.rules()

def create_simple_cpp_library(self, **kwargs):
self.create_file('src/cpp/test/test.hpp', contents=dedent("""
#ifndef __TEST_HPP__
#define __TEST_HPP__
int test(int);
extern "C" int test_exported(int);
#endif
"""))
self.create_file('src/cpp/test/test.cpp', contents=dedent("""
#include "test.hpp"
int test(int x) {
return x / 137;
}
extern "C" int test_exported(int x) {
return test(x * 42);
}
"""))
return self.make_target(spec='src/cpp/test',
target_type=CppLibrary,
sources=['test.hpp', 'test.cpp'],
**kwargs)
22 changes: 22 additions & 0 deletions tests/python/pants_test/backend/native/tasks/test_cpp_compile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from pants.backend.native.tasks.cpp_compile import CppCompile
from pants_test.backend.native.tasks.native_task_test_base import NativeTaskTestBase


class CppCompileTest(NativeTaskTestBase):
@classmethod
def task_type(cls):
return CppCompile

def test_caching(self):
cpp = self.create_simple_cpp_library()
context = self.context(target_roots=[cpp])

cpp_compile = self.create_task(context)
cpp_compile.execute()
cpp_compile.execute()
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import os

from pants.backend.native.targets.native_artifact import NativeArtifact
from pants.backend.native.tasks.cpp_compile import CppCompile
from pants.backend.native.tasks.link_shared_libraries import LinkSharedLibraries
from pants_test.backend.native.tasks.native_task_test_base import NativeTaskTestBase


class LinkSharedLibrariesTest(NativeTaskTestBase):
@classmethod
def task_type(cls):
return LinkSharedLibraries

def test_caching(self):
cpp = self. create_simple_cpp_library(ctypes_native_library=NativeArtifact(lib_name='test'),)

cpp_compile_task_type = self.synthesize_task_subtype(CppCompile, 'cpp_compile_scope')
context = self.context(target_roots=[cpp], for_task_types=[cpp_compile_task_type])

cpp_compile = cpp_compile_task_type(context, os.path.join(self.pants_workdir, 'cpp_compile'))
cpp_compile.execute()

link_shared_libraries = self.create_task(context)
link_shared_libraries.execute()
link_shared_libraries.execute()
9 changes: 7 additions & 2 deletions tests/python/pants_test/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,16 @@ def alias_groups(cls):
"""
return BuildFileAliases(targets={'target': Target})

@classmethod
def rules(cls):
# Required for sources_for:
return [RootRule(SourcesField)]

@classmethod
def build_config(cls):
build_config = BuildConfiguration()
build_config.register_aliases(cls.alias_groups())
build_config.register_rules(cls.rules())
return build_config

def setUp(self):
Expand Down Expand Up @@ -368,8 +374,7 @@ def _init_engine(cls):
native=init_native(),
build_configuration=cls.build_config(),
build_ignore_patterns=None,
# Required for sources_for:
rules=[RootRule(SourcesField)],
rules=cls.build_config().rules(),
).new_session()
cls._scheduler = graph_session.scheduler_session
cls._build_graph, cls._address_mapper = graph_session.create_build_graph(
Expand Down

0 comments on commit 55a4f4e

Please sign in to comment.