threads / patch / 53332

patchrebase: display an error if --root and --fork-point are both provided

Subject: [PATCH] rebase: display an error if --root and --fork-point are both provided

## tl;dr

3 messages between Apr 27, 2020 and Apr 27, 2020. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Elijah Newren via GitGitGadget· Apr 27, 2020, 16:21 UTC · lore
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(+)
Show changes to builtin/rebase.c +3 −0
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

-- 
gitgitgadget
Alban Gruin· Apr 27, 2020, 16:46 UTC · re: Elijah Newren via GitGitGadget · lore

Re: [PATCH] rebase: display an error if --root and --fork-point are both provided

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 --
Show changes to Documentation/git-rebase.txt +5 −3
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
Elijah Newren via GitGitGadget· Apr 27, 2020, 17:59 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH v2] rebase: display an error if --root and --fork-point are both provided

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>
Documentation-by: Alban Gruin <alban.gruin@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    rebase: display an error if --root and --fork-point are both provided
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-771%2Fnewren%2Frebase-fork-point-root-error-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-771/newren/rebase-fork-point-root-error-v2
Pull-Request: https://github.com/git/git/pull/771
Range-diff vs v1:
 1:  c4508c7ddae ! 1:  307124b00d4 rebase: display an error if --root and --fork-point are both provided
     @@ Commit message
          segfaulting on a NULL pointer) if both are specified.
      
          Reported-by: Alexander Berg <alexander.berg@atos.net>
     +    Documentation-by: Alban Gruin <alban.gruin@gmail.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     + ## Documentation/git-rebase.txt ##
     +@@ Documentation/git-rebase.txt: 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>::
     +@@ Documentation/git-rebase.txt: 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
     + -----------------------
     +
       ## builtin/rebase.c ##
      @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
       			die(_("cannot combine '--keep-base' with '--root'"));
 Documentation/git-rebase.txt | 7 +++++--
 builtin/rebase.c             | 3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)
Show changes to 2 files +8 −2

Documentation/git-rebase.txt, builtin/rebase.c

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f7a6033607f..fbad356c68e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -414,12 +414,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>::
@@ -600,6 +602,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
 -----------------------
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

-- 
gitgitgadget

← back to recent threads