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

Actually run deprecated targets fixer (Cherry-pick of #18860) #18893

Merged
merged 1 commit into from May 3, 2023

Conversation

huonw
Copy link
Contributor

@huonw huonw commented May 3, 2023

This fixes #18509 by updating update-build-files's maybe_rename_deprecated_targets rule to run the deprecated target fixers: previously, that rule was accidentally rerunning the deprecated fields fixers and deprecated targets weren't being fixed. This fix ensures experimental_shell_command is switched to shell_command and experimental_run_shell_command is switched to run_shell_command.

As part of this, I rewrote renamed_targets_rules.py to match the current phrasing of renamed_fields_rules.py: without doing that, I was getting graph errors. I suspect there was smaller rewrites that would've worked, but it didn't seem unreasonable to have both the renamed_..._rules.py files basically match from top to bottom, with only the type names and transformation details differing.

This doesn't do the update to workdir="/" for run_shell_command that I flagged in #18509 (comment). With the current token-based rewrites, doing that seems like a lot of work, although, it would be possible (do something similar to renamed_fields_rules.py matching parens/brackets and iterate looking for a workdir=... field, and updating/inserting it, all packaged up to be vaguely reusable).

This fixes pantsbuild#18509 by updating `update-build-files`'s
`maybe_rename_deprecated_targets` rule to run the deprecated target
fixers: previously, that rule was accidentally rerunning the deprecated
_fields_ fixers and deprecated targets weren't being fixed. This fix
ensures `experimental_shell_command` is switched to `shell_command` and
`experimental_run_shell_command` is switched to `run_shell_command`.

As part of this, I rewrote `renamed_targets_rules.py` to match the
current phrasing of `renamed_fields_rules.py`: without doing that, I was
getting graph errors. I suspect there was smaller rewrites that would've
worked, but it didn't seem unreasonable to have both the
`renamed_..._rules.py` files basically match from top to bottom, with
only the type names and transformation details differing.

This _doesn't_ do the update to `workdir="/"` for `run_shell_command`
that I flagged in
pantsbuild#18509 (comment).
With the current token-based rewrites, doing that seems like a lot of
work, although, it would be possible (do something similar to
`renamed_fields_rules.py` matching parens/brackets and iterate looking
for a `workdir=...` field, and updating/inserting it, all packaged up to
be vaguely reusable).
@huonw huonw added the category:bugfix Bug fixes for released features label May 3, 2023
@huonw huonw requested a review from thejcannon May 3, 2023 21:46
@huonw huonw merged commit f319738 into pantsbuild:2.16.x May 3, 2023
17 checks passed
@huonw huonw deleted the cherry-pick-18860-to-2.16.x branch May 3, 2023 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants