feat: --retry [<COUNT>] flag and --no-retry deprecation

[uwuclxdy]

What does this PR do

Standards checklist

• The PR title is descriptive
• I have read CONTRIBUTING.md
• Optional: I have tested the code myself
• If this PR introduces new user-facing messages they are translated

---

This PR enhances the --no-retry flag by implementing --retry [<COUNT>] flag to enable automatic retries of failed steps. --no-retry was kept to stay compatible with old scripts and is equivalent to --retry 0. It also supports permanent configuration from the config file:

# retry = true
# retry_count = 1

How it works

if a step fails:

1. topgrade (no flag, no config.toml entry):
   • user will be prompted to do like it was before
2. topgrade --retry 0 (or set in the config):
   • the step will be automatically skipped
3. topgrade --retry 5 (or set in the config):
   • the step will be retried 5 times without user confirmation and automatically continue to the next step

Feedback needed

Should this be made to work with -y flag (eg. automatically continue after retrying if -y is set, otherwise prompt the user)?

[GideonBear]

It doesn't seem like a good idea to do unattended retries to me. If a step fails, it's expected for the user to do something. Do you have a situation where it is desired to do unattended retries? This seems a bit like an XY-problem.

[uwuclxdy]

this is meant for fixing issues which are usually resolved after retrying (for example unstable internet connection which causes a step to fail and pass on 2nd retry for example). of course default behavior hasn't changed and user is still prompted what they want to do if --retry is not set. PR implements this for only the people who need it.

[GideonBear]

Okay, network conditions is fair; not every tool has this built-in. But don't we want "ask after 2 retries", instead of "skip after 2 retries"? Or possibly both?

[uwuclxdy]

yes, this would make a lot of sense. i was thinking about asking after all retries fail by default, skip only if a flag such as -y is also set.

[GideonBear]

skip only if a flag such as -y is also set.

FYI: I don't think it's a good idea to use -y for Topgrade internals at all; it's only for passing through to commands.

[uwuclxdy]

agree + -y currently doesn't skip failed steps. i made --no-retry work with --retry so it first retries [count] amount of times, then continues if --no-retry is also set. the naming is somewhat confusing and would appreciate a suggestion on how to name the extra flag (or rename --no-retry, remove from help and only keep an alias for it in code so legacy scripts continue to work). the new parameter could be named something like --skip-failed ig

[GideonBear]

(I was thinking --retry=no, --retry=ask, --retry=5 but that doesn't work because --retry=5 doesn't specify whether to ask or skip after all 5 retries fail.)

I agree --skip-failed is a more sensible name for what is currently called --no-retry, when we add automatic retries. --no-retry can indeed be an undocumented alias (maybe mention --no-retry in the description for --skip-failed) and then we can remove that in v17. But maybe there's an even better name, like --no-ask-retry maybe? --skip-failed still seems a bit confusing in combination with --retry=5.

One thing I don't understand is your definition of the retry count. You're using --retry=1 as default, meaning ask immediately after failure; but the first try is not a retry by definition, I would expect --retry=1 to mean 2 tries in total (1 retry), and the default to be --retry=0. Do you disagree?

[uwuclxdy]

alright I'll make --no-retry undocumented alias for --no-ask-retry flag (name can be changed if needed later).

as for --retry=1, this is equal to --retry, which will retry once. so running without a number of retries specified defaults to 1 retry. this should all be documented but don't know in how much detail should we go to not make it too bloated.

examples:
--retry / --retry 1 -> fail, retry once, ask
--retry 5 -> fail, retry 5 times, ask

--no-ask-retry auto continues instead of asking

[uwuclxdy]

also (i missed that), yes maybe my code is poorly written but --retry 1 will run a step twice if it fails.

this happens here: 5c21ae6#diff-68c6f449503a33c74a15800c0a9c8fd34028465a06edcac92b32bbb9db64bbceR80

edit: --retry 0 means skip without asking.... which kinda eliminates the need for --no-ask-retry? i somehow forgot about that lol. need to document it and just make --no-retry undocumented, as it equals to --retry 0 already.
https://github.com/uwuclxdy/topgrade/blob/5c21ae62316e0ee1b6dbbea05126592e1908eb97/src/config.rs#L1112

[uwuclxdy]

this is the behaviour as of the latest commit in this PR:

• retry = false (or unset) -> ask on failure
• retry = true -> auto retry failed steps (if retry_count not set, retry once)
• retry = true + retry_count = 0 or no_retry = true -> don't retry, skip failed steps and continue
• retry = true + retry_count = 5 -> retries a failed step 5 more times

in cli:

• unset -> ask (the same as retry = false or unset)
• --retry -> auto retry failed steps (if retry_count not set, retry once)
• --retry 0 -> don't retry, skip failed steps and continue
• --retry 5 -> retries a failed step 5 more times

now, i updated the example config file idk if it's descriptive enough tho

[GideonBear]

Is there a reason to have a default for --retry? I feel like it only makes it more confusing.

yes maybe my code is poorly written but --retry 1 will run a step twice if it fails.

Oh okay, I just skimmed your code and probably misunderstood.

I'm not really a fan of the disparity between the config file and the CLI in your explanation. I had the following in mind:

• ask_retry = true/false (default true) and --no-ask-retry: if false, do not pause to ask for user input after failure/after automatic retries
• auto_retry: <some number> (default 0) and --auto-retry=<some number>: the amount of times to automatically retry a step when it fails. After failing X+1 times, will resume like normal (go to next step if ask_retry = false, and ask if ask_retry = true)

I changed retry -> auto-retry because it makes it more clear that it's about automatic retries, seems better to me when not overloading --retry with other meaning.
stop_on_error (--no-stop-on-error) might also be a good (better?) name for ask_retry.

Feel free to argumentate your current proposal, it just doesn't seem very intuitive to me.

[uwuclxdy]

your reasoning for this is actually way better than what i could come up with, so this is how it works now:

config	CLI flag	description	default
ask_retry = true/false	--no-ask-retry	prompt user on failure	true
auto_retry = <number>	--auto-retry=<COUNT>	number of automatic retries per failed step	0
no_retry	--no-retry	hidden alias for --no-ask-retry

• when a step fails, auto retry auto_retry times
• after failing a step for auto_retry + 1 times:
  • ask_retry = true -> prompt the user
  • ask_retry = false -> automatically continue