narrow pex_binary files warning

[cognifloyd]

There is a helpful warning when using pex_binary and one or more of the transitive dependencies is a files target. However, that warning is overly broad, as there are situations when it is ok to have files targets.

When a PEX depends on a packaged target, such as python_distribution, then any transitive files targets are irrelevant because pex just installs the wheel with whatever is included in it.

Background

I am building a pex_binary that has explicit dependencies on a number of python_distribution targets. This pex_binary has entry_point=None, because I am using it as a relocatable archived virtualenv. I set include_tools=True so that I can take the pex file and run PEX_TOOLS=1 dist/st2.pex venv /opt/stackstorm/st2 to quickly install the entire virutalenv (which consists of the exact versions of deps locked in the resolve's lockfile). So, the files warning was also irrelevant for me, because I'm not going to run any of the 1st party code from within the pex; it will be extracted as a venv before the 1st party code runs.

While studying pex_binary, I found that the include_sources option skips including the transitive sources. But, generated dependencies, such as the wheels generated by python_distribution still get included in the pex (because include_local_dists=True is hard-coded for pex_binary). Because include_sources skips including the transitive sources, I figured this would be a clean/predictable way to avoid the warning and still build the packaged-virtualenv pex like I want.

Note

I labeled this as category:bugfix because the warning was not accounting for the behavioral change that happens when using the include_sources flag. But I can also see calling this category:user api change (it minimizes when you see the warning) or category:new feature (it allows more uses of pex_binary without that warning).

[benjyw]

This seems like yet another use case for "not traversing deps through build dependencies", as discussed in #18254

[cognifloyd]

This seems like yet another use case for "not traversing deps through build dependencies", as discussed in #18254

Agreed. Not traversing package deps would prevent (possibly) duplicating the sources if include_sources=True. But that would not mean that we could delete this warning altogether. So, we would need to adopt the FilteredTransitiveDepWalking primitive in both ...util_rules.pex_from_targets.create_pex_from_targets() and in backend.python.goals.package_pex_binary.package_pex_binary(). Even if/when we do that, checking for files targets when include_sources=False seems to be a waste of cycles. So, I think this PR makes sense either way.

[benjyw]

Nice