RFR: Improve "git sync" and "git fork" resilience

[Kevin Rushforth]

On Thu, 30 Sep 2021 13:57:52 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

> The Skara CLI tools `git sync` and `git fork` are very much appreciated, but they are not fully designed to make sure you get the intended result. Part of this problem is that they are too general, and part is that they are too dumb. :-)
> 
> These changes make sure that `git sync` always syncs between your personal fork, and the upstream repository (determined by the Git Forge provider as the "parent" repo you created your fork from), and `git fork` always sets up your repository for such `git sync` usage.
> 
> If you really know what you are doing, you *can* still use `git sync` to sync between fully unrelated repos using the `--to` and `--from` arguments, but this is highly discouraged (and will require the `--force` flag as well, to be accepted).
> 
> Before syncing, `git sync` will check that you have a proper `origin` and `upstream` remote configured, and that the upstream is consistent with what the Git Forge provider says. If the `upstream` remote is missing, it will be configured for you (unless `--no-remote` is given).
> 
> `git fork` will now always set the `upstream` remote (unless given `--no-remote`).
> 
> If these sounds like obvious behavior, it is worth noting what happened before. If `upstream` was missing, `git sync` had no idea what to do. `git fork` had a second, undocumented *) mode, in which you could run `git fork` without any arguments in a directory that already contained a checked out repo. That would create a personal fork of that repo on the Git Forge provider, but it would still keep `origin` as the upstream repo URI, and instead add a `fork` remote for the newly created personal fork. This mode of operation was barely supported (and broken) on `git sync`. The backwards way of naming the remotes could lead to no end of troubles. 
> 
> *) I say "undocumented" since there is no explicit documentation about this feature that I could found. However, the "upstream URI" argument to `git fork` is documented as "optional" (but all other documentation assumes that it is present, so the behavior when omitted is not specified).
> 
> When implementing these fixes, it was clear that the code was in desperate need of a massive cleanup. There were dead code, code duplication, unclear logic, misleading names, etc... So in the end the code is so different from what I started from that a side-by-side comparison might be impossible. You can see individual changes in the separate commits, or you can review the new version of the files as were they new additions. These refactorings also uncovered a bug in `git fork`, where adding `--no-remote` would disable `--sync` and `--setup-pre-push-hooks`. I fixed that also.
> 
> Unfortunately, we still don't have a testing story for our CLI tools. :-( I have however done extensive ad-hoc testing, with all possible combinations of flags, etc.

Looks good to me, too.

-------------

Marked as reviewed by kcr (Reviewer).

PR: https://git.openjdk.java.net/skara/pull/1226