From d7f345d42d44bd8c80f6ab97aa8dfed27fd439ed Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Wed, 25 Aug 2021 13:24:12 +0200 Subject: [PATCH 1/9] digest subset should not short-circuit when there are ignores involved. Signed-off-by: Andreas Stenius --- src/rust/engine/fs/src/lib.rs | 14 +++++++++++++ src/rust/engine/fs/store/src/snapshot_ops.rs | 10 ++++++---- .../engine/fs/store/src/snapshot_ops_tests.rs | 20 ++++++++++++++++++- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/rust/engine/fs/src/lib.rs b/src/rust/engine/fs/src/lib.rs index a09dcae45c9..4d33409d939 100644 --- a/src/rust/engine/fs/src/lib.rs +++ b/src/rust/engine/fs/src/lib.rs @@ -310,6 +310,20 @@ impl GitignoreStyleExcludes { ::ignore::Match::Ignore(_) => true, } } + + pub fn has_ignored_paths(&self, path: &Path) -> bool { + match path.to_str() { + None => return false, + Some(s) => { + for pattern in self.exclude_patterns().iter() { + if pattern.starts_with(s) { + return true; + } + } + return false; + } + } + } } #[derive(Debug, Clone, Eq, Hash, PartialEq)] diff --git a/src/rust/engine/fs/store/src/snapshot_ops.rs b/src/rust/engine/fs/store/src/snapshot_ops.rs index fd0b918017d..f6a859442b6 100644 --- a/src/rust/engine/fs/store/src/snapshot_ops.rs +++ b/src/rust/engine/fs/store/src/snapshot_ops.rs @@ -425,17 +425,19 @@ impl IntermediateGlobbedFilesAndDirectories { if (*wildcard == *DOUBLE_STAR_GLOB) || (*wildcard == *SINGLE_STAR_GLOB) { // Here we short-circuit all cases which would swallow up a directory without // subsetting it or needing to perform any further recursive work. + let has_ignores = exclude.has_ignored_paths(&directory_path); + let short_circuit: bool = match &remainder[..] { - [] => true, + [] => !has_ignores, // NB: Very often, /**/* is seen ending zsh-style globs, which means the same as // ending in /**. Because we want to *avoid* recursing and just use the subdirectory // as-is for /**/* and /**, we `continue` here in both cases. - [single_glob] if *single_glob == *SINGLE_STAR_GLOB => true, - [double_glob] if *double_glob == *DOUBLE_STAR_GLOB => true, + [single_glob] if *single_glob == *SINGLE_STAR_GLOB => !has_ignores, + [double_glob] if *double_glob == *DOUBLE_STAR_GLOB => !has_ignores, [double_glob, single_glob] if *double_glob == *DOUBLE_STAR_GLOB && *single_glob == *SINGLE_STAR_GLOB => { - true + !has_ignores } _ => false, }; diff --git a/src/rust/engine/fs/store/src/snapshot_ops_tests.rs b/src/rust/engine/fs/store/src/snapshot_ops_tests.rs index f63575f8f3e..129523c3acf 100644 --- a/src/rust/engine/fs/store/src/snapshot_ops_tests.rs +++ b/src/rust/engine/fs/store/src/snapshot_ops_tests.rs @@ -95,7 +95,7 @@ async fn subset_single_files() { async fn subset_recursive_wildcard() { let (store, tempdir, posix_fs, digester) = setup(); - let (merged_digest, _, _) = get_duplicate_rolands( + let (merged_digest, snapshot1, _) = get_duplicate_rolands( store.clone(), store.clone(), tempdir.path(), @@ -120,6 +120,24 @@ async fn subset_recursive_wildcard() { .await .unwrap(); assert_eq!(merged_digest, subset_roland2); + + // ** should not include explicitly excluded files + let subset_params3 = make_subset_params(&["!subdir/roland2", "subdir/**"]); + let subset_roland3 = store + .clone() + .subset(merged_digest, subset_params3) + .await + .unwrap(); + assert_eq!(subset_roland3, snapshot1.digest); + + // ** should not include explicitly excluded files + let subset_params4 = make_subset_params(&["!subdir/roland2", "**"]); + let subset_roland4 = store + .clone() + .subset(merged_digest, subset_params4) + .await + .unwrap(); + assert_eq!(subset_roland4, snapshot1.digest); } #[derive(Clone)] From 42684592c20ece701b485e9bcc1548cdc6d6f28c Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Fri, 3 Sep 2021 16:07:20 +0200 Subject: [PATCH 2/9] fixes after review feedback. Signed-off-by: Andreas Stenius # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/rust/engine/fs/src/lib.rs | 17 +++++++++++---- src/rust/engine/fs/store/src/snapshot_ops.rs | 22 +++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/rust/engine/fs/src/lib.rs b/src/rust/engine/fs/src/lib.rs index 4d33409d939..35ce501b175 100644 --- a/src/rust/engine/fs/src/lib.rs +++ b/src/rust/engine/fs/src/lib.rs @@ -311,16 +311,25 @@ impl GitignoreStyleExcludes { } } - pub fn has_ignored_paths(&self, path: &Path) -> bool { + /// + /// Find out if a path has any ignore patterns for files/paths in its tree. + /// + /// Used by the IntermediateGlobbedFilesAndDirectories in snapshot_ops.rs, + /// to check if it mayoptimize the snapshot subset operation on this tree, + /// or need to check for excluded files/directories. + /// + pub fn maybe_is_parent_of_ignored_path(&self, path: &Path) -> bool { match path.to_str() { - None => return false, + None => true, Some(s) => { for pattern in self.exclude_patterns().iter() { - if pattern.starts_with(s) { + if pattern.starts_with(s) || s.starts_with(pattern) { + // In case the pattern is shorter than path, we are inside a ignored tree, so both + // parent and child of ignored paths. return true; } } - return false; + false } } } diff --git a/src/rust/engine/fs/store/src/snapshot_ops.rs b/src/rust/engine/fs/store/src/snapshot_ops.rs index f6a859442b6..331bcc5ab1c 100644 --- a/src/rust/engine/fs/store/src/snapshot_ops.rs +++ b/src/rust/engine/fs/store/src/snapshot_ops.rs @@ -406,6 +406,12 @@ impl IntermediateGlobbedFilesAndDirectories { RestrictedPathGlob::Wildcard { wildcard } => { // NB: We interpret globs such that the *only* way to have a glob match the contents of // a whole directory is to end in '/**' or '/**/*'. + + if exclude.maybe_is_parent_of_ignored_path(&directory_path) { + // leave this directory in todo_directories, so we process ignore patterns correctly.. + continue; + } + if *wildcard == *DOUBLE_STAR_GLOB { globbed_directories.insert(directory_path, directory_node.clone()); } else if !globbed_directories.contains_key(&directory_path) { @@ -425,23 +431,23 @@ impl IntermediateGlobbedFilesAndDirectories { if (*wildcard == *DOUBLE_STAR_GLOB) || (*wildcard == *SINGLE_STAR_GLOB) { // Here we short-circuit all cases which would swallow up a directory without // subsetting it or needing to perform any further recursive work. - let has_ignores = exclude.has_ignored_paths(&directory_path); - - let short_circuit: bool = match &remainder[..] { - [] => !has_ignores, + let is_short_circuit_pattern: bool = match &remainder[..] { + [] => true, // NB: Very often, /**/* is seen ending zsh-style globs, which means the same as // ending in /**. Because we want to *avoid* recursing and just use the subdirectory // as-is for /**/* and /**, we `continue` here in both cases. - [single_glob] if *single_glob == *SINGLE_STAR_GLOB => !has_ignores, - [double_glob] if *double_glob == *DOUBLE_STAR_GLOB => !has_ignores, + [single_glob] if *single_glob == *SINGLE_STAR_GLOB => true, + [double_glob] if *double_glob == *DOUBLE_STAR_GLOB => true, [double_glob, single_glob] if *double_glob == *DOUBLE_STAR_GLOB && *single_glob == *SINGLE_STAR_GLOB => { - !has_ignores + true } _ => false, }; - if short_circuit { + if is_short_circuit_pattern + && !exclude.maybe_is_parent_of_ignored_path(&directory_path) + { globbed_directories.insert(directory_path, directory_node.clone()); continue; } From e163e00b7c97f0802d78ceae041d072689ec0589 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 6 Sep 2021 10:14:45 +0200 Subject: [PATCH 3/9] docker: include file dependencies in docker build context properly. Signed-off-by: Andreas Stenius # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../backend/docker/docker_build_context.py | 36 ++++- .../docker/docker_build_context_test.py | 144 ++++++++++-------- 2 files changed, 108 insertions(+), 72 deletions(-) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 8906b8ec312..93231afa7f3 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -3,6 +3,7 @@ import logging from dataclasses import dataclass +from itertools import chain from pants.backend.docker.target_types import DockerImageSources from pants.core.goals.package import BuiltPackage, PackageFieldSet @@ -12,9 +13,12 @@ from pants.engine.fs import AddPrefix, Digest, MergeDigests from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( + Dependencies, + DependenciesRequest, FieldSetsPerTarget, FieldSetsPerTargetRequest, Sources, + Targets, TransitiveTargets, TransitiveTargetsRequest, ) @@ -31,18 +35,34 @@ class DockerBuildContext: class DockerBuildContextRequest: address: Address context_root: str + build_upstream_images: bool = False @rule async def create_docker_build_context(request: DockerBuildContextRequest) -> DockerBuildContext: + # Get all targets to include in context. transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([request.address])) - # Get all sources, going into the build context. + # Get all dependencies from those root targets. + all_dependencies = await MultiGet( + Get(Targets, DependenciesRequest(target.get(Dependencies), include_special_cased_deps=True)) + for target in transitive_targets.roots + ) + + # Get Dockerfiles from all roots + dockerfiles_request = Get( + SourceFiles, + SourceFilesRequest( + sources_fields=[t.get(Sources) for t in transitive_targets.roots], + for_sources_types=(DockerImageSources,), + ), + ) + # Get all sources from all dependencies (i.e. files and resources). sources_request = Get( SourceFiles, SourceFilesRequest( - sources_fields=[t.get(Sources) for t in transitive_targets.closure], - for_sources_types=(DockerImageSources, FilesSources, ResourcesSources), + sources_fields=[t.get(Sources) for t in chain(*all_dependencies)], + for_sources_types=(FilesSources, ResourcesSources), ), ) @@ -51,14 +71,19 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc FieldSetsPerTargetRequest(PackageFieldSet, transitive_targets.dependencies), ) - sources, embedded_pkgs_per_target = await MultiGet( - sources_request, embedded_pkgs_per_target_request + dockerfiles, sources, embedded_pkgs_per_target = await MultiGet( + dockerfiles_request, + sources_request, + embedded_pkgs_per_target_request, ) # Package binary dependencies for build context. embedded_pkgs = await MultiGet( Get(BuiltPackage, PackageFieldSet, field_set) for field_set in embedded_pkgs_per_target.field_sets + # Exclude docker images, unless build_upstream_images is true. + if request.build_upstream_images + or not isinstance(getattr(field_set, "sources", None), DockerImageSources) ) packages_str = ", ".join(a.relpath for p in embedded_pkgs for a in p.artifacts if a.relpath) @@ -77,6 +102,7 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc MergeDigests( d for d in ( + dockerfiles.snapshot.digest, sources.snapshot.digest, *embedded_pkgs_digests, ) diff --git a/src/python/pants/backend/docker/docker_build_context_test.py b/src/python/pants/backend/docker/docker_build_context_test.py index 862a2666ca5..a86add87033 100644 --- a/src/python/pants/backend/docker/docker_build_context_test.py +++ b/src/python/pants/backend/docker/docker_build_context_test.py @@ -1,79 +1,89 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from pants.backend.docker.docker_build_context import ( - DockerBuildContextRequest, - create_docker_build_context, -) +from __future__ import annotations + +from textwrap import dedent + +import pytest + +from pants.backend.docker.docker_build_context import DockerBuildContext, DockerBuildContextRequest +from pants.backend.docker.rules import rules from pants.backend.docker.target_types import DockerImage -from pants.backend.python.goals.package_pex_binary import PexBinaryFieldSet -from pants.backend.python.target_types import PexBinary -from pants.core.goals.package import BuiltPackage, PackageFieldSet -from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest +from pants.core.target_types import Files +from pants.core.util_rules.source_files import rules as source_files_rules from pants.engine.addresses import Address -from pants.engine.fs import EMPTY_DIGEST, EMPTY_SNAPSHOT, AddPrefix, Digest, MergeDigests -from pants.engine.target import ( - FieldSetsPerTarget, - FieldSetsPerTargetRequest, - TransitiveTargets, - TransitiveTargetsRequest, -) -from pants.engine.unions import UnionMembership -from pants.testutil.rule_runner import MockGet, run_rule_with_mocks +from pants.engine.fs import Snapshot +from pants.testutil.rule_runner import QueryRule, RuleRunner -def test_create_docker_build_context(): - img = DockerImage(address=Address("src/test", target_name="image"), unhydrated_values={}) - pex = PexBinary( - address=Address("src/test", target_name="bin"), - unhydrated_values={"entry_point": "src.test.main:main"}, - ) - request = DockerBuildContextRequest( - address=img.address, - context_root=".", +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *rules(), + *source_files_rules(), + QueryRule(DockerBuildContext, (DockerBuildContextRequest,)), + ], + target_types=[DockerImage, Files], ) - result = run_rule_with_mocks( - create_docker_build_context, - rule_args=[request], - mock_gets=[ - MockGet( - output_type=TransitiveTargets, - input_type=TransitiveTargetsRequest, - mock=lambda _: TransitiveTargets([img], [pex]), - ), - MockGet( - output_type=SourceFiles, - input_type=SourceFilesRequest, - mock=lambda _: SourceFiles( - snapshot=EMPTY_SNAPSHOT, - unrooted_files=tuple(), - ), - ), - MockGet( - output_type=FieldSetsPerTarget, - input_type=FieldSetsPerTargetRequest, - mock=lambda request: FieldSetsPerTarget([[PexBinaryFieldSet.create(pex)]]), - ), - MockGet( - output_type=BuiltPackage, - input_type=PackageFieldSet, - mock=lambda _: BuiltPackage(EMPTY_DIGEST, []), - ), - MockGet( - output_type=Digest, - input_type=AddPrefix, - mock=lambda _: EMPTY_DIGEST, - ), - MockGet( - output_type=Digest, - input_type=MergeDigests, - mock=lambda _: EMPTY_DIGEST, - ), + +def assert_build_context( + rule_runner: RuleRunner, context_root: str, address: Address, expected_files: list[str] +) -> None: + context = rule_runner.request( + DockerBuildContext, + [ + DockerBuildContextRequest( + address=address, + context_root=context_root, + ) ], - # need AddPrefix here, since UnionMembership.is_member() throws when called with non - # registered types - union_membership=UnionMembership({PackageFieldSet: [PexBinaryFieldSet], AddPrefix: []}), ) - assert result.digest == EMPTY_DIGEST + snapshot = rule_runner.request(Snapshot, [context.digest]) + assert sorted(expected_files) == sorted(snapshot.files) + + +def test_file_dependencies(rule_runner: RuleRunner) -> None: + # img_A -> files_A + # img_A -> img_B -> files_B + rule_runner.add_to_build_file( + "src/a", + dedent( + """\ + docker_image(name="img_A", dependencies=[":files_A", "src/b:img_B"]) + files(name="files_A", sources=["files/**"]) + """ + ), + ) + rule_runner.add_to_build_file( + "src/b", + dedent( + """\ + docker_image(name="img_B", dependencies=[":files_B"]) + files(name="files_B", sources=["files/**"]) + """ + ), + ) + rule_runner.create_files("src/a", ["Dockerfile"]) + rule_runner.create_files("src/a/files", ["a01", "a02"]) + rule_runner.create_files("src/b", ["Dockerfile"]) + rule_runner.create_files("src/b/files", ["b01", "b02"]) + + # We want files_B in build context for img_B + assert_build_context( + rule_runner, + ".", + Address("src/b", target_name="img_B"), + expected_files=["src/b/Dockerfile", "src/b/files/b01", "src/b/files/b02"], + ) + + # We want files_A in build context for img_A, but not files_B + assert_build_context( + rule_runner, + ".", + Address("src/a", target_name="img_A"), + expected_files=["src/a/Dockerfile", "src/a/files/a01", "src/a/files/a02"], + ) From efa2fa668475d541f4cd5153ae07a83222374bef Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 6 Sep 2021 16:32:13 +0200 Subject: [PATCH 4/9] attempt at eliminating as much as possible the need for docker_image(context_root). Signed-off-by: Andreas Stenius # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/docker/docker_build.py | 8 +- .../backend/docker/docker_build_context.py | 45 ++++++---- .../docker/docker_build_context_test.py | 83 ++++++++++++++++++- .../pants/backend/docker/target_types.py | 18 +++- 4 files changed, 129 insertions(+), 25 deletions(-) diff --git a/src/python/pants/backend/docker/docker_build.py b/src/python/pants/backend/docker/docker_build.py index 059ce6a4fe7..bacc7f5670c 100644 --- a/src/python/pants/backend/docker/docker_build.py +++ b/src/python/pants/backend/docker/docker_build.py @@ -61,7 +61,13 @@ async def build_docker_image( docker, context = await MultiGet( Get(DockerBinary, DockerBinaryRequest()), Get( - DockerBuildContext, DockerBuildContextRequest(field_set.address, field_set.context_root) + DockerBuildContext, + DockerBuildContextRequest( + address=field_set.address, + context_root=field_set.context_root, + organize_context_tree=not field_set.context_root_field.value, + build_upstream_images=True, + ), ), ) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 93231afa7f3..7080cc1c58f 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -10,7 +10,7 @@ from pants.core.target_types import FilesSources, ResourcesSources from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address -from pants.engine.fs import AddPrefix, Digest, MergeDigests +from pants.engine.fs import Digest, DigestSubset, MergeDigests, PathGlobs, RemovePrefix, Snapshot from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( Dependencies, @@ -35,6 +35,7 @@ class DockerBuildContext: class DockerBuildContextRequest: address: Address context_root: str + organize_context_tree: bool = True build_upstream_images: bool = False @@ -49,7 +50,7 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc for target in transitive_targets.roots ) - # Get Dockerfiles from all roots + # Get Dockerfiles from all roots. dockerfiles_request = Get( SourceFiles, SourceFilesRequest( @@ -89,25 +90,35 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc packages_str = ", ".join(a.relpath for p in embedded_pkgs for a in p.artifacts if a.relpath) logger.debug(f"Packages for Docker image: {packages_str}") - embedded_pkgs_digests = tuple(built_package.digest for built_package in embedded_pkgs) - if request.context_root != ".": - # Copy packages to context root tree, unless the context root is at the project root. - embedded_pkgs_digests = await MultiGet( - Get(Digest, AddPrefix(digest, request.context_root)) for digest in embedded_pkgs_digests + embedded_pkgs_digest = [built_package.digest for built_package in embedded_pkgs] + all_digests = (dockerfiles.snapshot.digest, sources.snapshot.digest, *embedded_pkgs_digest) + + if request.organize_context_tree and request.context_root != ".": + # Get all files not in context tree, they are where they should be already. + rooted_trees = await MultiGet( + Get(Snapshot, DigestSubset(digest, PathGlobs(["**", f"!{request.context_root}"]))) + for digest in all_digests + ) + + # Get all files in context tree, they will be moved up to root. + context_trees = await MultiGet( + Get(Digest, DigestSubset(digest, PathGlobs([f"{request.context_root}/**"]))) + for digest in all_digests ) - # Merge build context. + # Strip context root from all files in context tree. + organized_trees = await MultiGet( + Get(Snapshot, RemovePrefix(digest, request.context_root)) for digest in context_trees + ) + + # The result is the newly organized tree, along with the files that weren't in the context + # root tree. + all_digests = (*[s.digest for s in organized_trees], *[s.digest for s in rooted_trees]) + + # Merge all digests to get the final docker build context. context = await Get( Digest, - MergeDigests( - d - for d in ( - dockerfiles.snapshot.digest, - sources.snapshot.digest, - *embedded_pkgs_digests, - ) - if d - ), + MergeDigests(d for d in all_digests if d), ) return DockerBuildContext(context) diff --git a/src/python/pants/backend/docker/docker_build_context_test.py b/src/python/pants/backend/docker/docker_build_context_test.py index a86add87033..b40b48635c0 100644 --- a/src/python/pants/backend/docker/docker_build_context_test.py +++ b/src/python/pants/backend/docker/docker_build_context_test.py @@ -4,6 +4,7 @@ from __future__ import annotations from textwrap import dedent +from typing import Optional import pytest @@ -30,7 +31,11 @@ def rule_runner() -> RuleRunner: def assert_build_context( - rule_runner: RuleRunner, context_root: str, address: Address, expected_files: list[str] + rule_runner: RuleRunner, + context_root: str, + address: Address, + expected_files: list[str], + organize_context_tree: Optional[bool] = None, ) -> None: context = rule_runner.request( DockerBuildContext, @@ -38,6 +43,10 @@ def assert_build_context( DockerBuildContextRequest( address=address, context_root=context_root, + organize_context_tree=context_root != "." + if organize_context_tree is None + else organize_context_tree, + build_upstream_images=False, ) ], ) @@ -75,9 +84,17 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: # We want files_B in build context for img_B assert_build_context( rule_runner, - ".", + "src/b", Address("src/b", target_name="img_B"), - expected_files=["src/b/Dockerfile", "src/b/files/b01", "src/b/files/b02"], + expected_files=["Dockerfile", "files/b01", "files/b02"], + ) + + # We want files_A in build context for img_A, but not files_B + assert_build_context( + rule_runner, + "src/a", + Address("src/a", target_name="img_A"), + expected_files=["Dockerfile", "files/a01", "files/a02"], ) # We want files_A in build context for img_A, but not files_B @@ -87,3 +104,63 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: Address("src/a", target_name="img_A"), expected_files=["src/a/Dockerfile", "src/a/files/a01", "src/a/files/a02"], ) + + # Mixed. + rule_runner.add_to_build_file( + "src/c", + dedent( + """\ + docker_image(name="img_C", dependencies=["src/a:files_A", "src/b:files_B"]) + """ + ), + ) + rule_runner.create_files("src/c", ["Dockerfile"]) + + assert_build_context( + rule_runner, + "src", + Address("src/c", target_name="img_C"), + expected_files=[ + "src/c/Dockerfile", + "src/a/files/a01", + "src/a/files/a02", + "src/b/files/b01", + "src/b/files/b02", + ], + organize_context_tree=False, + ) + + +def test_files_out_of_tree(rule_runner: RuleRunner) -> None: + # src/a:img_A -> res/static:files + rule_runner.add_to_build_file( + "src/a", + dedent( + """\ + docker_image(name="img_A", dependencies=["res/static:files"]) + """ + ), + ) + rule_runner.add_to_build_file( + "res/static", + dedent( + """\ + files(name="files", sources=["!BUILD", "**/*"]) + """ + ), + ) + rule_runner.create_files("src/a", ["Dockerfile"]) + rule_runner.create_files("res/static", ["s01", "s02"]) + rule_runner.create_files("res/static/sub", ["s03"]) + + assert_build_context( + rule_runner, + "src/a", + Address("src/a", target_name="img_A"), + expected_files=[ + "Dockerfile", + "res/static/s01", + "res/static/s02", + "res/static/sub/s03", + ], + ) diff --git a/src/python/pants/backend/docker/target_types.py b/src/python/pants/backend/docker/target_types.py index d7242553e24..1e5c4057d0a 100644 --- a/src/python/pants/backend/docker/target_types.py +++ b/src/python/pants/backend/docker/target_types.py @@ -22,11 +22,21 @@ class DockerDependencies(Dependencies): class DockerContextRoot(StringField): alias = "context_root" - default = "." + default = None help = ( - "Root directory for the Docker build context.\n\nDefault is to use the directory of the " - "`BUILD` file. Use '/' to use the project root, thus putting any resource from the entire " - "project within scope (if the target also has a dependency on it)." + "By default, the required files are assembled into a build context as follows:\n" + "\n" + " * The sources of `files` targets are assembled at their relative path from the " + "repo root.\n" + " * The sources of `resources` targets are assembled at their relative path from " + "their source roots.\n" + " * The artifacts of any packageable targets are built, as if by running " + "`./pants package`, and placed in the context under a subdirectory named for the " + "target's path from the repo root.\n" + "\n" + "[Advanced] By overriding with a custom value, the files will not be assembled, " + "but rather left at their default locations, which may be outside of the Docker " + "build context, and thus unusable for ADD/COPY commands in the `Dockerfile`." ) From 96485956a842178830c4cd0cae50da4da2377b77 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 6 Sep 2021 16:37:57 +0200 Subject: [PATCH 5/9] cleanup. Signed-off-by: Andreas Stenius # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/docker/docker_build_context.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 7080cc1c58f..43427c248f4 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -10,7 +10,7 @@ from pants.core.target_types import FilesSources, ResourcesSources from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address -from pants.engine.fs import Digest, DigestSubset, MergeDigests, PathGlobs, RemovePrefix, Snapshot +from pants.engine.fs import Digest, DigestSubset, MergeDigests, PathGlobs, RemovePrefix from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( Dependencies, @@ -96,7 +96,7 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc if request.organize_context_tree and request.context_root != ".": # Get all files not in context tree, they are where they should be already. rooted_trees = await MultiGet( - Get(Snapshot, DigestSubset(digest, PathGlobs(["**", f"!{request.context_root}"]))) + Get(Digest, DigestSubset(digest, PathGlobs(["**", f"!{request.context_root}"]))) for digest in all_digests ) @@ -108,12 +108,12 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc # Strip context root from all files in context tree. organized_trees = await MultiGet( - Get(Snapshot, RemovePrefix(digest, request.context_root)) for digest in context_trees + Get(Digest, RemovePrefix(digest, request.context_root)) for digest in context_trees ) # The result is the newly organized tree, along with the files that weren't in the context # root tree. - all_digests = (*[s.digest for s in organized_trees], *[s.digest for s in rooted_trees]) + all_digests = (*organized_trees, *rooted_trees) # Merge all digests to get the final docker build context. context = await Get( From 3503d8693905755f6ee6733422bd2bf6f4f09e0b Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Mon, 6 Sep 2021 16:42:26 +0200 Subject: [PATCH 6/9] indent. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../docker/docker_build_context_test.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/python/pants/backend/docker/docker_build_context_test.py b/src/python/pants/backend/docker/docker_build_context_test.py index b40b48635c0..db71e58baf2 100644 --- a/src/python/pants/backend/docker/docker_build_context_test.py +++ b/src/python/pants/backend/docker/docker_build_context_test.py @@ -62,18 +62,18 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: "src/a", dedent( """\ - docker_image(name="img_A", dependencies=[":files_A", "src/b:img_B"]) - files(name="files_A", sources=["files/**"]) - """ + docker_image(name="img_A", dependencies=[":files_A", "src/b:img_B"]) + files(name="files_A", sources=["files/**"]) + """ ), ) rule_runner.add_to_build_file( "src/b", dedent( """\ - docker_image(name="img_B", dependencies=[":files_B"]) - files(name="files_B", sources=["files/**"]) - """ + docker_image(name="img_B", dependencies=[":files_B"]) + files(name="files_B", sources=["files/**"]) + """ ), ) rule_runner.create_files("src/a", ["Dockerfile"]) @@ -110,7 +110,7 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: "src/c", dedent( """\ - docker_image(name="img_C", dependencies=["src/a:files_A", "src/b:files_B"]) + docker_image(name="img_C", dependencies=["src/a:files_A", "src/b:files_B"]) """ ), ) @@ -137,16 +137,16 @@ def test_files_out_of_tree(rule_runner: RuleRunner) -> None: "src/a", dedent( """\ - docker_image(name="img_A", dependencies=["res/static:files"]) - """ + docker_image(name="img_A", dependencies=["res/static:files"]) + """ ), ) rule_runner.add_to_build_file( "res/static", dedent( """\ - files(name="files", sources=["!BUILD", "**/*"]) - """ + files(name="files", sources=["!BUILD", "**/*"]) + """ ), ) rule_runner.create_files("src/a", ["Dockerfile"]) From d768563a686d582ca3de4fe8aa2353fc3762e873 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Tue, 7 Sep 2021 11:14:48 +0200 Subject: [PATCH 7/9] docker: remove confusing context_root field of docker_image. Signed-off-by: Andreas Stenius # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/docker/docker_build.py | 22 ++----------- .../backend/docker/docker_build_context.py | 26 +-------------- .../docker/docker_build_context_test.py | 24 ++------------ .../pants/backend/docker/docker_build_test.py | 33 ------------------- .../pants/backend/docker/target_types.py | 21 ------------ 5 files changed, 5 insertions(+), 121 deletions(-) diff --git a/src/python/pants/backend/docker/docker_build.py b/src/python/pants/backend/docker/docker_build.py index bacc7f5670c..bc56e515314 100644 --- a/src/python/pants/backend/docker/docker_build.py +++ b/src/python/pants/backend/docker/docker_build.py @@ -7,11 +7,7 @@ from pants.backend.docker.docker_binary import DockerBinary, DockerBinaryRequest from pants.backend.docker.docker_build_context import DockerBuildContext, DockerBuildContextRequest -from pants.backend.docker.target_types import ( - DockerContextRoot, - DockerImageSources, - DockerImageVersion, -) +from pants.backend.docker.target_types import DockerImageSources, DockerImageVersion from pants.core.goals.package import BuiltPackage, BuiltPackageArtifact, PackageFieldSet from pants.engine.process import Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule @@ -24,7 +20,6 @@ class DockerFieldSet(PackageFieldSet): required_fields = (DockerImageSources,) - context_root_field: DockerContextRoot image_version: DockerImageVersion sources: DockerImageSources @@ -38,17 +33,6 @@ def dockerfile_relpath(self) -> str: def dockerfile_path(self) -> str: return os.path.join(self.address.spec_path, self.dockerfile_relpath) - @property - def context_root(self) -> str: - """Translates context_root from a users perspective in the BUILD file, to Dockers - perspective of the file system.""" - path = self.context_root_field.value or "." - if os.path.isabs(path): - path = os.path.relpath(path, "/") - else: - path = os.path.join(self.address.spec_path, path) - return path - @property def image_tag(self) -> str: return ":".join(s for s in [self.address.target_name, self.image_version.value] if s) @@ -64,8 +48,6 @@ async def build_docker_image( DockerBuildContext, DockerBuildContextRequest( address=field_set.address, - context_root=field_set.context_root, - organize_context_tree=not field_set.context_root_field.value, build_upstream_images=True, ), ), @@ -77,7 +59,7 @@ async def build_docker_image( docker.build_image( field_set.image_tag, context.digest, - field_set.context_root, + ".", field_set.dockerfile_path, ), ) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 43427c248f4..6bccd744809 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -10,7 +10,7 @@ from pants.core.target_types import FilesSources, ResourcesSources from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address -from pants.engine.fs import Digest, DigestSubset, MergeDigests, PathGlobs, RemovePrefix +from pants.engine.fs import Digest, MergeDigests from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( Dependencies, @@ -34,8 +34,6 @@ class DockerBuildContext: @dataclass(frozen=True) class DockerBuildContextRequest: address: Address - context_root: str - organize_context_tree: bool = True build_upstream_images: bool = False @@ -93,28 +91,6 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc embedded_pkgs_digest = [built_package.digest for built_package in embedded_pkgs] all_digests = (dockerfiles.snapshot.digest, sources.snapshot.digest, *embedded_pkgs_digest) - if request.organize_context_tree and request.context_root != ".": - # Get all files not in context tree, they are where they should be already. - rooted_trees = await MultiGet( - Get(Digest, DigestSubset(digest, PathGlobs(["**", f"!{request.context_root}"]))) - for digest in all_digests - ) - - # Get all files in context tree, they will be moved up to root. - context_trees = await MultiGet( - Get(Digest, DigestSubset(digest, PathGlobs([f"{request.context_root}/**"]))) - for digest in all_digests - ) - - # Strip context root from all files in context tree. - organized_trees = await MultiGet( - Get(Digest, RemovePrefix(digest, request.context_root)) for digest in context_trees - ) - - # The result is the newly organized tree, along with the files that weren't in the context - # root tree. - all_digests = (*organized_trees, *rooted_trees) - # Merge all digests to get the final docker build context. context = await Get( Digest, diff --git a/src/python/pants/backend/docker/docker_build_context_test.py b/src/python/pants/backend/docker/docker_build_context_test.py index db71e58baf2..4163527a19a 100644 --- a/src/python/pants/backend/docker/docker_build_context_test.py +++ b/src/python/pants/backend/docker/docker_build_context_test.py @@ -4,7 +4,6 @@ from __future__ import annotations from textwrap import dedent -from typing import Optional import pytest @@ -32,20 +31,14 @@ def rule_runner() -> RuleRunner: def assert_build_context( rule_runner: RuleRunner, - context_root: str, address: Address, expected_files: list[str], - organize_context_tree: Optional[bool] = None, ) -> None: context = rule_runner.request( DockerBuildContext, [ DockerBuildContextRequest( address=address, - context_root=context_root, - organize_context_tree=context_root != "." - if organize_context_tree is None - else organize_context_tree, build_upstream_images=False, ) ], @@ -84,23 +77,13 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: # We want files_B in build context for img_B assert_build_context( rule_runner, - "src/b", Address("src/b", target_name="img_B"), - expected_files=["Dockerfile", "files/b01", "files/b02"], + expected_files=["src/b/Dockerfile", "src/b/files/b01", "src/b/files/b02"], ) # We want files_A in build context for img_A, but not files_B assert_build_context( rule_runner, - "src/a", - Address("src/a", target_name="img_A"), - expected_files=["Dockerfile", "files/a01", "files/a02"], - ) - - # We want files_A in build context for img_A, but not files_B - assert_build_context( - rule_runner, - ".", Address("src/a", target_name="img_A"), expected_files=["src/a/Dockerfile", "src/a/files/a01", "src/a/files/a02"], ) @@ -118,7 +101,6 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: assert_build_context( rule_runner, - "src", Address("src/c", target_name="img_C"), expected_files=[ "src/c/Dockerfile", @@ -127,7 +109,6 @@ def test_file_dependencies(rule_runner: RuleRunner) -> None: "src/b/files/b01", "src/b/files/b02", ], - organize_context_tree=False, ) @@ -155,10 +136,9 @@ def test_files_out_of_tree(rule_runner: RuleRunner) -> None: assert_build_context( rule_runner, - "src/a", Address("src/a", target_name="img_A"), expected_files=[ - "Dockerfile", + "src/a/Dockerfile", "res/static/s01", "res/static/s02", "res/static/sub/s03", diff --git a/src/python/pants/backend/docker/docker_build_test.py b/src/python/pants/backend/docker/docker_build_test.py index 0112e8de45c..5943e2b2263 100644 --- a/src/python/pants/backend/docker/docker_build_test.py +++ b/src/python/pants/backend/docker/docker_build_test.py @@ -16,12 +16,6 @@ @pytest.mark.parametrize( "target_values, expected_features", [ - ( - dict(), - dict( - context_root="docker/test/.", - ), - ), ( dict( version="1.2.3", @@ -30,30 +24,6 @@ version="1.2.3", ), ), - ( - dict( - context_root="/", - ), - dict( - context_root=".", - ), - ), - ( - dict( - context_root="foo/bar", - ), - dict( - context_root="docker/test/foo/bar", - ), - ), - ( - dict( - context_root="/foo/bar", - ), - dict( - context_root="foo/bar", - ), - ), ], ) def test_build_docker_image_rule(target_values, expected_features): @@ -65,9 +35,6 @@ def test_build_docker_image_rule(target_values, expected_features): field_set = DockerFieldSet.create(image) def build_context_mock(request: DockerBuildContextRequest) -> DockerBuildContext: - if "context_root" in expected_features: - assert expected_features["context_root"] == request.context_root - return DockerBuildContext(digest=EMPTY_DIGEST) result = run_rule_with_mocks( diff --git a/src/python/pants/backend/docker/target_types.py b/src/python/pants/backend/docker/target_types.py index 1e5c4057d0a..5e75f221b3c 100644 --- a/src/python/pants/backend/docker/target_types.py +++ b/src/python/pants/backend/docker/target_types.py @@ -20,31 +20,10 @@ class DockerDependencies(Dependencies): supports_transitive_excludes = True -class DockerContextRoot(StringField): - alias = "context_root" - default = None - help = ( - "By default, the required files are assembled into a build context as follows:\n" - "\n" - " * The sources of `files` targets are assembled at their relative path from the " - "repo root.\n" - " * The sources of `resources` targets are assembled at their relative path from " - "their source roots.\n" - " * The artifacts of any packageable targets are built, as if by running " - "`./pants package`, and placed in the context under a subdirectory named for the " - "target's path from the repo root.\n" - "\n" - "[Advanced] By overriding with a custom value, the files will not be assembled, " - "but rather left at their default locations, which may be outside of the Docker " - "build context, and thus unusable for ADD/COPY commands in the `Dockerfile`." - ) - - class DockerImage(Target): alias = "docker_image" core_fields = ( *COMMON_TARGET_FIELDS, - DockerContextRoot, DockerDependencies, DockerImageSources, DockerImageVersion, From 63c13552e6551e5e5ec47f22b08db4d311e100ea Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Wed, 8 Sep 2021 11:54:59 +0200 Subject: [PATCH 8/9] review feedback. Signed-off-by: Andreas Stenius # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/docker/docker_binary.py | 2 +- src/python/pants/backend/docker/docker_build.py | 7 +++---- .../pants/backend/docker/docker_build_context.py | 11 ++++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/python/pants/backend/docker/docker_binary.py b/src/python/pants/backend/docker/docker_binary.py index a61bd8e59b4..30acaca8073 100644 --- a/src/python/pants/backend/docker/docker_binary.py +++ b/src/python/pants/backend/docker/docker_binary.py @@ -24,7 +24,7 @@ class DockerBinary(BinaryPath): DEFAULT_SEARCH_PATH = SearchPath(("/usr/bin", "/bin", "/usr/local/bin")) def build_image( - self, tag: str, digest: Digest, context_root: str, dockerfile: Optional[str] = None + self, tag: str, digest: Digest, context_root: str = ".", dockerfile: Optional[str] = None ) -> Process: args = [self.path, "build", "-t", tag] if dockerfile: diff --git a/src/python/pants/backend/docker/docker_build.py b/src/python/pants/backend/docker/docker_build.py index bc56e515314..0405002693d 100644 --- a/src/python/pants/backend/docker/docker_build.py +++ b/src/python/pants/backend/docker/docker_build.py @@ -57,10 +57,9 @@ async def build_docker_image( ProcessResult, Process, docker.build_image( - field_set.image_tag, - context.digest, - ".", - field_set.dockerfile_path, + tag=field_set.image_tag, + digest=context.digest, + dockerfile=field_set.dockerfile_path, ), ) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 6bccd744809..6d4d9e720b2 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -7,7 +7,7 @@ from pants.backend.docker.target_types import DockerImageSources from pants.core.goals.package import BuiltPackage, PackageFieldSet -from pants.core.target_types import FilesSources, ResourcesSources +from pants.core.target_types import FilesSources from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address from pants.engine.fs import Digest, MergeDigests @@ -44,11 +44,11 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc # Get all dependencies from those root targets. all_dependencies = await MultiGet( - Get(Targets, DependenciesRequest(target.get(Dependencies), include_special_cased_deps=True)) + Get(Targets, DependenciesRequest(target.get(Dependencies))) for target in transitive_targets.roots ) - # Get Dockerfiles from all roots. + # Get the Dockerfile from the root target. dockerfiles_request = Get( SourceFiles, SourceFilesRequest( @@ -56,12 +56,13 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc for_sources_types=(DockerImageSources,), ), ) - # Get all sources from all dependencies (i.e. files and resources). + + # Get all sources from all dependencies (i.e. files). sources_request = Get( SourceFiles, SourceFilesRequest( sources_fields=[t.get(Sources) for t in chain(*all_dependencies)], - for_sources_types=(FilesSources, ResourcesSources), + for_sources_types=(FilesSources,), ), ) From a9a1e7c5bb850976668517b0dac323f1b0dde868 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Wed, 8 Sep 2021 15:04:34 +0200 Subject: [PATCH 9/9] cleanup. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/docker/docker_binary.py | 8 ++++---- .../pants/backend/docker/docker_binary_test.py | 8 +++----- .../pants/backend/docker/docker_build_context.py | 16 ++++++++-------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/python/pants/backend/docker/docker_binary.py b/src/python/pants/backend/docker/docker_binary.py index 30acaca8073..6e511205cef 100644 --- a/src/python/pants/backend/docker/docker_binary.py +++ b/src/python/pants/backend/docker/docker_binary.py @@ -23,13 +23,13 @@ class DockerBinary(BinaryPath): DEFAULT_SEARCH_PATH = SearchPath(("/usr/bin", "/bin", "/usr/local/bin")) - def build_image( - self, tag: str, digest: Digest, context_root: str = ".", dockerfile: Optional[str] = None - ) -> Process: + def build_image(self, tag: str, digest: Digest, dockerfile: Optional[str] = None) -> Process: args = [self.path, "build", "-t", tag] if dockerfile: args.extend(["-f", dockerfile]) - args.append(context_root) + + # Add build context root. + args.append(".") return Process( argv=tuple(args), diff --git a/src/python/pants/backend/docker/docker_binary_test.py b/src/python/pants/backend/docker/docker_binary_test.py index b0de141e5eb..d802733456a 100644 --- a/src/python/pants/backend/docker/docker_binary_test.py +++ b/src/python/pants/backend/docker/docker_binary_test.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from hashlib import sha256 -from os import path from pants.backend.docker.docker_binary import DockerBinary from pants.engine.fs import Digest @@ -10,16 +9,15 @@ def test_docker_binary_build_image(): - source_path = "src/test/repo" docker_path = "/bin/docker" - dockerfile = path.join(source_path, "Dockerfile") + dockerfile = "src/test/repo/Dockerfile" docker = DockerBinary(docker_path) digest = Digest(sha256().hexdigest(), 123) tag = "test:latest" - build_request = docker.build_image(tag, digest, source_path, dockerfile) + build_request = docker.build_image(tag, digest, dockerfile) assert build_request == Process( - argv=(docker_path, "build", "-t", tag, "-f", dockerfile, source_path), + argv=(docker_path, "build", "-t", tag, "-f", dockerfile, "."), input_digest=digest, description=f"Building docker image {tag}", ) diff --git a/src/python/pants/backend/docker/docker_build_context.py b/src/python/pants/backend/docker/docker_build_context.py index 6d4d9e720b2..ae7588dd6f9 100644 --- a/src/python/pants/backend/docker/docker_build_context.py +++ b/src/python/pants/backend/docker/docker_build_context.py @@ -42,12 +42,6 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc # Get all targets to include in context. transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([request.address])) - # Get all dependencies from those root targets. - all_dependencies = await MultiGet( - Get(Targets, DependenciesRequest(target.get(Dependencies))) - for target in transitive_targets.roots - ) - # Get the Dockerfile from the root target. dockerfiles_request = Get( SourceFiles, @@ -57,11 +51,17 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc ), ) - # Get all sources from all dependencies (i.e. files). + # Get all dependencies for the root target. + root_dependencies = await MultiGet( + Get(Targets, DependenciesRequest(target.get(Dependencies))) + for target in transitive_targets.roots + ) + + # Get all sources from the root dependencies (i.e. files). sources_request = Get( SourceFiles, SourceFilesRequest( - sources_fields=[t.get(Sources) for t in chain(*all_dependencies)], + sources_fields=[t.get(Sources) for t in chain(*root_dependencies)], for_sources_types=(FilesSources,), ), )