A generic "generate" goal for version-controlled generated files #18235
Replies: 3 comments 4 replies
-
|
Native ability to define simple targets that write back to the non-sandboxed filesystem (with built-in up-to-date-ness checking) would be great! Some use-cases I've encountered:
Some small/bikeshed-y thoughts:
|
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
👍
I believe something supporting # whatever.py
from datetime import datetime
import os
import sys
print(os.getcwd())
print(sys.path)
with open("last_run.txt", "w") as f:
f.write(str(datetime.now()))This seems to be how a generator like https://github.com/pantsbuild/pants/blob/99d56537fa8b9020c107ab347e9e6b3416fc52a8/build-support/bin/generate_github_workflows.py is recommended to be used. (I think
Yeah, fully agreed that [cli.alias]
quick-checks = "tailor update-build-files fmt lint check ::"A new |
Beta Was this translation helpful? Give feedback.
-
|
Hmm. I guess my mental model around |
Beta Was this translation helpful? Give feedback.
-
|
Just recording the workaround we use for this for now:
E.g. for a file # path/to/BUILD
shell_command(
name="generate-foo",
command="echo abc > foo-generated.txt",
tools=["echo"],
output_files=["foo-generated.txt"],
)
files(name="foo", sources=["foo.txt"])
run_shell_command(
name="write-foo",
# different file name so that they can be compared in the test
command="cp {chroot}/path/to/foo-generated.txt foo.txt",
execution_dependencies=[":generate-foo"]
)
experimental_test_shell_command(
# if this fails, run `pants run path/to:write-foo`
name="check-foo-is-up-to-date",
command="diff foo.txt foo-generated.txt",
execution_dependencies=[":generate-foo", ":foo"],
) |
Beta Was this translation helpful? Give feedback.
-
|
Now packaged up into a macro: def checked_in_codegen(
*,
name,
output_file = None,
output_file_from_stdout=False,
output_directory = None,
help_text_on_failure,
codegen_target,
codegen_target_fields,
extra_test_target_fields=None,
):
"""A target that packages up code generation and checking it into the repo.
Exactly one of output_file or output_directory must be specified.
This creates various sub-targets. The interesting ones are:
- `{name}`: a file or files target referring to the checked-in file(s) on disk
- `{name}-write`: a `pants run`-able target that runs the code generation and updates the file(s) on disk
- `{name}-is-up-to-date`: a `pant test`-able target that checks the files on disk match the result of re-running codegen.
FIXME: https://github.com/pantsbuild/pants/discussions/18235 this codegen should be maintained
as part of a native goal, e.g. a `save="."` field to `adhoc_tool` or similar.
Fields:
name:
The base name to use (also ends up as the name of the 'file' target for the result of the generation)
output_file:
The name of file that should be saved to the repository (becomes a sibling of the BUILD file
calling this macro). Mutually exclusive with `output_directory`.
output_file_from_stdout:
If True, the `output_file` is created from what the script prints; if False (default), the
`output_file` is created on disk by executing `codegen_target(...)` within its sandbox, and
captured from that. Must not be set when using `output_directory`.
output_directory:
The name of the directory that should be saved to the repository (the _directory_ becomes a
sibling of the BUILD file calling this macro, i.e. any files within the directory are niblings of the
BUILD file, not siblings). Mutually exclusive with `output_file`.
help_text_on_failure:
Informative text to print if the "up to date?" test fails, usually "how to run the codegen generation"
codegen_target:
A Pants target value to run the codegen, usually either `shell_command` or `adhoc_tool`.
codegen_target_fields:
Any fields required for `codegen_target`; used like `codegen_target(..., **codegen_target_fields)`.
extra_test_target_fields:
Any additional fields to pass to the "up to date?" test `test_shell_command` target.
"""
# There's three basic step to this:
#
# 1. doing the codegen
# 2. writing it to disk (on demand)
# 3. verifying that what's on disk is up to date (in CI etc.)
# Step 1: codegen
output_fields = {}
# validate the output configuration makes sense,
# either one output file (potentially from stdout)
if output_file is not None:
if output_directory is not None:
raise ValueError(f"{build_file_dir()}:{name}: expected one of output_file or output_directory, found both {output_file=} and {output_directory=}")
if output_file_from_stdout:
output_fields["stdout"] = output_file
else:
output_fields["output_files"] = [output_file]
output_pathname = output_file
elif output_directory is not None:
if output_file_from_stdout:
raise ValueError(f"{build_file_dir()}:{name}: cannot set output_file_from_stdout when using output_directory")
output_fields["output_directories"] = [output_directory]
# NB. the trailing / is required for the rsync below, to get uniform handling between
# output_file vs. output_directory: suppose we have output_directory="X", with contents
# file.txt (i.e. X/file.txt exists):
#
# - `rsync X Y`, will result in Y/X/file.txt ❌
# - `rsync X/ Y`, will result in Y/file.txt ✅
output_pathname = f"{output_directory}/"
else:
raise ValueError(f"{build_file_dir()}:{name}: expected one of output_file or output_directory, found neither")
raw_codegen_tgt = codegen_target(
name=f"{name}-codegen-raw",
**codegen_target_fields,
**output_fields,
)
raw_codegen = f":{raw_codegen_tgt.name}"
# NB. the codegened file name and what's on disk are the same, but we need to be able to tell
# them apart, for the test, so we put the codegen into a dedicated directory, as an
# implementation detail of this target
codegen_dir = build_file_dir() / "_codegen"
codegen_tgt = relocated_files(
name=f"{name}-codegen",
files_targets=[raw_codegen],
src=str(build_file_dir()),
dest=str(codegen_dir),
)
codegen = f":{codegen_tgt.name}"
# Step 2: writing it to disk
run_shell_command(
name=f"{name}-write",
command=f"""
rsync --verbose --recursive --archive {{chroot}}/{codegen_dir}/{output_pathname} ./{output_pathname}
""",
execution_dependencies=[codegen],
)
if output_file is not None:
assert output_directory is None, "checked above"
checked_in_tgt = file(name=name, source=output_file)
else:
assert output_directory is not None, "checked above"
checked_in_tgt = files(name=name, sources=[f"{output_directory}/**"])
checked_in = f":{checked_in_tgt.name}"
# Step 3: verifying up to date-ness
if extra_test_target_fields is None:
extra_test_target_fields = {}
test_shell_command(
name=f"{name}-is-up-to-date",
tools=["diff"],
command=f"""
if ! diff --unified=1 {{chroot}}/{codegen_dir / output_pathname} {output_pathname}; then
echo
echo '{help_text_on_failure}'
exit 1
fi
""",
execution_dependencies=[codegen, checked_in],
**extra_test_target_fields,
)We use it, for instance, for generating models using https://github.com/koxudaxi/datamodel-code-generator/, so we end up with say: python_requirement(
name="_datamodel-code-generator",
requirements=["datamodel-code-generator==0.31.2"],
# separate resolve to avoid infecting the runtime libraries with this built-time tool's
# dependency constraints
resolve="datamodel-codegen",
)
# FIXME (https://github.com/pantsbuild/pants/issues/20108): running the python_requirement directly
# generally works... except in CI where the "named caches" aren't preserved across runs, so we
# workaround this by reifying an explicit PEX. Once running a requirement directly is reliable, this
# can be removed.
pex_binary(
name="datamodel-codegen",
dependencies=[":_datamodel-code-generator"],
script="datamodel-codegen",
resolve="datamodel-codegen",
layout="packed",
)
file(name="json-schema", source="schema.json")
checked_in_codegen(
name="model-codegen",
output_file="__init__.py",
codegen_target=adhoc_tool,
codegen_target_fields=dict(
runnable=":datamodel-codegen",
args=[
"--input=schema.json",
"--input-file-type=jsonschema",
"--output=__init__.py",
...
],
execution_dependencies=[":json-schema"],
),
help_text_on_failure="❌ Models not matched, run `pants run path/to/here:model-codegen-write` to update",
)This isn't as automatic as would be nice, but it does at least have the tests that run on For common codegen tasks like this, we create aliases, e.g. # pants.toml
[cli.alias]
custom-model-codegen = "run //path/to/here:model-codegen-write" |
Beta Was this translation helpful? Give feedback.
-
|
I would also add that it would be useful to have outputs of tests be output as well. For example some of them run long running computations which I can't rerun all the time to get the outputs. Normally I write them out as one "test" and then have other acceptance tests that just assume the data was already generated locally. (Additionally I version control them with a data version control system (dvc)). |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
For a variety of reasons (many of which could probably be summarized as "legacy"), some generated files need to be committed to version control.
Since these files need to be in version control, it would be great if pants could make it easier to manage them. Here are the goals I have for whatever solution we use:
A survey of current pants goals
goal:
export-codegenAt first blush, codegen seems ideal for making generated files. But codegen in pants is not designed for version controlled files.
When pants generates code, it is considered an internal by-product as described in the PR that introduced the
export-codegengoal:The
export-codegengoal makes these generated files available indist/for those who want to:So, we need a different form of codegen in pants for files that should be version controlled.
goals: lint, fmt, fix
Version controlled files are the bread and butter of
lint,fmt, andfixwhich is opposite ofexport-codegenwhich is for files that are not version controlled.Fixers and formatters also run during the
lintgoal so that lint fails if they need to fix or format a file.Code generation of files that are version controlled is remarkably similar to
fmtandfix. Files that need to be changed are advertised via thelintgoal. Logically, fmt and fix are very different than codegen - but the process, the UX, and the underlying rules seem quite similar to me.In the StackStorm project, I've shoved my (version controlled) file generation under the
fmtumbrella even though it's not really a formatter. Logically this generation is also not a fixer. But, one of the benefits of doing this is that: lint will fail if the generator needs to regenerate the file(s) and it will provide an error message that explains how to run./pants fmt ...to resolve the lint error.I've heard of others that create a "test" that fails if the generated file(s) were not properly regenerated. I really like using
lintfor this to take advantage of an the fine grained caching and reusing the cache for both lint and fmt/fix.goal: generate-lockfiles
Another goal that is very similar to the file generation feature I'm looking for is the
generate-lockfilesgoal. The warnings/errors saying to regenerate can be triggered in many places where rules need to use the lockfiles. So, there is no need to integrate that warning in a lint backend/rule. The goal also provides a fairly ergonomic interface for regenerating those.generate-lockfiles looks up the resolves, translates them into lockfile paths, and then generates the lockfile at that path.
Proposal for a new "generate" goal
If there were a generic "generate" goal, it could go in the opposite direction of
generate-lockfiles: given the path of a lockfile, look up the resolve associated with that lockfile, and then generate the lockfile at that path.Similarly, that "generate" goal could take a path to another generated file, look up owning target for it, and if that target is explicitly for generated files, run the plugin/subsystem/tool that encompasses the codegen logic for that target.
This goal could be only for non-lockfile generated files. If we did include lockfile generation in this goal, then the owning target would probably be a synthetic target synthesized from the config in pants.toml -- so it is explicitly defined (even though not defined in a BUILD file).
This goal should be modeled on fix/fmt (not modeled on export-codegen). This would extend the lint request/result classes (like fix/fmt do) so that file generation happens in a sandbox during lint, failing lint if something should be regenerated. When lint fails here, the error message would prompt the user to run
./pants generate path/to/generated/file/or/dir.For a plugin to provide this generate rule, it would either have to add a custom target, or add a field to an existing target to allow flagging it as owning generated files. This would be a codified assumption built into the
generategoal: all generated files must be owned by exactly 1 file generating target: If unowned, they will not be materialized in the workspace; If owned by more than one file generating targets, raise an error. Generating a file that matches a file glob for that target counts for this.Then, if any other linters can work with a generated files the standard
skip_toolfields provide an escape hatch if the generated code does not need to meet the same standards as other files. Or, for StackStorm, I can add an extra linter specific to the generated file that checks more than just whether or not it needs to be regenerated.Goal: run
A lot of work has gone into runnable targets like
adhoc_toolandshell_command. If combined with the generate goal, those could allow repos to use light-weight run targets and file generator targets instead of writing a medium- or heavy-weight pants plugins.So, a runnable target could be tied to the file generating target such that the adhoc_tool or pex_binary is used to actually do the generation when a user does
./pants generate path/to/generated/file.For the
rungoal, the UX focuses on the generator script/target. For this "generate" goal, the UX focuses on the files to generate. So they are complementary.goal: go-generate
#16909
This seems very similar and could perhaps be rolled into the new "generate" goal. go-generate references the go modules that define the generation. I'm looking at something that targets the generated files. But, often go:generate commands work within the same module where they are defined, so maybe it is effectively the same thing.
But, this is something that would not run via Lint. So, that would probably be a go backend option to skip running for lint is the goals were combined.
discussion
Actually naming the new "generate" goal is a different discussion. When we discuss that, we'll need to make sure it can be distinguished from the codegen that already exists in pants.
So, would a generic generate goal be helpful for anyone else? Does a generate goal that follows a process similar to fix/fmt make sense for other projects?
Beta Was this translation helpful? Give feedback.
All reactions