Re: [PATCH] rebase: display an error if --root and --fork-point are both provided
- From
Alban Gruin <alban.gruin@gmail.com>
- Date
- Apr 27, 2020, 16:46 UTC
- Message-ID
- <7f8b3ab5-9632-637d-27db-7e35330ff31d@gmail.com>
- In-Reply-To
- <pull.771.git.git.1588004500766.gitgitgadget@gmail.com>
Hi Elijah,
Le 27/04/2020 à 18:21, Elijah Newren via GitGitGadget a écrit :
Show 49 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> --root implies we want to rebase all commits since the beginning of
> history. --fork-point means we want to use the reflog of the specified
> upstream to find the best common ancestor between <upstream> and
> <branch> and only rebase commits since that common ancestor. These
> options are clearly contradictory, so throw an error (instead of
> segfaulting on a NULL pointer) if both are specified.
>
> Reported-by: Alexander Berg <alexander.berg@atos.net>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> rebase: display an error if --root and --fork-point are both provided
>
> --root implies we want to rebase all commits since the beginning of
> history. --fork-point means we want to use the reflog of the specified
> upstream to find the best common ancestor between and and only rebase
> commits since that common ancestor. These options are clearly
> contradictory, so throw an error (instead of segfaulting on a NULL
> pointer) if both are specified.
>
> Reported-by: Alexander Berg alexander.berg@atos.net
> [alexander.berg@atos.net]Signed-off-by: Elijah Newren newren@gmail.com
> [newren@gmail.com]
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-771%2Fnewren%2Frebase-fork-point-root-error-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-771/newren/rebase-fork-point-root-error-v1
> Pull-Request: https://github.com/git/git/pull/771
>
> builtin/rebase.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index bff53d5d167..287ac1aa21b 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1652,6 +1652,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> die(_("cannot combine '--keep-base' with '--root'"));
> }
>
> + if (options.root && fork_point > 0)
> + die(_("cannot combine '--root' with '--fork-point'"));
> +
> if (action != ACTION_NONE && !in_progress)
> die(_("No rebase in progress?"));
> setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0);
>
> base-commit: af6b65d45ef179ed52087e80cb089f6b2349f4ec
> The patch is fairly obvious and looks good to me.
The documentation must be updated, too, since it does not mention the incompatibility between the two switches:
-- snip --
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index bed500f151..a30de5aa20 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -448,12 +448,14 @@ When --fork-point is active, 'fork_point' will be used instead of <branch>` command (see linkgit:git-merge-base[1]). If 'fork_point' ends up being empty, the <upstream> will be used as a fallback. + -If either <upstream> or --root is given on the command line, then the -default is `--no-fork-point`, otherwise the default is `--fork-point`. +If <upstream> is given on the command line, then the default is +`--no-fork-point`, otherwise the default is `--fork-point`. + If your branch was based on <upstream> but <upstream> was rewound and your branch contains commits which were dropped, this option can be used with `--keep-base` in order to drop those commits from your branch. ++ +See also INCOMPATIBLE OPTIONS below. --ignore-whitespace:: --whitespace=<option>:: @@ -635,6 +637,7 @@ In addition, the following pairs of options are incompatible: * --preserve-merges and --empty= * --keep-base and --onto * --keep-base and --root + * --fork-point and --root BEHAVIORAL DIFFERENCES ----------------------- -- snap -- Cheers, Alban