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

Add --console-script option to the run goal. #13257

Closed
wants to merge 1 commit into from

Conversation

kaos
Copy link
Member

@kaos kaos commented Oct 14, 2021

This allows you to run any console scripts provided by plugins. Example:

diff --git a/pants.toml b/pants.toml
index b4e2d739b..0656e547c 100644
--- a/pants.toml
+++ b/pants.toml
@@ -22,6 +22,7 @@ backend_packages.add = [
   "internal_plugins.releases",
 ]
 plugins = [
+  "cowsay",
   "hdrhistogram",  # For use with `--stats-log`.
   # NOTE: Keep this version in sync with `generate_docs.py`!
   "toolchain.pants.plugin==0.14.0",
$ ./pants run --console-script=cowsay -- mooo
  ____
| mooo |
  ====
    \
     \
       ^__^
       (oo)\_______
       (__)\       )\/\
           ||----w |
           ||     ||

This is a useful addition for plugin authors to be able to add custom functionality to the pants cli in an easy way. Or, to install generic admin tools with pants..

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
class RunConsoleScriptRequest:
console_script: str


class RunSubsystem(GoalSubsystem):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The run goal is generic, so by adding a python backend feature directly to it your saying it's ok to add a JavaScript one, a ruby one, etc as those backend get built. Can't this already be accomplished with a pex_binary target that has a console script entry point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't rely on the python backend, it relies on the fact that pants is a python app. You wouldn't have javascript or ruby etc plugins to pants (afaict).

I was just considering suggesting the arg be called just --script rather than --console-script though...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This complements the extraordinary power we get with script on pex_binary. (but I almost made the mistake of going down the road of supporting calling scripts from 3rdparty deps this way...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so perhaps the request class could be better named, like RunPluginScriptRequest ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still failing to see how you couldn't accomplish this with a pex_binary target that has a 3rdparty dep and a script entry point today. Do you have a real example at hand to help clarify how that doesn't work or works but is more awkward, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've learned quite a bit about the possibilities with console scripts (and the pex_binary script field) today, so can definitely see that the alternative isn't that far off.

Thanks for the input. :)

Copy link
Contributor

@jsirois jsirois Oct 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, making more sense. What's an example of a plugin script though? You're adding a new ad-hoc plugin mechanism here (tacking it on the run goal) that appears to allow for a plugin to provide scripts that do arbitrary but un-integrated things.

If you do have useful examples, then the next question is is this the right way? It's not discoverable. Instead you'd probably want a 1st class way to register these with Pants like you do rules. At that point Pants could display help for these things and allow you to run them in a less ad-hoc way, perhaps simply via pants ~script-name (the syntax example is completely unimportant, the thrust is making this facility 1st class if it's truly useful).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. Yes, it does work using the 3rdparty dep and pex_binary route. The awkwardness is simply that if I already have it listed in plugins, it is there ready to be used. Without this, I'd have to add some targets to a BUILD file to get there. This is boilerplate required per script I want to call. As a plugin author, it would be a hassle to ask users to add this to their BUILD files in order to access the added tooling.

Granted, the overhead isn't much, but it adds BUILD scaffolding noise that is avoidable, as well as keeping the configuration DRY, as otherwise, any version constraints will be duplicated (not in conflict though, you could use two different versions).

So, this is more of a nice feature, than required to get where I want..

And with synthetic targets, I think this point is pretty much moot by now, as the plugin might register its own set of "provided" pex_binary targets that can be used right away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's an example of a plugin script though?

IIRC this idea was based on having a company wide pants plugin making pants the main executable to integrate with a lot of custom in-house tooling and other stuff related to development without having to maintain and update each project as the plugin evolves..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, I wonder why that would not just be another goal...

@kaos
Copy link
Member Author

kaos commented Oct 14, 2021

I think the command line flag for this would be better as the shorter --script (this also reflects the pex_binary field name for similar purpose).

@kaos
Copy link
Member Author

kaos commented Oct 14, 2021

Closing in favour of using pex_binary targets, and running them. With the new alias support, that makes it as good as this (only a bit more configuration/setup, but a small price to pay, all things considered.)

Some example tidbits for future reference...

# pants.toml or .pantsrc
[cli.alias]
do = "--no-pantsd run :invoke --"
django-admin = "run :django-admin --"
# root BUILD file (or any other BUILD file, adjust the aliases accordingly)
python_requirement(
    name="django",
    requirements=["Django"]
)

pex_binary(
    name="django-admin",
    script="django-admin",
    dependencies=[":django"],
)

python_requirement(
    name="pyinvoke",
    requirements=["invoke"],
)

pex_binary(
    name="invoke",
    script="invoke",
    dependencies=[":pyinvoke"],
)

This allows us to call into these tools:

$ ./pants do --list
Available tasks:

  hello

$ ./pants django-admin --version
3.2.8

@kaos kaos closed this Oct 14, 2021
@kaos kaos deleted the run_console_script branch October 14, 2021 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants