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

Stop using AddressWithOrigin for precise file arguments #10551

Merged
merged 4 commits into from Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 12 additions & 17 deletions src/python/pants/backend/codegen/protobuf/python/rules.py
Expand Up @@ -4,7 +4,7 @@
from pants.backend.codegen.protobuf.protoc import Protoc
from pants.backend.codegen.protobuf.target_types import ProtobufSources
from pants.backend.python.target_types import PythonSources
from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles
from pants.core.util_rules.determine_source_files import SourceFilesRequest
from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest
from pants.core.util_rules.strip_source_roots import StrippedSourceFiles
from pants.engine.addresses import Addresses
Expand Down Expand Up @@ -47,34 +47,29 @@ async def generate_python_from_protobuf(
# actually generate those dependencies; it only needs to look at their .proto files to work
# with imports.
transitive_targets = await Get(TransitiveTargets, Addresses([request.protocol_target.address]))
all_sources_request = Get(
SourceFiles,
AllSourceFilesRequest(
# NB: By stripping the source roots, we avoid having to set the value `--proto_path`
# for Protobuf imports to be discoverable.
all_stripped_sources_request = Get(
StrippedSourceFiles,
SourceFilesRequest(
(tgt.get(Sources) for tgt in transitive_targets.closure),
for_sources_types=(ProtobufSources,),
),
)
unstripped_target_sources_request = Get(
SourceFiles, AllSourceFilesRequest([request.protocol_target[ProtobufSources]]),
target_stripped_sources_request = Get(
StrippedSourceFiles, SourceFilesRequest([request.protocol_target[ProtobufSources]]),
)

(
downloaded_protoc_binary,
create_output_dir_result,
all_sources_unstripped,
target_sources_unstripped,
all_sources_stripped,
target_sources_stripped,
) = await MultiGet(
download_protoc_request,
create_output_dir_request,
all_sources_request,
unstripped_target_sources_request,
)

# NB: By stripping the source roots, we avoid having to set the value `--proto_path`
# for Protobuf imports to be discoverable.
all_sources_stripped, target_sources_stripped = await MultiGet(
Get(StrippedSourceFiles, SourceFiles, all_sources_unstripped),
Get(StrippedSourceFiles, SourceFiles, target_sources_unstripped),
all_stripped_sources_request,
target_stripped_sources_request,
)

input_digest = await Get(
Expand Down
Expand Up @@ -7,7 +7,7 @@

from pants.backend.python.target_types import PythonRequirementsField, PythonSources
from pants.base.specs import AddressSpecs, DescendantAddresses
from pants.core.util_rules.determine_source_files import AllSourceFilesRequest
from pants.core.util_rules.determine_source_files import SourceFilesRequest
from pants.core.util_rules.strip_source_roots import StrippedSourceFiles
from pants.engine.addresses import Address
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand Down Expand Up @@ -73,7 +73,7 @@ async def map_first_party_modules_to_addresses() -> FirstPartyModuleToAddressMap
for tgt in candidate_explicit_targets
)
stripped_sources_per_explicit_target = await MultiGet(
Get(StrippedSourceFiles, AllSourceFilesRequest([tgt[PythonSources]]))
Get(StrippedSourceFiles, SourceFilesRequest([tgt[PythonSources]]))
for tgt in candidate_explicit_targets
)

Expand Down
6 changes: 2 additions & 4 deletions src/python/pants/backend/python/dependency_inference/rules.py
Expand Up @@ -12,7 +12,7 @@
from pants.backend.python.rules import ancestor_files
from pants.backend.python.rules.ancestor_files import AncestorFiles, AncestorFilesRequest
from pants.backend.python.target_types import PythonSources, PythonTestsSources
from pants.core.util_rules.determine_source_files import AllSourceFilesRequest
from pants.core.util_rules.determine_source_files import SourceFilesRequest
from pants.core.util_rules.strip_source_roots import StrippedSourceFiles
from pants.engine.fs import Digest, DigestContents
from pants.engine.internals.graph import Owners, OwnersRequest
Expand Down Expand Up @@ -89,9 +89,7 @@ async def infer_python_dependencies(
if not python_inference.imports:
return InferredDependencies()

stripped_sources = await Get(
StrippedSourceFiles, AllSourceFilesRequest([request.sources_field])
)
stripped_sources = await Get(StrippedSourceFiles, SourceFilesRequest([request.sources_field]))
modules = tuple(
PythonModule.create_from_stripped_path(PurePath(fp))
for fp in stripped_sources.snapshot.files
Expand Down
36 changes: 11 additions & 25 deletions src/python/pants/backend/python/lint/bandit/rules.py
Expand Up @@ -22,11 +22,7 @@
LintSubsystem,
)
from pants.core.util_rules import determine_source_files, strip_source_roots
from pants.core.util_rules.determine_source_files import (
AllSourceFilesRequest,
SourceFiles,
SpecifiedSourceFilesRequest,
)
from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest
from pants.engine.fs import (
Digest,
DigestSubset,
Expand All @@ -37,14 +33,14 @@
)
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import FieldSetWithOrigin
from pants.engine.target import FieldSet
from pants.engine.unions import UnionRule
from pants.python.python_setup import PythonSetup
from pants.util.strutil import pluralize


@dataclass(frozen=True)
class BanditFieldSet(FieldSetWithOrigin):
class BanditFieldSet(FieldSet):
required_fields = (PythonSources,)

sources: PythonSources
Expand All @@ -62,15 +58,15 @@ class BanditPartition:


def generate_args(
*, specified_source_files: SourceFiles, bandit: Bandit, output_file: Optional[str]
*, source_files: SourceFiles, bandit: Bandit, output_file: Optional[str]
) -> Tuple[str, ...]:
args = []
if bandit.config is not None:
args.append(f"--config={bandit.config}")
if output_file:
args.append(f"--output={output_file}")
args.extend(bandit.args)
args.extend(specified_source_files.files)
args.extend(source_files.files)
return tuple(args)


Expand Down Expand Up @@ -100,26 +96,16 @@ async def bandit_lint_partition(
),
)

all_source_files_request = Get(
SourceFiles, AllSourceFilesRequest(field_set.sources for field_set in partition.field_sets)
)
specified_source_files_request = Get(
SourceFiles,
SpecifiedSourceFilesRequest(
(field_set.sources, field_set.origin) for field_set in partition.field_sets
),
source_files_request = Get(
SourceFiles, SourceFilesRequest(field_set.sources for field_set in partition.field_sets)
)

requirements_pex, config_digest, all_source_files, specified_source_files = await MultiGet(
requirements_pex_request,
config_digest_request,
all_source_files_request,
specified_source_files_request,
requirements_pex, config_digest, source_files = await MultiGet(
requirements_pex_request, config_digest_request, source_files_request
)

input_digest = await Get(
Digest,
MergeDigests((all_source_files.snapshot.digest, requirements_pex.digest, config_digest)),
Digest, MergeDigests((source_files.snapshot.digest, requirements_pex.digest, config_digest))
)

address_references = ", ".join(
Expand All @@ -129,7 +115,7 @@ async def bandit_lint_partition(
lint_subsystem.reports_dir / "bandit_report.txt" if lint_subsystem.reports_dir else None
)
args = generate_args(
specified_source_files=specified_source_files,
source_files=source_files,
bandit=bandit,
output_file=report_path.name if report_path else None,
)
Expand Down
Expand Up @@ -6,12 +6,11 @@
from pants.backend.python.lint.bandit.rules import BanditFieldSet, BanditRequest
from pants.backend.python.lint.bandit.rules import rules as bandit_rules
from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonLibrary
from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress
from pants.core.goals.lint import LintResults
from pants.engine.addresses import Address
from pants.engine.fs import DigestContents, FileContent
from pants.engine.rules import RootRule
from pants.engine.target import TargetWithOrigin
from pants.engine.target import Target
from pants.testutil.engine.util import Params
from pants.testutil.external_tool_test_base import ExternalToolTestBase
from pants.testutil.interpreter_selection_utils import skip_unless_python27_and_python3_present
Expand All @@ -29,26 +28,19 @@ class BanditIntegrationTest(ExternalToolTestBase):
def rules(cls):
return (*super().rules(), *bandit_rules(), RootRule(BanditRequest))

def make_target_with_origin(
self,
source_files: List[FileContent],
*,
interpreter_constraints: Optional[str] = None,
origin: Optional[OriginSpec] = None,
) -> TargetWithOrigin:
def make_target(
self, source_files: List[FileContent], *, interpreter_constraints: Optional[str] = None
) -> Target:
for source_file in source_files:
self.create_file(source_file.path, source_file.content.decode())
target = PythonLibrary(
return PythonLibrary(
{PythonInterpreterCompatibility.alias: interpreter_constraints},
address=Address.parse(":target"),
)
if origin is None:
origin = SingleAddress(directory="", name="target")
return TargetWithOrigin(target, origin)

def run_bandit(
self,
targets: List[TargetWithOrigin],
targets: List[Target],
*,
config: Optional[str] = None,
passthrough_args: Optional[str] = None,
Expand All @@ -74,23 +66,23 @@ def run_bandit(
)

def test_passing_source(self) -> None:
target = self.make_target_with_origin([self.good_source])
target = self.make_target([self.good_source])
result = self.run_bandit([target])
assert len(result) == 1
assert result[0].exit_code == 0
assert "No issues identified." in result[0].stdout.strip()
assert result[0].results_file is None

def test_failing_source(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_bandit([target])
assert len(result) == 1
assert result[0].exit_code == 1
assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout
assert result[0].results_file is None

def test_mixed_sources(self) -> None:
target = self.make_target_with_origin([self.good_source, self.bad_source])
target = self.make_target([self.good_source, self.bad_source])
result = self.run_bandit([target])
assert len(result) == 1
assert result[0].exit_code == 1
Expand All @@ -100,8 +92,8 @@ def test_mixed_sources(self) -> None:

def test_multiple_targets(self) -> None:
targets = [
self.make_target_with_origin([self.good_source]),
self.make_target_with_origin([self.bad_source]),
self.make_target([self.good_source]),
self.make_target([self.bad_source]),
]
result = self.run_bandit(targets)
assert len(result) == 1
Expand All @@ -110,28 +102,17 @@ def test_multiple_targets(self) -> None:
assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout
assert result[0].results_file is None

def test_precise_file_args(self) -> None:
target = self.make_target_with_origin(
[self.good_source, self.bad_source],
origin=FilesystemLiteralSpec(self.good_source.path),
)
result = self.run_bandit([target])
assert len(result) == 1
assert result[0].exit_code == 0
assert "No issues identified." in result[0].stdout
assert result[0].results_file is None

@skip_unless_python27_and_python3_present
def test_uses_correct_python_version(self) -> None:
py2_target = self.make_target_with_origin(
py2_target = self.make_target(
[self.py3_only_source], interpreter_constraints="CPython==2.7.*"
)
py2_result = self.run_bandit([py2_target])
assert len(py2_result) == 1
assert py2_result[0].exit_code == 0
assert "py3.py (syntax error while parsing AST from file)" in py2_result[0].stdout

py3_target = self.make_target_with_origin(
py3_target = self.make_target(
[self.py3_only_source], interpreter_constraints="CPython>=3.6"
)
py3_result = self.run_bandit([py3_target])
Expand All @@ -152,28 +133,28 @@ def test_uses_correct_python_version(self) -> None:
assert "No issues identified." in batched_py3_result.stdout

def test_respects_config_file(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_bandit([target], config="skips: ['B303']\n")
assert len(result) == 1
assert result[0].exit_code == 0
assert "No issues identified." in result[0].stdout.strip()
assert result[0].results_file is None

def test_respects_passthrough_args(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_bandit([target], passthrough_args="--skip B303")
assert len(result) == 1
assert result[0].exit_code == 0
assert "No issues identified." in result[0].stdout.strip()
assert result[0].results_file is None

def test_skip(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_bandit([target], skip=True)
assert not result

def test_3rdparty_plugin(self) -> None:
target = self.make_target_with_origin(
target = self.make_target(
[FileContent("bad.py", b"aws_key = 'JalrXUtnFEMI/K7MDENG/bPxRfiCYzEXAMPLEKEY'\n")],
# NB: `bandit-aws` does not currently work with Python 3.8. See
# https://github.com/pantsbuild/pants/issues/10545.
Expand All @@ -188,7 +169,7 @@ def test_3rdparty_plugin(self) -> None:
assert result[0].results_file is None

def test_output_file(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_bandit([target], additional_args=["--lint-reports-dir='.'"])
assert len(result) == 1
assert result[0].exit_code == 1
Expand Down