Add `[cli.alias]` config section for command line alias support. #13228

kaos · 2021-10-12T11:42:53Z

Pre-work for user friendly ad-hoc script execution support.
slack thread

Included in this PR is an example alias for running pyupgrade, as well as green and all-changed aliases:
Example use to run on the docker backend:

$ ./pants pyupgrade 'src/python/pants/backend/docker/**/*.py'
13:41:45.74 [INFO] Initialization options changed: reinitializing scheduler...
13:41:46.38 [INFO] Scheduler initialized.
13:41:49.67 [INFO] Completed: Format with Autoflake - autoflake made no changes.
13:41:51.98 [INFO] Completed: Format with Black - Black made no changes.
All done! ✨ 🍰 ✨
33 files left unchanged.


13:41:53.93 [INFO] Completed: Format with docformatter - Docformatter made no changes.
13:41:54.43 [INFO] Completed: Format with isort - isort made no changes.
13:41:56.41 [INFO] Completed: Format with pyupgrade - pyupgrade made no changes.

✓ Black made no changes.
✓ Docformatter made no changes.
✓ autoflake made no changes.
✓ isort made no changes.
✓ pyupgrade made no changes.

By adding this to pants.toml:

[cli.alias]
pyupgrade = "--backend-packages=pants.backend.python.lint.pyupgrade fmt"

So this is generally useful already on its own.
I have not run any benchmarks to see how big impact this will have on pants startup times.

UPDATED

Added checks for alias names, so they can not be confused with potential options or dir specs. Also verifies that an alias does not shadow an existing goal/subsystem. Thanks @chrisjrn for pointing that out.

Added debug logging of alias expansion, to aid troubleshooting. I haven't looked, but if there is a verbose flag, perhaps that log could be made an info level when verbose==true.

Also supports nesting aliases. This will be useful to build up aliases in incremental steps, while still keeping them DRY.

Such as this example:

[cli.alias]
all-changed = "--changed-since=HEAD --changed-dependees=transitive"
all-green = "all-changed green"
green = "fmt lint check"

Signed-off-by: Andreas Stenius <andreas.stenius@svenskaspel.se> # 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]

chrisjrn · 2021-10-12T15:01:16Z

First of all, this is just a wonderful idea. Thank you!

Secondly, have you given thought to how you might deal with new goals introduced in Pants that clash with an alias that's already defined? (my first thoughts are this should error, but I haven't thought much about it)

Eric-Arellano

This is SO cool! 🎉

src/python/pants/option/global_options.py

src/python/pants/option/alias.py

src/python/pants/option/global_options.py

src/python/pants/option/alias.py

benjyw · 2021-10-12T15:36:39Z

First of all, this is just a wonderful idea. Thank you!

Secondly, have you given thought to how you might deal with new goals introduced in Pants that clash with an alias that's already defined? (my first thoughts are this should error, but I haven't thought much about it)

This is a good question. You could clash even on an existing goal, right? I guess this is a "there be dragons" situation, and we document that you have to be careful...

chrisjrn · 2021-10-12T15:39:30Z

You could clash even on an existing goal, right? I guess this is a "there be dragons" situation, and we document that you have to be careful...

I suspect erroring completely would be better behaviour, unless explicitly silenced – better to know that something's wrong and fix it, than having weird and hard-to-debug weirdnesses that nobody else can make sense of

benjyw · 2021-10-12T15:48:52Z

Yeah, I guess we should:

A) Validate that aliases don't contain slashes or dashes (so they cannot collide with an option or a spec)
B) Validate that they don't collide with known goal names.

[ci skip-rust] [ci skip-build-wheels]

# 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]

src/python/pants/option/alias.py

benjyw

Users are going to absolutely love this! This is definitely worth a blog post.

Eric-Arellano

Amazing 🎉

Eric-Arellano · 2021-10-13T15:22:04Z

src/python/pants/build_graph/build_configuration.py

@@ -31,7 +32,7 @@


 # Subsystems used outside of any rule.
-_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed}
+_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed, CliOptions}


@stuhood @benjyw is this fine? I thought for some reason we wanted to chip away at this list.

Maybe, but I don't recall why. If anything, I think that there are a lot of potential usecases for breaking out our bootstrap and global options into more global subsystems.

src/python/pants/option/alias.py

# 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]

stuhood

Great idea: thanks!

stuhood · 2021-10-13T15:58:12Z

pants.toml

+
+[cli.alias]
+all-changed = "--changed-since=HEAD --changed-dependees=transitive"
+green = "fmt lint check"


Let's hold off on adding this one... it has bikeshed potential due to the ordering of the commands.

stuhood · 2021-10-13T15:59:48Z

src/python/pants/build_graph/build_configuration.py

@@ -31,7 +32,7 @@


 # Subsystems used outside of any rule.
-_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed}
+_GLOBAL_SUBSYSTEMS: Set[Type[Subsystem]] = {GlobalOptions, Changed, CliOptions}


Maybe, but I don't recall why. If anything, I think that there are a lot of potential usecases for breaking out our bootstrap and global options into more global subsystems.

stuhood · 2021-10-13T16:04:41Z

src/python/pants/option/options_bootstrapper.py

+            # Finally, we expand any aliases and re-populates the bootstrap args, in case there was
+            # any from an alias.
+            alias = CliAlias.from_dict(post_bootstrap_config.get("cli", "alias", dict, {}))


It's possible that this will break the rust client when aliases are used, since it also does options parsing: #11922 ... but maybe only if bootstrap options are defined in the alias? Probably not a blocker.

I have made some headway on porting options parsing to fully consuming that backend, but haven't been able to make a push to land it yet.

Note that #21695 ports the CLI aliasing feature to Rust, so it will be available to the rust client.

stuhood · 2021-10-13T16:07:01Z

src/python/pants/option/options_bootstrapper.py

+        flags = set()
+        short_flags = set()
+
+        def capture_the_flags(*args: str, **kwargs) -> None:
+            for arg in args:
+                flags.add(arg)
+                if len(arg) == 2:
+                    short_flags.add(arg)
+                elif kwargs.get("type") == bool:
+                    flags.add(f"--no-{arg[2:]}")
+
+        GlobalOptions.register_bootstrap_options(capture_the_flags)


This registration will now run each time this method is called: should consider moving it into a dataclass to be used for both calls in OptionsBootstrapper.create.

Just as a refactoring, to get the method out of the OptionsBootstrapper?

Note: I commented on specific lines in here.

Mostly for performance reasons. _get_bootstrap_args is called twice, and so will re-register the bootstrap options twice, which will render silenced deprecation warnings twice, etc.

Ah, but the alias may expand to a bootstrap arg, so we need to visit them again.. will look a bit closer at this..

Sure: but look at the specific section I commented on: it's only the portion that captures the flags and short-flags. So that part can be reused I think?

This isn't a huge deal: don't worry too much about it.

No, it's fine, and I see my confusion now, the args in the closure is shadowing the args from the method. Will fix.
I.e. I came to the wrong conclusion for my last comment.

stuhood · 2021-10-13T16:10:33Z

src/python/pants/option/alias.py

+                "Notice: this option must be placed in a config file (e.g. `pants.toml`) to have "
+                "any effect."


Based on the implementation, it's not clear to me that this is true (although it would be pretty pointless to define aliases on the CLI): they'll be ignored during the bootstrap pass, then loaded from the bootstrap args, which includes CLI args?

It also looks like these should work in pantsrc files, so maybe worth mentioning that: users are likely to want to define their own aliases.

Agree with the pantsrc, mentioning it too.

The aliases are loaded from post_bootstrap_config which is a Config.load(), with only config files and bootstrap options, so any options for the cli subsystem from a command line flag won't ever be in there, right?

Ah, you're right. Should be fine... can possibly move where it is constructed later if it ends up mattering, but it likely won't.

[ci skip-rust] [ci skip-build-wheels]

# 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]

kaos · 2021-10-13T19:09:28Z

There's a TODO for a typing issue with memoized_classmethod in here now, tracked by #13244.

kaos · 2021-10-13T20:33:46Z

I'll address any remaining comments in a follow up PR, if there are any.

…tsbuild#13228) This allows you to run pants with commands that are easier to remember, even if there are long or many options involved. The following two invocations are equivalent, given the pants configuration below: ``` $ ./pants all-changed fmt $ ./pants --changed-since=HEAD --changed-dependees=transitive fmt ``` `pants.toml`: ``` [cli.alias] all-changed = "--changed-since=HEAD --changed-dependees=transitive" ``` # 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]

kaos requested review from stuhood, benjyw and Eric-Arellano October 12, 2021 11:42

kaos mentioned this pull request Oct 12, 2021

Add run support for experimental_run_shell_command targets. #13229

Merged

Eric-Arellano approved these changes Oct 12, 2021

View reviewed changes

src/python/pants/option/global_options.py Outdated Show resolved Hide resolved

src/python/pants/option/global_options.py Outdated Show resolved Hide resolved

src/python/pants/option/alias.py Show resolved Hide resolved

benjyw reviewed Oct 12, 2021

View reviewed changes

src/python/pants/option/global_options.py Outdated Show resolved Hide resolved

benjyw reviewed Oct 12, 2021

View reviewed changes

src/python/pants/option/alias.py Outdated Show resolved Hide resolved

kaos changed the title ~~Add [GLOBAL.alias] config section for command line alias support.~~ Add [CLI.alias] config section for command line alias support. Oct 13, 2021

kaos added 5 commits October 13, 2021 09:19

Merge remote-tracking branch 'upstream/main' into cli_alias_command_line

3dae2cc

[ci skip-rust] [ci skip-build-wheels]

Review feedback.

0edbc11

# 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]

Support nested aliases.

12666dd

# 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]

Validate alias names to avoid possible conflicts with options/specs.

0234b70

# 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]

Throw error if alias shadows existing goal/subsystem.

4a75b55

# 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]

kaos changed the title ~~Add [CLI.alias] config section for command line alias support.~~ Add [cli.alias] config section for command line alias support. Oct 13, 2021

Add help text to cli subsystem.

5633907

# 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]

Eric-Arellano reviewed Oct 13, 2021

View reviewed changes

src/python/pants/option/alias.py Show resolved Hide resolved

benjyw approved these changes Oct 13, 2021

View reviewed changes

Eric-Arellano approved these changes Oct 13, 2021

View reviewed changes

Review feedback.

2e6b642

# 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]

kaos enabled auto-merge (squash) October 13, 2021 15:45

Eric-Arellano approved these changes Oct 13, 2021

View reviewed changes

stuhood approved these changes Oct 13, 2021

View reviewed changes

Eric-Arellano disabled auto-merge October 13, 2021 16:12

kaos added 2 commits October 13, 2021 18:20

Merge remote-tracking branch 'upstream/main' into cli_alias_command_line

ae28587

[ci skip-rust] [ci skip-build-wheels]

Review feedback from stu. Thanks!

1a42c89

# 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]

kaos merged commit 2714522 into pantsbuild:main Oct 13, 2021

kaos deleted the cli_alias_command_line branch October 13, 2021 20:37

kaos mentioned this pull request May 5, 2023

Support flag-like CLI aliases #18912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `[cli.alias]` config section for command line alias support. #13228

Add `[cli.alias]` config section for command line alias support. #13228

kaos commented Oct 12, 2021 •

edited

Loading

chrisjrn commented Oct 12, 2021

Eric-Arellano left a comment

benjyw commented Oct 12, 2021

chrisjrn commented Oct 12, 2021 •

edited

Loading

benjyw commented Oct 12, 2021

benjyw left a comment

Eric-Arellano left a comment

Eric-Arellano Oct 13, 2021

stuhood Oct 13, 2021 •

edited

Loading

stuhood left a comment

stuhood Oct 13, 2021

stuhood Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

benjyw Nov 27, 2024

stuhood Oct 13, 2021

kaos Oct 13, 2021

stuhood Oct 13, 2021

kaos Oct 13, 2021

stuhood Oct 13, 2021 •

edited

Loading

kaos Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

kaos Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021

kaos commented Oct 13, 2021

kaos commented Oct 13, 2021

		"Notice: this option must be placed in a config file (e.g. `pants.toml`) to have "
		"any effect."

Add [cli.alias] config section for command line alias support. #13228

Add [cli.alias] config section for command line alias support. #13228

Conversation

kaos commented Oct 12, 2021 • edited Loading

UPDATED

chrisjrn commented Oct 12, 2021

Eric-Arellano left a comment

Choose a reason for hiding this comment

benjyw commented Oct 12, 2021

chrisjrn commented Oct 12, 2021 • edited Loading

benjyw commented Oct 12, 2021

benjyw left a comment

Choose a reason for hiding this comment

Eric-Arellano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuhood Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuhood Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

stuhood Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuhood Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

kaos Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

stuhood Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

kaos Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaos commented Oct 13, 2021

kaos commented Oct 13, 2021

Add `[cli.alias]` config section for command line alias support. #13228

Add `[cli.alias]` config section for command line alias support. #13228

kaos commented Oct 12, 2021 •

edited

Loading

chrisjrn commented Oct 12, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

kaos Oct 13, 2021 •

edited

Loading

stuhood Oct 13, 2021 •

edited

Loading

kaos Oct 13, 2021 •

edited

Loading