Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make it easier to interpolate artifact names #18659

Closed
cburroughs opened this issue Apr 3, 2023 · 4 comments
Closed

make it easier to interpolate artifact names #18659

cburroughs opened this issue Apr 3, 2023 · 4 comments
Assignees

Comments

@cburroughs
Copy link
Contributor

cburroughs commented Apr 3, 2023

Is your feature request related to a problem? Please describe.

Right now it is easy to tell pants to "package all the things" with pants package ::. This fills up dist with all of the goodies. However, instead of having foo.tar.gz I'd like to have foo-VERSION.tar.gz so that everything is ready for uploading publishing. (Where version is maybe a constant like v.1.2.3 or just the git sha.)

output_path can almost do that, but then I have to re-implement the / --> . logic.

output_path
    type: str | None
    default: None

    Where the built asset should be located.
    
    If undefined, this will use the path to the BUILD file, followed by the target name. For example, `src/python/project:app`
    would be `src.python.project/app.ext`.
    
    When running `pants package`, this path will be prefixed by `--distdir` (e.g. `dist/`).
    
    Warning: setting this value risks naming collisions with other package targets you may have.

Describe the solution you'd like
Give all "just a file" artifacts an output_name to go along with output_path, the "path to output" logic then looks something like:

  • default: build_file_dir.replace('/', '.') + name
  • if output_name is set: build_file_dir.replace('/', '.') + output_name
  • if output_path is set: `output_Path

Describe alternatives you've considered

  • set the target name to something dynamic like "foo + git_sha". Quick and dirty but then the name changes on every commit.
  • Write a versioned_tarball macro.

Additional context
Doing it with a macro isn't that bad. But I feel like there is some fundamental inconsistency where some "packages" like docker or python_distributions have a separate "version", but others do not.

spinoff from: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1680240146011409

@tdyas tdyas self-assigned this Jul 15, 2024
@tdyas
Copy link
Contributor

tdyas commented Jul 15, 2024

The challenging part of this request is probably the migration path for users who are setting the path and filename using output_path. If the semantics for output_path change to only be the path, then the output filename for existing users will change if they do not also update output_name.

Alternative solution: Keep only the output_path field, but define string replacements for the normalized directory ("spec path" in Pants parlance) and normalized target name. The default then becomes something like "{normalized_spec_path}/{normalized_target_name}" but with some provision made for the file suffix (e.g., .pex).

@tdyas
Copy link
Contributor

tdyas commented Jul 15, 2024

@cburroughs: #21175 is a draft solution. Let me know if the shape of it works for you.

@cburroughs
Copy link
Contributor Author

The template approach looks reasonable to me, and I like having fewer fields. Thank you.

tdyas added a commit that referenced this issue Jul 17, 2024
…21175)

As described in #18659, when a
user sets `output_path` on a packageable target, the user gives up the
default behavior which uses a normalized version of the target's
directory as the directory into which the place the built artifact.
While the user could replicate the behavior manually, it would be a
better user experience if the user can still have partial access to some
of the default behavior.

This PR teaches the `output_path` field how to derive its value from a
template instead of hard-coding its behavior in Python. The
`output_path` field learns the following template replacements:

- `spec_path_normalized`: The path to the target's directory ("spec
path") with forward slashes replaced by dots.
- `target_name_normalized`: The target's name 
- `file_suffix`: For target's which produce single file artifacts, this
is the file type suffix to use. This is empty if not applicable.

The default value for the `output_path` field is now
`"${spec_path_normalized}/${target_name_normalized}${file_suffix}"`
which replicates the existing behavior.

`string.Template` is used as the template engine.

This PR also fixes some incorrect usages of `output_path` in several
backends.
@tdyas
Copy link
Contributor

tdyas commented Jul 17, 2024

Implemented by #21175.

@tdyas tdyas closed this as completed Jul 17, 2024
tobni pushed a commit to tobni/pants that referenced this issue Jul 22, 2024
…antsbuild#21175)

As described in pantsbuild#18659, when a
user sets `output_path` on a packageable target, the user gives up the
default behavior which uses a normalized version of the target's
directory as the directory into which the place the built artifact.
While the user could replicate the behavior manually, it would be a
better user experience if the user can still have partial access to some
of the default behavior.

This PR teaches the `output_path` field how to derive its value from a
template instead of hard-coding its behavior in Python. The
`output_path` field learns the following template replacements:

- `spec_path_normalized`: The path to the target's directory ("spec
path") with forward slashes replaced by dots.
- `target_name_normalized`: The target's name 
- `file_suffix`: For target's which produce single file artifacts, this
is the file type suffix to use. This is empty if not applicable.

The default value for the `output_path` field is now
`"${spec_path_normalized}/${target_name_normalized}${file_suffix}"`
which replicates the existing behavior.

`string.Template` is used as the template engine.

This PR also fixes some incorrect usages of `output_path` in several
backends.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants