Add back python_app / bundle, but improved

[Eric-Arellano]

A user explained that they have use for this goal. They very intentionally want the loose files to be kept outside the .pex file.

However, we should make this much simpler than before and redesign from first principles.

[Eric-Arellano]

Requirements

• Be able to have loose files not in the binary, e.g. not in the .pex file.
• Be able to set the rel_path for files so that they have a common prefix.
• Be able to choose the archive format.

Nice to haves

• Get rid of bundles; it was confusing.
• Consolidate the bundle goal into binary. It was confusing to everyone what the difference was.
• Generalize this pattern; rather than python_app, jvm_app, etc, have something like archive.
• Allow >1 binary to appear in the final archive.

Proposal

Add a new archive target, with the fields:

• binaries -> path to binary targets, i.e. targets that ./pants binary knows how to operate on, even if not traditional python_binary targets.
• files -> path to files targets (or subclasses)
• format, e.g. .zip vs. .tar.gz

To accommodate rel_path, files gains a new field path_prefix: Optional[str]. This path_prefix will get used everywhere the files target is used, including when running tests. It's unclear how to guarantee this happens, e.g. if we want to special case the hydrate_sources rule. What should ./pants filedeps do? This step should be broken out as a precursor.

archive(
  binaries=[":bin1", ":bin2"],
  files=["//:files1"],
  format=".zip"
)

files(
  name="files1",
  sources=["*.json"],
  path_prefix="json_files/"
)

[stuhood]

Thanks!

And as discussed in the comments on #10756, it would be good to be consistent about whether there are separate files and binaries lists, or whether because both of them should be treated as "untyped files", they should go in a single list for which we assume all entries can be converted into "untyped files".

To accommodate rel_path, files gains a new field path_prefix: Optional[str]. This path_prefix will get used everywhere the files target is used, including when running tests. It's unclear how to guarantee this happens, e.g. if we want to special case the hydrate_sources rule. What should ./pants filedeps do? This step should be broken out as a precursor.

This will be important.

One related challenge that occurred to me is that configuring the rel_path on files targets might mean that we end up needing a parallel option on (python_)binary targets, because currently you can only specify their name. Given that, some other prior art that @tdyas has mentioned is the maven assembly plugin (examples), which is a bit more imperative. An imperative strategy might involve declaring filtering/relocating/etc imperative statements that you could declare on a target that depended on the files... i.e., on an archive or binary, or on some intermediate relocated_files target or something.

EDIT: And another thing that we should likely fix here as well is that files targets are currently relative to the buildroot... but it's possible that they should be relative to a sourceroot or to their definition location.

[stuhood]

We've marked this as a blocker for 2.0.0rc0, but mostly because of this question:

And another thing that we should likely fix here as well is that files targets are currently relative to the buildroot... but it's possible that they should be relative to a sourceroot or to their definition location.

If it turns out that we don't need to change files, then this probably won't be a blocker for rc0.

[Eric-Arellano]

I was typing out some thoughts about how technically we can allow any target type in the files field, and a more apt name might be loose_sources. This could be convenient to put a python_library target in the field and have it included as loose files.

But while typing it out, I realized that in order to get the files to show up without their source roots in both archive and test, you would need to use a files() dependency for test, anyways. If you depend on a python_library in a python_tests target, we will strip the source root.

In other words, I think that always using a files() target to refer to loose source files is good. And +1 to the relative path feature living on a files target.

--

There's one downside that Pants generally doesn't work as well if >1 target refers to the same file. The main place this breaks things is dep inference. But, fortunately, our dep inference code only considers Python targets, so it's legal to have a files owner and a python_library target at the same time.

[Eric-Arellano]

I'm working on the files target changes for now, and came up with these use cases for the rel_path feature:

1. Remove a prefix, e.g. src/resources/f.json becomes f.json
2. Add a prefix, e.g. f.json becomes json/f.json.
3. Remove a prefix and add a prefix at the same time, e.g. src/resources/f.json becomes json/f.json.

Some other requirements:

• filedeps should use the original paths
• test and archive must use the modified paths
• It's unclear what other goals like fmt and run should do. Likely, use the modified paths for consistency.

I'm thinking the most flexible design will be a path_prefix_mapper field, similar to the mapper field for bundle:

# 1 remove 'src/python'
files(
  path_prefix_mapper={"src/python": ""},
)

# 2 add 'thirdparty'
files(
  path_prefix_mapper={"": "thirdparty"},
)

# 3 remove and then add
files(
  path_prefix_mapper={"src/python": "thirdparty"},
)

This field would validate that the dict does not have >1 entry. Why? This manipulation should apply to every single file in the files target; you should not have branching like {"parent/dir1": "foo", "parent/dir2": "bar"}. If you need that granularity, create more granular files targets.

I think I'd prefer to avoid multiple different fields that are all related, like we had with bundle: https://v1.pantsbuild.org/build_dictionary.html. I found it a bit difficult to know when to use what, and how they related. We also don't have a good Target API story for multiple fields being related and validating their interaction, and it goes against the Field-Driven nature of the API: #9561.

[stuhood]

I'm working on the files target changes for now, and came up with these use cases for the rel_path feature:
..
Some other requirements:

This all looks great!

It's unclear what other goals like fmt and run should do. Likely, use the modified paths for consistency.

fmt is not running the code, so it shouldn't need to apply source root rewriting (I'm guessing it already doesn't?). It would also make it much more challenging to write the manipulated outputs back into the filesystem.

And re: run: I think that with the formulation on this ticket, a *_binary target would not depend on the loose files: rather, an archive would. That would mean that run would not need to worry about loose files when passed a *_binary target. On the other hand, I do wonder what the behavior of run for an archive would be... but could punt on that.

This field would validate that the dict does not have >1 entry. Why? This manipulation should apply to every single file in the files target; you should not have branching like {"parent/dir1": "foo", "parent/dir2": "bar"}. If you need that granularity, create more granular files targets.

The single-entry dict is a bit odd. Maybe a type path_prefix_mapping=Mapping(from=, to=)?

[stuhood]

I think that determining the connection between the "mapping" and source roots is the main thing to nail down with regard to unblocking rc0. Basically, determining whether:

1. source roots should be stripped from files by default, with the mapping as an "overridden source root" for the file
2. the default should continue to be that files are absolute (ie, their default source root is the buildroot)