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

RFE: "stg push --dry-run" versus "stg push --force" #60

Closed
chucklever opened this issue May 6, 2020 · 6 comments
Closed

RFE: "stg push --dry-run" versus "stg push --force" #60

chucklever opened this issue May 6, 2020 · 6 comments
Milestone

Comments

@chucklever
Copy link
Contributor

If there's already a convenient way to do this, please let me know if I'm "doing it wrong". And apologies for being long-winded.

I frequently need to re-order patches in a series. I'm in the zone pushing and popping, and then suddenly I push a patch that doesn't apply cleanly and I realize I can't re-order the series that way without significant changes. But the workspace then needs to be cleaned up because it's full of conflicts, and the patch that didn't apply is now empty. It takes several minutes to hunt down the previous version of the patch, fix up the conflicts, and so on before I can get on with the original task. "stg undo" does not seem capable enough to make this painless (or perhaps I don't understand how to use it).

Instead, I'd like to propose possible tooling changes:

  1. Make "stg undo" behave in a more convenient way -- instead of leaving the conflicts, can it actually undo things to the pre-push state and leave both the patch and the workspace in clean condition?
  2. Add the ability to try out a push instead of actually performing it. Call it "stg try" or "stg push --dry-run", which would attempt to apply the patch and report either "the conflicts would be here and here" or "that will work fine, go ahead for real".
  3. Or, make "stg push" always behave transactionally: if there are conflicts, then it simply fails and leaves the series in its pre-push state (I believe "stg import" behaves this way).
  4. Or, make the default behavior of "stg push" transactional as above, as that as less surprising, but then add a new option to "stg push" that retains the current "just do it and I'll clean up the mess" behavior; say "stg push --force". This possibly adds a step or two when rebasing introduces conflicts in a series, so rebase could continue using "stg push --force".

As always, thanks very much for your efforts!

@jpgrayson
Copy link
Collaborator

Hi @chucklever. These are good questions and, I think, a common use case.

Perhaps the missing piece in your workflow is stg undo --hard. In the case you describe, when stg push some-patch results in conflicts and the proper resolution is to totally undo the push attempt (and not resolve the conflicts), using stg undo --hard will put your stack and working tree back in the state it was in before the failed push attempt.

I think that equipped with stg undo --hard, you may be all set. Please let me know if that is not the case or if there is something else I'm missing.

Regarding the possibility of changing the default behavior of stg push to be transactional, i.e. doing the equivalent of stg push "$@" || stg undo --hard, my initial reaction is no because it is very normal workflow to push a patch, have and repair conflicts, and refresh the patch with the conflicts repaired.

If we were to add an option to stg push, it would be what you call --dry-run, except I don't think that's the right name since "dry-run" very strongly implies that stack state will not change. Another possible name would be --avoid-conflict or --no-conflict.

@chucklever
Copy link
Contributor Author

I absolutely agree that "pushing then repairing" is a necessary function. I'm simply claiming that should not be the default behavior of "stg push", because:

  • the current behavior can be surprising and temporarily destructive
  • the usual UI pattern, in that case, is to need "--force" to go ahead and be destructive
  • "stg push" is inconsistent with "stg import" which is always nondestructive

So it seems like "stg push" should be transactional, and "stg push --force" (or something like it) should be used for the current "push then repair" behavior. And I realize that could be a significant and surprising workflow change for long-time stgit users such as myself.

Note: I'm not being religious, just scratching my chin and squinting. I am happy to give "stg undo --hard" a try, and I would be happy with a dry-run behavior no matter what it is called.

One alternative might be to add "--transactional" and "--force" options to "stg push", and then introduce a config option to make one or the other the default behavior. Or possibly I could create an alias for "stg push --no-conflict" and just use that alias every day.

Thanks for your time and attention.

@lkraav
Copy link

lkraav commented May 6, 2020

I admit, stg push has had me grunting at it more than once. Just not often enough to file this issue.

Not sure I ever realized stg undo --hard could get me out of a failed push bind. Does it explicitly suggest such when a patch fails 🤔

@jpgrayson
Copy link
Collaborator

This discussion is really helpful. My takeaway is that stg push could benefit from one or more usability improvements. The menu of options thus far:

  • No push conflicts by default, --force option to override.
    • Perhaps accompanied by a stgit.push.conflict-policy config option to allow users to emulate the current default behavior.
  • Continue to allow push conflicts by default, --no-conflict option to override.
  • Add more user-facing output to help with what to do when push conflicts are encountered. E.g.:
    • Push failed due to conflicts. Push again with --force if you would like to resolve the conflicts.
    • Push succeeded with conflicts. Resolve the conflicts and refresh, or use 'stg undo --hard' to revert to the previous stack and work tree state.

@chucklever
Copy link
Contributor Author

I like the addition of the enhanced conflict reports.

@jpgrayson jpgrayson added this to the 2.1 milestone Oct 24, 2022
jpgrayson added a commit that referenced this issue Dec 4, 2022
@jpgrayson
Copy link
Collaborator

@chucklever @lkraav

I think I've got this reasonably fixed in d7322f6.

For the behavior I think you want:

  1. Set the new stgit.push.allow-conflicts configuration variable to false.
  2. Run stg push, stg goto, etc. normally.
  3. When you want to deal with conflicts and conflict resolution, run stg push --conflicts.

I've left the default behavior unchanged.

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

No branches or pull requests

3 participants