@happy-kitchen-89482 re transitive dep traversal and pex_binary targets: Building a

pex_binary

uses

PexFromTargetsRequest

which has several phases: • resolve, requirements, interpreter_constraints ◦ (these use transitive targets for calculation, and I don’t see a problem with traversing through package targets for this) •

include_source_files

uses transitive targets to see which raw source files should be included ◦ This is where my changes would stop at any binary targets ◦ This gets fed into

PythonSourceFilesRequest

which would not return any dists, effectively dropping all the packaged targets and their deps. •

include_local_dists

builds any required dists/wheels for inclusion in the pex. ◦ These are actually in a separate pex from the source files pex. ◦ This is where all of the subtraction logic you wrote comes into play. ◦ This re-runs the

TransitiveTargetsRequest

but without excluding packaged targets to find all of the local dists. ◦ Because fewer sources were identified by the

include_source_files

stuff, the

remaining_sources

should have fewer files to begin with, so the subtraction logic should have less conflicts to deal with. https://github.com/pantsbuild/pants/blob/c58ce117e2d99927811379a90de68fca49eda330/src/python/pants/backend/python/util_rules/pex_from_targets.py#L540-L570 https://github.com/pantsbuild/pants/blob/c58ce117e2d99927811379a90de68fca49eda330/src/python/pants/backend/python/util_rules/local_dists.py#L1[…]209 So, my fix works around this bug https://github.com/pantsbuild/pants/issues/15855 by simply not including files from dists to begin with, before the dist subtraction happens. If we were to fix this without my transitive dep predicate, I think we need to adjust the subtraction logic so it can subtract unrooted files as well, but I’m not sure if that is sufficient because the conflicting files in the demo repo are

resources

which are rooted: https://github.com/pantsbuild/pants/blob/c58ce117e2d99927811379a90de68fca49eda330/src/python/pants/backend/python/util_rules/local_dists.py#L205

Got it. But I don't understand how your change addresses the issue in

include_source_files

?

edit: Doh. I never hit send on this; it's been sitting in my drafts. Yeah. We need something else for inferred deps, I’m not sure what. With inference, a pex_binary could have an entry point, and that entry point gets inferred deps on other python files that could be part of a local dist. But, since the dist itself is not in the

transitive_targets.closure

, I don’t think the

LocalDists

bit will request it. Since inference won’t add deps on dists, it is as if they didn’t exist today.